[asto18089]

Summary

xAI's 2026-07 live incident (grok-4.5 400 "tool parameter root must be an object type" on apply_patch's root oneOf of required-groups) made us flatten every root-level oneOf/anyOf into a single object schema. xAI has since relaxed the rule in its published function-calling documentation, so the unconditional flattening now rewrites tool schemas that the provider explicitly accepts, erasing declared branch semantics.

Document evidence

xAI function-calling documentation — https://docs.x.ai/developers/tools/function-calling, verbatim: "A root anyOf or oneOf also works when every branch is itself an object…" (the live sentence continues with an illustrative clause). The page's only documented 400 is a root-shape verdict: "A tool whose parameters root is neither an object nor a union of objects (for example, a scalar, an array, or an anyOf/oneOf with a non-object branch) cannot be compiled into a tool-call grammar and is rejected with a 400 error that names the tool."

Incident record

The incident is recorded in upstream fix ab6c68a4d (2026-07-19, upstream PR codewhale-hq#4546, "Live-hit release blocker"): the live 400 fired on apply_patch's three required-groups (patch | replace | changes). Earlier in-repo annotations quoted a two-branch union; that quote was inaccurate and is corrected here.

Changes (head b7ab69b6e)

• sanitize_for_xai_parameters passes a root union through when the root is exactly one well-formed oneOf/anyOf whose every branch declares type object and the union stays semantically usable after the registry's exact preprocessing: the guard rehearses the dependent-keyword drop + sanitize + annotation-keyword strip on a clone and judges the projected shape by four kill rules — R1 two projected branches deep-equal (exactly-one violated for every instance), R2 at least two required-free object branches (the required group was the branch's only cross-branch discriminator; neither an optional property map nor any minProperties floor restores it — floors 0, 1 and 2 all leave every shared-root-valid instance clearing them matching both alternatives; reviews 5802378120, 5808313704, 5811366579), and two branches sharing an equally-constrained surviving required group (partially pruned groups: the cross-branch discriminators are dropped while a locally shared requirement like c survives on both branches — every root-valid instance then matches both; branches whose shared-field entries the containing root already implies (vacuous {} or identical to the root's own schema for the field) normalize to absent so root-implied variants of the same surviving group compare equal; review 5812238335), R2 (pairwise admission, reviews 5812488975 + 5814012140) an all-object union where some ordered branch pair stays indistinguishable over the root-valid domain (see below), and R3 a branch carries a dead inner union. Only a union the rules cannot kill ships unflattened; a killed union takes the flattening fallback, which keeps the shared root properties usable.
• Pairwise admission (new, b7ab69b6e): the shared-group arm fires only on signatures that normalize equal, and two more registry forms slipped past it while the union served no root-valid instance — a root strictly stronger than both branch-local shared-field entries (a minLength: 2 root dominating one branch's {} and the other's {"type":"string"}: unequal signatures, both implied by the root, so every root-valid instance matches both alternatives; review 5812488975), and two different surviving groups the enclosing root already requires in full ([a] and [b] under root required: ["a","b"]; review 5814012140). The new arm flips the default to fail-closed without becoming a subsumption solver: every ordered branch pair of an all-object union must stay discriminable by one of three bounded proofs, and the first unproven pair takes the flattening fallback — (1) a group name only the right branch requires and the enclosing required does not, so a root-valid instance may omit it; (2) disjoint const/enum pins on a shared field with a candidate value the enclosing entry admits; (3) a depth-bounded self-contained nested-group difference (keeps the reviewed nested-discriminator control shipping). Each arm's witness is an omission witness (carry the left branch's group plus everything the enclosing schema requires, omit one discriminating name — or pin one discriminating value on a shared field), and it counts as proven only when both shapes carry structural keys only and every carried field's entry is value-permissive (absent/empty/type/const/enum — a concrete satisfying value provably exists); a restrictive additionalProperties on the left branch, a sibling allOf/minProperties at the enclosing node, or a minLength on a carried field leaves the pair unproven and the union flattens (pinned by xai_sanitize_flattens_union_whose_witness_fields_carry_further_constraints). Everything unproven flattens.
• Single-traversal defect walk (new, b7ab69b6e): the R3 recursion and the outer keyword walk used to visit the same branch subtrees independently, doubling the work at every healthy nesting level — exponential on a linear-size document, executed synchronously during xAI request preparation for every tool-bearing request (reviews 5814012140, 5814046611). The walk now memoizes per node (stable addresses on an unmutated serde_json tree, single-parent nodes keep cached reasons path-exact), so each subtree computes exactly once.
• Everything the relaxation does not cover still goes through the Responses-API flattening pass: non-object roots, unions with a non-object branch, bare required-group branches (apply_patch keeps its protective flattening), semantically killed unions (R1–R3 above), composite roots — union with a sibling root allOf/enum/not, both union keywords at once (the doc names one union, not an intersection) — and malformed (non-array) or empty union values, which the flattening pass removes instead of shipping. Root-level keywords that are not composition operators (annotations, $defs, $ref, if/then/else, …) are outside the root-composition rule: they ride along beside the union exactly as they rode the flattening pass before.
• Branches without an explicit object type do not count as object branches: xAI's own 400 message called the incident's union "a union with a non-object branch".
• The incident rationale is retained in-code, anchored to the recorded upstream fix.
• Added a forkguard_xai_root_union_contract_passes_documented_and_flattens_composites behavior test (fork-register precedent): the pass-through is fork-distinct behavior (upstream still flattens all root unions), so the parent repo's docs/fork-modifications.md registration + fingerprint is a follow-up duty when the parent consumes this gitlink.
• Upstream-first: not attempted. Upstream relocated this file to crates/models/ and has diverged from the fork base, so the behavior ships as a registered fork modification; revisit an upstream contribution at the next sync.

Test evidence (red-green)

• xai_sanitize_passes_root_union_of_object_branches_through and xai_chat_keeps_root_union_of_object_branches fail against the old implementation (root union folded, branches erased). Mutation-verified: at the first commit's state, removing the guard turned exactly these two red; at the head the same mutation turns five red (these two plus xai_sanitize_passes_root_anyof_of_object_branches_through, xai_sanitize_union_passes_through_with_dependent_keyword_note, and the forkguard_ pin).
• xai_sanitize_flattens_apply_patch_root_one_of and xai_sanitize_still_flattens_root_union_with_non_object_branch pin the preserved 400-avoidance behavior (verified red when branch-type detection is loosened).
• Review-round additions, each verified red against a targeted mutation: xai_sanitize_passes_root_anyof_of_object_branches_through (anyOf spelling), xai_sanitize_flattens_object_union_with_root_all_of_sibling, xai_sanitize_flattens_object_union_with_root_enum_and_not_siblings, xai_sanitize_flattens_when_both_union_keywords_are_present, xai_sanitize_flattens_malformed_union_values, xai_sanitize_flattens_empty_root_union, xai_sanitize_union_passes_through_with_dependent_keyword_note, plus the forkguard_ contract pin (5 red against the loose guard, 4 red against an anyOf-blind loop, 1 red without the dependent-keyword drop).
• Third-round additions, each verified red against a targeted mutation: xai_sanitize_hoists_single_element_object_union (a len-1 object-branch union collapses to the equivalent plain object root — red under an always-true guard), xai_sanitize_union_with_bare_required_branches_passes_through_pruned (pins the disclosed registration-bypass edge outcome: branches pruned, no note), and xai_sanitize_still_coerces_scalar_root_to_object (scalar roots stay on the flattening pass — red under an always-true guard). Loosening the branch-type check turns 4 red (including the base-level xai_chat_lands_constraint_note_when_tool_description_is_empty wire pin).
• 42/42 xai-filtered tests green in codewhale-tui (28/28 at the base; 31/31 after the first commit; 39/39 before this round's pins).
• Fourth-round semantic-admission additions, each verified red against a targeted mutation: xai_sanitize_flattens_registry_pruned_cross_branch_required_union (R2 via the registry prune), xai_sanitize_flattens_oneof_gutted_branches_that_differ_only_in_annotations (R1 after the annotation strip), xai_sanitize_flattens_union_whose_branch_carries_dead_inner_union (R3), plus recursive-normalization and keep-required-belt pins.
• Fifth/sixth-round addition xai_sanitize_registry_path_flattens_minproperties_floored_cross_required_union walks the exact production order (sanitize THEN canonicalize, registry.rs:278-279) from the original valid schema for floors 1 and 2 and keeps each original witness valid ({"a":"x"}, {"a":"x","extra":"y"}); re-verified red under both the presence-only exemption and the floor-2 exemption, green under the required-free rule.
• Seventh-round additions (b7ab69b6e), each verified red against a targeted mutation in this round's code: xai_sanitize_registry_path_flattens_root_stronger_shared_required_union (review 5812488975's witness — red when the pairwise arm is disabled; production order sanitize THEN canonicalize, original witness {"a":"x","c":"zz"} stays valid), xai_sanitize_flattens_union_whose_groups_the_root_already_requires (review 5814012140's witness — red when the pairwise arm is disabled AND when arm 1's enclosing-required check alone is dropped; the flatten fallback keeps the "Exactly one of these parameter groups: a | b" note), xai_sanitize_flattens_union_whose_witness_fields_carry_further_constraints (a left branch carrying a restrictive additionalProperties, or a carried field entry carrying a minLength, leaves the pair's witness unprovable — red when the witness gate is disabled), and xai_union_defect_walk_computes_each_subtree_once (a 10-level deep healthy pinned-branch union: the walk must report no defect and stay at one computation per document node — red when the memo is removed, which reproduced the reported 2^depth re-traversal). Round mutations re-run on the final commit: disabling the pairwise arm turns 3 red (both witnesses + the witness-gate pin); dropping only arm 1's enclosing-required check turns the root-required-groups witness red; disabling arm 2 (pin discrimination) turns the const-differentiated pass-through pin red; disabling arm 3 (nested groups) turns the reviewed self-contained nested-discriminator control red; removing the memo turns the deep-chain pin red. Local totals at b7ab69b6e: 58/58 xai_-filtered lib tests, 95/95 schema_sanitize suite.

Testing

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features --locked (warning-free under the CI allow list — CI green on the prior head; a local stable-1.99.1 run reports 8 pre-existing lints in unrelated files: toolchain drift, zero lints in this PR's files)
• cargo test --workspace --all-features --locked — CI green through the prior head; the fork CI check job runs cargo nextest run --workspace --all-features --locked plus the doc-test pass. The b7ab69b6e head's CI run is the required gate for this round; the local codewhale-tui lib + xai suites are green on it.

Behavior changes

• Tool schemas whose root is a documented-good union of object branches now reach xAI unflattened; declared per-branch semantics survive the wire.
• All previously protected shapes — including composite roots and malformed/empty union values that an earlier revision of this PR would have passed through — flatten exactly as before.
• Narrowed by b7ab69b6e: unions whose projected branches are only root-dominated or root-required-duplicated shapes of each other (previously shipped through signature-inequality gaps), and unions whose branch shapes are too restrictive to prove an admission witness, now take the flattening fallback; the fallback keeps the shared root properties usable and re-derives the parameter-groups note. Genuinely discriminating unions — disjoint non-root-required groups, disjoint pins, self-contained nested-group differences — still ship unflattened.

Known merge surface

• fix(tui): replay reasoning_content for kimi k3.1 #137 (fix/kimi-k31-replay-spellings): hunks disjoint, no textual conflict (verified by compare; note it targets feat/host-armed-guardrail-prompts rather than pinvou3-clean — disjointness holds against that branch too).
• Sibling PRs fix(tui): hold Moonshot tool schemas to the MFJS docs #143/fix(tui): mirror MiniMax reasoning_details only on gateways #145/fix(tui): ship Anthropic input schemas verbatim #146/fix(tui): hold DeepSeek strict shaping to documented rules #147/fix(tui): let todo items default to a pending status #148 (same-day provider batch): fix(tui): mirror MiniMax reasoning_details only on gateways #145/fix(tui): ship Anthropic input schemas verbatim #146/fix(tui): hold DeepSeek strict shaping to documented rules #147/fix(tui): let todo items default to a pending status #148 clean both directions (fix(tui): hold DeepSeek strict shaping to documented rules #147 also touches schema_sanitize.rs but auto-merges); fix(tui): hold Moonshot tool schemas to the MFJS docs #143 add/add-conflicts with this PR in crates/tui/src/client/chat.rs (both append test blocks at the same anchor in mod image_block_wire_tests; the added test-name sets are disjoint, so whichever merges second rebases mechanically, keeping both test blocks).

Probe declaration

No live probes were used; the pass-through rule is grounded solely in the quoted published documentation, and the preserved flattening paths are pinned by the retained incident fixture plus the review-round mutations. Undocumented branch-level keywords inside object-typed branches now reach the wire as declared (pre-PR they were silently deleted); the doc's only documented 400 is a root-shape verdict, and the doc demonstrates branch type/properties/required/const as supported.

Two bounded edges are disclosed rather than handled:

1. Undocumented branch-level keywords — for example $ref, or a branch-top-level nested union — now reach the wire as declared; pre-PR the flattening erased the whole root union, so the exposure delta is limited to branch-top-level keys (unions nested inside branch properties already shipped pre-PR via the property merge).
2. For schemas that bypass registration-time sanitize — parent-engine MCP tools, whose McpPool::to_api_tools clones input_schema verbatim — nothing in this PR changes the shipped bytes (pre-existing behavior; no pass-through decision and no note are derived on that route). On the sanitize route, the fix-shape choice this body previously left open is now resolved by the semantic guard for its fully-degenerate forms: any union whose branches end up required-free after the prune (R2, floors irrelevant), share an equally-constrained surviving required group (R2), or are pairwise indistinguishable over the root-valid domain (R2 pairwise admission — root-dominated shared-field entries, root-required duplicated groups) takes the flattening fallback, so the "Exactly one of these parameter groups" note survives for them. A union whose branches keep genuinely discriminating required groups still passes through, and those silently pruned dangling entries still lose the note — disclosed rather than handled, for the same reason as before: the loss is confined to branch-level constraint metadata, the surviving union remains semantically usable, and neither shape risks the documented 400. The pairwise arm's residual is the boundary of its witness proofs: an instance whose carried fields' entries are only partially checkable (an exotic-but-permissive-in-reality keyword such as a branch-level format) takes the flattening fallback rather than shipping, so the boundary can only ever cost branch-level metadata on the flatten side, never wire-side unusability.

No-Issue: provider compatibility batch grounded in the published provider docs; the repository has issues disabled.

[asto18089]

Fresh review (12-lane, from scratch) + fix wave 7552ef05c

Reviewed from a fresh clone at base tip 11cba403dc. Rebase status: no-op by inspection — the branch sat exactly on the current pinvou3-clean tip (ahead 1 / behind 0, merge-base = tip), so no rebase push was needed; the fix wave below lands on top of it.

What the review verified (all confirmed)

• Claim 1 (pass-through mechanism), Claim 3 (bare required-groups still flatten), Claim 4 (incident annotation retained), Claim 5 (red-green + 31/31): delivered. Baseline cargo test -p codewhale-tui --lib --locked xai = exactly 31/31 as claimed; removing the guard turns exactly the two new tests red with the claimed failure mode ("root oneOf must survive" — union folded, branches erased); loosening branch-type detection turns both preserve-pins red. Doc quotes verified verbatim against the live page (2026-10-10 wording matches; the page's only documented 400 is the root-shape rule). Incident narrative is genuine repository history: upstream fix ab6c68a4d (upstream PR fix(xai): flatten root oneOf tool schemas rejected with 400 codewhale-hq/Codewhale#4546, merged 2026-07-19T00:30Z, "Live-hit release blocker") records the exact 400 text quoted.
• Diff integrity: CLEAN — 5 hunks / 2 files, byte-identical to the compare API, single commit, DCO present, no smuggled changes, no fmt churn.
• Integration: single production call site (chat.rs:1202); xAI is pinned to WirePolicy::Fixed(ChatCompletions), so no Responses/Anthropic lane can bypass the change; every tool's parameters are sanitized; registry pre-pass doesn't pre-flatten unions.
• Performance: no regression — the passthrough route is strictly cheaper (skips merge clones + root fixups); the flatten route adds one shallow O(branches) scan; no new allocations, no stack-depth change; note re-append across turns is impossible (per-request clone-then-decorate, byte-stable descriptions).
• Conventions: commit message, DCO, single-purpose scope, changelog exclusion all compliant; existing chat.rs guard pins (T1/T7) unaffected.

Findings (all fixed in 7552ef05c)

REQUIRED-1 — the guard passed through shapes the relaxation does not cover, contradicting the PR's own contract. The PR said "root allOf/enum/not … still goes through the flattening pass", but root_union_is_all_object_branches inspected only oneOf/anyOf, so a root {oneOf: [object, object], allOf: [...]} (or with enum/not, or both union keywords at once — an intersection the doc's singular "a root anyOf or oneOf" doesn't describe) sailed through with the composite keywords intact — undocumented grammar shapes with no per-tool fallback, i.e. the exact incident class this shim exists to prevent. A present-but-malformed union value ("anyOf": null beside a valid oneOf) likewise rode to the wire. Fixed: the guard now requires exactly one well-formed union and no sibling root allOf/enum/not; composites and malformed values flatten exactly as before the relaxation. Pinned red by xai_sanitize_flattens_object_union_with_root_all_of_sibling, ..._with_root_enum_and_not_siblings, ..._when_both_union_keywords_are_present, ..._malformed_union_values (restoring the loose guard turns all five new pins red).

SHOULD-1 — three untested decisions of the new route (mutation-survivors on the original head). An anyOf spelling of the documented-good shape, the empty-union→flatten route, and the dependent-keyword note on the pass-through route had no pins (mutating each decision left 31/31 green). Added xai_sanitize_passes_root_anyof_of_object_branches_through, xai_sanitize_flattens_empty_root_union, xai_sanitize_union_passes_through_with_dependent_keyword_note; each verified red against its targeted mutation.

SHOULD-2 — fork-register precedent. The pass-through is fork-distinct behavior (upstream main still flattens all root unions). Per the register precedent (forkguard_* tests shipped in CodeWhale #62/#79), added forkguard_xai_root_union_contract_passes_documented_and_flattens_composites pinning both halves of the contract. The parent repo's docs/fork-modifications.md entry + fingerprint remains a follow-up duty when the parent consumes this gitlink (no fork-modifications.md/fork-guard.sh exists in this repo, so nothing to update here).

MINOR-1 — inaccurate incident annotation. The in-code comment quoted the trigger as a two-branch union [{required:["patch"]}, {required:["changes"]}]; the actual schema (and the original ab6c68a4d commit message) has three required-groups (patch | replace | changes). Corrected and anchored to the recorded fix commit; the unrecorded "2026-07-18" incident date is replaced by the recorded fix date.

MINOR-2 — PR template. Body now uses the template with Testing/Checklist ticked against actually-run commands; the release-lane clippy status is disclosed honestly (CI green; local stable 1.99.1 flags 8 pre-existing lints in unrelated files, zero in this PR's files).

Non-blocking observations (no action requested)

• Branch-level keywords inside object-typed branches (additionalProperties, patternProperties, prefixItems, nested composition, branch title/default…) now reach the wire as declared where pre-PR flattening silently deleted them. The doc's only documented 400 is a root-shape verdict and its examples demonstrate branch type/properties/required/const, so this is doc-consistent; the probe declaration already concedes the grounding is doc-only. If a live probe ever 400s on a branch keyword, that is the moment to consider a Moonshot-style per-tool omission lane for xAI — not before.
• prune_dangling_required can silently drop a branch's required entries when they aren't declared in that branch's properties (no note on the pass-through route). This is the module's pre-existing dirty-schema doctrine applied identically on every lane; it is live for MCP-sourced tools whose schemas skip the registry pass. Left as-is deliberately.
• mod image_block_wire_tests in chat.rs is a pre-existing catch-all (26 mostly non-image tests, including the prior xAI wire test); the new test lands beside them per existing practice. A rename/split is a follow-up, out of scope.
• Pre-existing Custom-provider divergence: a Custom provider aimed at an xAI endpoint bypasses the xAI branch entirely (chat wire: no sanitization) — out of this PR's enum scope.
• Merge surface: fix(tui): mirror MiniMax reasoning_details only on gateways #145/fix(tui): ship Anthropic input schemas verbatim #146/fix(tui): hold DeepSeek strict shaping to documented rules #147/fix(tui): let todo items default to a pending status #148 and fix(tui): replay reasoning_content for kimi k3.1 #137 verified clean against this branch; fix(tui): hold Moonshot tool schemas to the MFJS docs #143 add/add-conflicts in chat.rs (both append tests at the same anchor) — mechanical rebase, keep both test blocks.
• Environment note: one unrelated provider_picker test flaked once under heavy parallel load on this machine and passed in isolation/baseline; unrelated to this change.

Post-fix gates: cargo fmt --all -- --check clean; xai filter 39/39 green (was 31/31); scoped clippy -D warnings zero lints in changed files; mutation battery red/green as described above.

[asto18089]

Fresh from-scratch review round 2 (10 lanes) + follow-up 63b594472

Re-ran the entire review from zero after verifying the rebase surface: pinvou3-clean is still at 11cba403dc (0 behind, merge-base = tip), so the rebase is a mechanical no-op — proven in a fresh clone (git rebase → "Current branch is up to date", tree unchanged). Everything below re-verified from scratch in a clean GitHub clone; earlier-round conclusions were treated as unproven.

What was re-verified and held:

• Diff integrity / no smuggling: 2 files, 5 hunks, every hunk serves the claim; byte-exact three-way composition (commit 1 patch + fix-wave patch reconstruct both final files bit-for-bit); all 11 deleted lines are comments; the touched pre-existing test keeps every assertion byte-identical; no CI/lockfile/version/other-crate touches, no hidden characters, no test gating.
• Claims vs implementation: all behavioral claims verified with file:line evidence (exactly-one-well-formed-union guard with fail-closed branch typing; flatten preserved for non-object roots, non-object branches, allOf/enum/not siblings, both union keywords, malformed/empty unions; apply_patch still flattens against its real in-repo schema; passthrough applies the dependent-keyword drop + semantics-preserving sanitizer; incident anchor ab6c68a4d facts exact — three required-groups, upstream fix(xai): flatten root oneOf tool schemas rejected with 400 codewhale-hq/Codewhale#4546 "Live-hit release blocker").
• Fork-distinctness: upstream (codewhale-hq/Codewhale main) still flattens all root unions — its sanitize_for_xai_parameters is byte-identical to this fork's base; no upstream commit since ab6c68a4d relaxes it; zero upstream hits for the passthrough. Not duplicate work; upstream-first status now disclosed in the body (file relocated to crates/models/ upstream, ~700 lines diverged).
• Docs grounding: both quoted xAI doc sentences verified verbatim against the live page (page footer "Last updated: October 10, 2026" matches the cited date); the doc's own email/sms union example matches the PR's fixture shape.
• Integration: exactly one production call site (chat.rs:1202); xAI is pinned to the ChatCompletions lane, so every xAI request is covered; sanitize runs per-request on a wire copy; sibling sanitizers (Moonshot/Kimi/DeepSeek/Anthropic/Responses) untouched — non-xAI behavior bit-identical to base.
• Merge surface: all three body claims mechanically verified — fix(tui): replay reasoning_content for kimi k3.1 #137 hunks disjoint; fix(tui): mirror MiniMax reasoning_details only on gateways #145/fix(tui): ship Anthropic input schemas verbatim #146/fix(tui): hold DeepSeek strict shaping to documented rules #147/fix(tui): let todo items default to a pending status #148 clean in both merge directions; fix(tui): hold Moonshot tool schemas to the MFJS docs #143 add/add-conflict confined to chat.rs at the shared mod image_block_wire_tests anchor with disjoint test-name sets, and a keep-both resolution was actually built and passes cargo test -p codewhale-tui --lib image_block_wire_tests 26/26.
• Tests & mutations (executed, not assumed): 39/39 xai filter confirmed at head. Guard-deletion mutation → 5 red (see body correction below). Loose-guard mutation (commit-1 behavior ported) → exactly the 5 predicted reds. anyOf-blind loop → exactly 4. Dependent-drop removal → exactly 1. cargo fmt clean. CI on the head is fully green, including the fork CI check job that runs cargo nextest --workspace --all-features --locked + doc tests.
• Performance: passthrough is a strict subset of the old flatten path's work (drops note formatting, merged-property deep clones, root-level pruning) plus a shallow allocation-free O(branches) guard, once per tool per request; no new recursion, clones, repetition, or lock changes. In-place mutation at the call site — no schema clone.

Found and fixed in this round (63b594472 + body rewrite):

1. Body accuracy — "head delta adds tests only" was false: the fix wave rewrites the production guard (root_union_is_all_object_branches). Full-workspace coverage does exist for the head (fork CI check green on 7552ef05c), so the Testing bullet now cites that correctly. The mutation-count claim ("exactly those two red") was true at the first commit's state but stale at the head (5 red) — now stated with both scopes. "(was 31/31)" clarified to "(28/28 at the base; 31/31 after the first commit)".
2. Guard doc — the flatten fall-through enumeration omitted the empty union (code and test already handle it); the doc comment now names it (63b594472, comment-only).

Found, disclosed in the body, deliberately not fixed here (follow-up decision):

• Bounded MCP-path edge: parent-engine MCP tool schemas reach the xAI lane raw (McpPool::to_api_tools clones input_schema verbatim — the registry sanitize at registry.rs:278 covers native/dynamic/plugin/subagent tools but not parent-catalog MCP tools). For such a schema with a root union of object-typed branches carrying bare required, base surfaced the "Exactly one of these parameter groups must be provided" note via flattening; the head passthrough prunes the bare required (dangling, after properties:{} injection) and ships identical unconstrained branches with no note. Empirically proven by running both revisions on the same input. Bounded because the schema-level constraint is destroyed on both routes, neither shape risks the documented 400, native tools are unaffected (pre-eroded at registration identically on base and head), and for properties-backed branches the passthrough preserves strictly more contract than base. The two fix shapes (route bare-required unions back to flattening vs. surface the composition note on the passthrough route) trade doc-letter fidelity against verbal-contract parity — left as a follow-up rather than smuggled into this PR.
• Probe declaration now also names $ref/branch-top-level nested unions as undocumented keywords that reach the wire as declared.

Non-blocking notes (no action taken): fixture JSON duplication across three passthrough tests (~90 lines, file-idiomatic inline style); forkguard_ test cites the parent register generically (no concrete entry exists yet — companion duty at gitlink consumption, as declared); incident date appears as 2026-07-18 (commit 1, local) and 2026-07-19 (head, UTC of ab6c68a4d) — timezone-consistent but mixed; the new chat wire test lives in mod image_block_wire_tests following the #112 precedent though it tests schemas, not image blocks; provider_picker::xai_auth_status_distinguishes_oauth_from_api_key_auth showed a pre-existing env-race flake under parallel mutation runs (passes in isolation, non-causal to this PR — in line with the known credential-test flake family).

Gates re-run in this round: fmt clean; 39/39 xai green; scoped clippy still clean in changed files; fork CI green on the pushed head. Ready for re-review; the only open thread is the disclosed MCP-path edge.

[JensenChen28]

基于 head 63b5944723b7108663586fdb158e3898bbf30b7e / base 11cba403dc6bd6ca5c4dca55042785b03449ac8e 完成规范与需求两个轴的完整差异审查。两个轴各确认同一个 P2，需先修复后再批准。

实际验证：fetch 固定 base/head、完整源码与 MCP→chat 调用链检查、git diff --check 通过，并运行基于源码推导输出的有限 JSON Schema 语义反例（不是 Rust sanitizer 的执行）。已只读核对远程 CI；未运行本地 Cargo 测试或真实 xAI 请求，不将作者测试计数视为本次执行结果。提交前已复核 head/base、正文和讨论未变化。

[JensenChen28]

[P2] 避免把有效的 MCP oneOf 转成不可满足的约束。有效输入可在根级声明 properties a/b，并使用 oneOf:[{type:"object",required:["a"]},{type:"object",required:["b"]}]；例如 {"a":"x"} 原本只匹配一个分支。这两个分支通过新 guard 后，sanitize 会各注入空 properties 并剪掉 required，得到两个相同的 {type:"object",properties:{}}。此时每个对象都匹配两个分支，因而没有对象能满足 oneOf。McpPool::to_api_tools (mcp.rs:4065–4074) 原样复制 input_schema，故原生工具的注册期 sanitize 并未排除这个触发条件。旧路径会移除该 union 并保留参数组说明，仍可发出可满足的对象 schema；新退化比 PR 已披露的说明丢失更严重。请对无法保留约束的分支继续使用 protective flattening，或安全保留 required，并补充这个 raw-MCP 回归场景。仅补说明文字不足以修复不可满足的 oneOf；这里不声称已观察到 provider 400。

[asto18089]

R3 from-scratch review complete — no blocking findings; polish wave pushed (ca58cb972, tree byte-identical to the locally built commit; pushed via the Git Data API because github.com git endpoints are unreachable from this sandbox).

Scope: 9 fresh lanes (claims/root-cause, smuggle, guard logic, adversarial regression hunt, runtime battery, performance, elegance, body audit, merge surface) + live xAI-docs re-verification + a self-run cargo mutation battery. Base pinvou3-clean never advanced (base tip == merge-base 11cba403dc → rebase no-op re-proven); CI was green on the prior head and re-runs on the new one.

Verified again from scratch

• Root-cause placement: the guard lives in the single xAI sanitize choke point covering every CodeWhale-built ChatCompletions path (stream/non-stream, warmup, retries, tool-loop); the app-server forwarder never sanitized (unchanged, out of claim scope). No built-in tool ships the pass-through shape today — the beneficiaries are external schemas (MCP/dynamic/plugin), which is the honest, claimed population.
• Zero smuggle: 2 files, 568+/11−, every deletion a comment; per-commit story exact (22776fe impl → 7552ef0 guard narrowing → 63b5944 comments); unicode/CRLF/whitespace scans clean; test-name delta +11/−0.
• Runtime: 42/42 xai-filter green at the new head (39/39 at the prior head, 28/28 at base — all three counts re-measured). Mutation battery re-executed: guard-delete → exactly the claimed 5 red; loose branch typing → 4 red (including the base-level xai_chat_lands_constraint_note_when_tool_description_is_empty wire pin); anyOf-blind → the claimed 4 red; dependent-drop / allOf-sibling / empty-union / both-keywords mutations → 1/2/1/1 red as claimed; the xai_auth_status provider-picker failure reproduces only under concurrent load and passes isolated (non-causal, environmental).
• Live docs basis holds today (page dateModified 2026-10-10): both quotes verbatim-exact; the keyword-support disclosure matches exactly what the page demonstrates (type/properties/required/const in branches; no $ref/prefixItems/branch-nested unions anywhere on the page).
• Perf claims confirmed: allocation-free O(B) guard, pass-through work a strict subset of the old flatten work, no schema clone at the call site; wire-size growth bounded by the declared input schema.
• Merge surface re-verified by merge-tree: fix(tui): mirror MiniMax reasoning_details only on gateways #145/fix(tui): ship Anthropic input schemas verbatim #146/fix(tui): hold DeepSeek strict shaping to documented rules #147/fix(tui): let todo items default to a pending status #148/fix(tui): replay reasoning_content for kimi k3.1 #137 clean both directions; fix(tui): hold Moonshot tool schemas to the MFJS docs #143 add/add at the image_block_wire_tests anchor with disjoint test-name sets — re-checked against fix(tui): hold Moonshot tool schemas to the MFJS docs #143's mid-review force-push (tip bed7891); conflict and disjointness persist. fix(tui): hold DeepSeek strict shaping to documented rules #147's schema_sanitize.rs overlap is textual only, no semantic interaction (its strict-lane changes leave sanitize, sanitize_for_responses, drop_dependent_keywords, and sanitize_for_xai_parameters untouched).
• Incident anchor re-verified: upstream ab6c68a4d / fix(xai): flatten root oneOf tool schemas rejected with 400 codewhale-hq/Codewhale#4546 (2026-07-19, "Live-hit release blocker", three required-groups patch | replace | changes), and upstream HEAD still flattens all root unions — fork-distinctness stands; upstream-first remains correctly disclosed as unattempted.

Findings → dispositions (all polish; 0 BLOCKER, 0 MAJOR)

1. Body Testing bullet attributed the guard narrowing to "the head commit" — it was 7552ef0; commits after it are comment/pins-only. → body corrected.
2. Disclosure 2 said "bare required"; the mechanism (silent prune_dangling_required) also fires for mixed partially-dangling groups (["a","patch"] keeps a, silently drops patch, no note). → disclosure wording widened.
3. chat.rs comment said branches pass "intact" — len-1 unions are hoisted by collapse_single_element_unions and branch-level normalization applies. → comment qualified.
4. Guard doc's "everything the relaxation does not cover" overclaimed: non-composition root keywords ($ref, if/then/else, annotations) ride the pass-through exactly as they rode the flattening pass. → doc comment now states the composition-operator rule and the rider behavior; body updated.
5. Quote (a) was a mid-sentence truncation without ellipsis. → ellipsis + continuation note added.
6. Three implicit outcomes were unpinned. → added xai_sanitize_hoists_single_element_object_union, xai_sanitize_union_with_bare_required_branches_passes_through_pruned (pins the disclosed MCP-edge outcome), xai_sanitize_still_coerces_scalar_root_to_object (the observable zero-union-route pin; also red under an always-true guard).

Still open (unchanged, deliberate): the follow-up decision on the disclosed registration-bypass edge — route bare-required unions back to the flattening pass vs surface the composition note on the pass-through route — and the parent-repo docs/fork-modifications.md + fingerprint duty when the parent consumes this gitlink.

[JensenChen28]

已复审最新 head ca58cb9720788cdfedae28fb11e76bbe4ba39ac8（base 11cba403dc6bd6ca5c4dca55042785b03449ac8e）。规范与需求两个轴均确认原 P2 仍未修复，继续请求修改；沿用现有线程，未重复创建相同 inline finding。

crates/tui/src/tools/schema_sanitize.rs:98 的生产逻辑与上一已审 head 相同。新增 xai_sanitize_union_with_bare_required_branches_passes_through_pruned 直接断言两个分支都变成相同 {type:"object",properties:{}}，因此固定了问题输出：每个 object 都匹配两支，无法满足要求恰好一支匹配的 oneOf。这并非仅丢失说明或与旧 flatten 路径等价；旧路径输出可满足的 object，新路径的合法对象集合为空。PR 披露该边界不能消除回归，且仅恢复描述 note 也不足以修复。请对这类会被 prune 的分支保留可用的 protective flatten 路径，或安全保留实际约束，并把新测试改为验证有效实例仍可用。

实际验证：完整累计差异及新增提交源码复核、MCP→chat 可达链、git diff --check、生产代码去注释比较及已保存的有限语义反例。没有运行 Rust/Cargo 或真实 provider 请求；不声称观察到 HTTP 400。发送前 head/base、正文和最新讨论已复核未变化。

[zhuowp]

Request changes on ca58cb9720788cdfedae28fb11e76bbe4ba39ac8, reviewed against pinvou3-clean at 11cba403dc6bd6ca5c4dca55042785b03449ac8e.

[P2] Preserve satisfiable required groups before retaining the root oneOf. At crates/tui/src/tools/schema_sanitize.rs:93–97, an object-typed branch is admitted to the new pass-through path even when its required fields are declared in the root's shared properties. The subsequent general sanitizer prunes those branch requirements and leaves the root oneOf intact. For a valid pair such as oneOf: [{type: "object", required: ["patch"]}, {type: "object", required: ["changes"]}] with both fields declared at the root, the output has two identical {type: "object", properties: {}} branches. Every object matches both branches, and nonobjects match neither: no JSON value satisfies the resulting oneOf.

Raw MCP schemas reach this path through to_api_tools, the active catalog and the actual xAI Chat request builder. The previous fallback removed the root union and retained usable root properties plus the required-group note; this is a new regression for a valid input schema shape. The added bare-required test currently asserts the impossible output, so its passing result does not establish a callable contract. Preserve the effective requirements, or keep shapes that cannot survive the general sanitizer on the existing flattening path. Add a check that an originally valid argument still satisfies the emitted schema.

This finding is limited to the qualifying schema/provider combination; complete object variants and the existing unsupported-shape fallbacks were separately checked. Source and isolated production-function comparisons establish the transformation and actual request reachability. No live xAI request was made, and no particular HTTP status or remote failure text is claimed. Current CI and local/isolated execution are recorded separately in the v3 review artifacts. This source verdict grants no merge or release permission.

[asto18089]

已修复，头 53367f56b（ca58cb97 之上单提交），同时关闭 5788075613、5790040245、5796658406 与 inline 4482870825 的同一 P2。感谢两家独立指认同一退化。

修法（取两家一致处方：对会被剪掉约束的分支保留 protective flatten）：pass-through guard 由单一形状判定扩为 root_union_is_all_object_branches && union_branches_keep_required——后者对 union 每支逐条镜像 prune_dangling_required 的保留条件（required 条目须是分支自身 properties 的键），任一支会被剪即判负、整体回落 flatten。选「任一支」而非「全部」：任一支放宽都会改变恰一匹配的实例集，保守判定最安全。flatten 路径在 sanitize 之前从原始 schema 重导出 parameter groups note，约束以说明形式保留、输出可满足。未选「安全保留 required」：通用 sanitizer 对全类剪 dangling required，强留会破坏该 pass 的类级不变量并 fork 线上契约。McpPool::to_api_tools 原样复制 input_schema 无需改（注册态保留原始形状，xAI chat 路由是权威门禁）。

测试：坏钉翻转 …_passes_through_pruned → xai_sanitize_flattens_union_with_bare_required_branches（断言 flatten 输出 + note）；新增 raw-MCP 回归（根 properties a/b + 双 bare-required 分支）：断言 union 不上线、根 properties 存活、note 保留，并以仓内 jsonschema（Draft 2020-12）断言有效实例 {"a":"x"} 仍通过。既有 pass-through 正例（required 被分支自身 properties 覆盖或本不 qualify）路由不变、全部保留通过；client::chat 两个 xAI wire 测试同绿。cargo：schema_sanitize 78 passed / 0 failed；rustfmt --check 干净。

[JensenChen28]

已复审最新 head 53367f56b30884ea26342d16c826106ae6269820（base 11cba403dc6bd6ca5c4dca55042785b03449ac8e）。规范与需求两个轴均确认原 P2 已在源码层面修复，未发现新的实质问题，批准本 PR。

对应原线程：新 guard 在 sanitize 前检查 oneOf/anyOf 每个分支的 required；裸 required、部分 dangling 或非字符串条目将回到既有 flatten 路径，保留根级 properties，并从原始 schema 恢复参数组说明。此前 a/b 反例不再发出两个相同的 oneOf 分支。旧的缺陷输出测试已改为检查 flatten/note，新回归测试还使用 jsonschema 验证原合法实例 {"a":"x"} 仍有效。结构完整的对象分支仍可保留 union，既有 composite/malformed 保护未变。

非阻断文案建议：PR 正文的旧 head、测试归因与“此 MCP 边界留待后续”的描述尚未同步，请更新为本次实际修复；字面 @/tmp/144-reply.md 评论未包含可读的回复内容。

实际验证：fetch 固定 base/head、累计完整差异与新增提交/MCP 调用链源码检查、git diff --check，以及基于源码的有限分支谓词反例。未在本机执行 Rust/Cargo 测试或真实 provider；新 jsonschema 测试仅已审源码，不声称本机运行。已只读核对该 head CI，check/部分分析仍运行中，批准不表示所有门禁已完成。父仓消费 gitlink 的 fork 登记/指纹与集成验证仍属对应父仓变更。发送前已复核远程 head/base、正文和讨论未变化。

[zhuowp]

Request changes on 53367f5: one remaining P2 in the same schema-preservation root cause.

The direct raw-MCP case is fixed. However, an allowed and activated MCP tool in the normal child-agent registry path, or a script plugin, is sanitized by ToolRegistry before reaching the xAI helper. For an object schema with shared root properties and object-typed required-only alternatives, that earlier pass has already removed the branch requirements. The new guard in crates/tui/src/tools/schema_sanitize.rs therefore sees nothing left to prune and preserves two identical object branches in oneOf. No JSON argument can satisfy exactly one of those branches. The target branch's fallback removes that union and leaves the shared properties usable.

Please cover the actual registry-to-provider transformation, including information already removed by preprocessing and recursive branch cleanup, rather than checking only requirements still present in the direct input. Keep the valid self-contained-object union behavior and the existing compatible fallback.

This affects tools with the stated schema shape when they are allowed, activated and sent through xAI Chat; it does not affect every MCP tool. Current production-module probes and complete source-chain checks support the finding. No live provider rejection or HTTP 400 is claimed. The cumulative two-file change and affected complete test bodies were reviewed; merge and release remain subject to the repository's gates and authorization.

[asto18089]

已修复，头 a8e94c263（53367f56 之上单提交），关闭 review 5799828607。接受——前提经第一手核实：tools/registry.rs:278 注册路径确实对每个工具 schema 先跑通用 sanitize（剪掉挂在根共享 properties 上的分支 required），xAI helper 到手时 keep-required 守卫「无可可剪」即放行，两个被剪成相同的裸 object 分支以不可满足的恰一 oneOf 上线。

修法：pass-through 守卫追加第三条 oneof_branches_mutually_distinct——oneOf 的 object 分支两两深等即整体回落 flatten（相同分支对恰一语义恒不可满足，无论它如何变成相同：registry 预剪、raw MCP 直运、或其它上游清洗，全部覆盖）。按处方保持两条既有行为不变：自含相异分支的合法 union 继续原样 pass-through；anyOf 重复分支恒可满足（"至少一个"）继续 pass-through，不动。flatten 回落保留根共享 properties 可用，与既有 fallback 一致。

测试：新增 xai_sanitize_flattens_oneof_whose_bare_branches_arrive_already_identical——先跑注册路径同款 sanitize 模拟 ToolRegistry 预处理、再把产物喂给 xAI helper（即评审要求的 registry-to-provider 全链覆盖），断言 union 不上线、根共享 properties 存活、note 为空（裸分支无 required 组）；同测钉住 anyOf 孪生形状仍 pass-through。既有正例（自含 distinct 分支）全部保持通过。schema_sanitize 79 passed / 0 failed（前头 78 + 新增 1）、xAI wire 44 / 0、rustfmt --check 干净。

[JensenChen28]

Standards

Request changes at a8e94c2639d5a39d64c0774e382fb0072ad0062f against 11cba403. My earlier approval at 53367f56 was too broad: the direct raw-input repair did not establish compatibility for every normalization path. The latest fix correctly handles already-pruned identical flat branches, but a recursive-cleanup case remains.

[P2] Check the branches after recursive normalization (crates/tui/src/tools/schema_sanitize.rs:102). All three admission checks inspect the original parameters, then sanitize recursively changes the branches. This valid input passes those checks:

{"type":"object","properties":{"inner":{"type":"object","properties":{"a":{"type":"string"},"b":{"type":"string"}}}},"oneOf":[{"type":"object","properties":{"inner":{"type":"object","required":["a"]}},"required":["inner"]},{"type":"object","properties":{"inner":{"type":"object","required":["b"]}},"required":["inner"]}]}

The top-level branch requirements name their own declared inner property, and the branches are distinct before normalization. Recursive sanitation then injects empty properties into each inner object and removes its a/b requirement. Both root alternatives become identical. No argument can satisfy exactly one, although {"inner":{"a":"x"}} satisfies the original schema. The target's flattening fallback retains the shared root inner properties and remains usable, albeit permissive.

An admitted active raw MCP tool reaches this helper through the parent's MCP catalog without the registry prepass. This is a concrete schema-shape regression, not a claim about all MCP tools or an authorization bypass. Checking only original branch equality cannot prevent equality manufactured by recursive cleanup.

Spec

The direct flat-input repair and new registry-flat repair remain credited. Distinct self-contained object unions and duplicate anyOf controls retain the intended behavior. Compatibility is still partial for recursively normalized branches. Please base the preservation decision on the effective normalized shape and cover that actual transformation, preserving the compatible fallback and self-contained controls. The new already-identical fixture does not exercise initially distinct nested constraints.

Verification: latest cumulative two-file diff and raw-MCP call chain, clean exact-head checkout, full diff check, and a restricted source-transcribed transformation with independent object/required/exactly-one semantics and controls. No local Rust/Cargo, real provider rejection or HTTP 400 is claimed; peer executions were not adopted. CI success does not establish compatibility for this input.

Standards: one P2 compatibility blocker; prior description advisory remains. Spec: one remaining P2 in the same root cause. No merge or other-reviewer dismissal performed.

[zhuowp]

Request changes on a8e94c2: one remaining P2 in the same schema-transformation root cause.

The previously reported plain, already-identical registry branches now correctly fall back. However, oneof_branches_mutually_distinct compares raw JSON values before the remaining lossy transformations. Different branch title/description annotations make those values unequal without making their validation constraints different. A valid shared-root required-group schema can therefore pass through the actual registry sanitizer/canonicalizer, lose its branch requirements, and still reach xAI with two validation-equivalent object branches in oneOf. No JSON argument can satisfy exactly one branch.

There is also an ordering case in the raw MCP path: two initially different branches can pass the equality guard and then become identical when drop_dependent_keywords removes their only distinguishing constraints. The target branch's fallback keeps the previously valid argument usable. These are the same effective-schema issue, not two independent findings.

Recursive normalization can also produce this collapse using only ordinary type, properties and required; the remaining issue is not limited to unsupported conditional keywords.

Please evaluate the validation-relevant branches after the complete normalization/projection sequence, preserving the compatible fallback when the union cannot remain usable. Cover the real registry sequence and post-transformation collapse while retaining valid object-union and anyOf behavior. This does not require a general JSON Schema solver.

The cumulative two-file change and all 17 affected complete test bodies were reviewed. Current named CI passes the affected tests; those fixtures do not cover these residual cases. Static production-chain and schema evidence support this conditional tool-contract failure. No live HTTP 400, deployed frequency or universal MCP failure is claimed. A review-only auxiliary cleanup failure is excluded from passing execution evidence. Merge and release remain subject to repository gates and authorization.

[asto18089]

已修复，头 c6084478a（a8e94c263 之上单提交），关闭 review 5800015446。接受——反例成立：分支 required 组命名的自家嵌套对象在输入级三项检查全过（required 属于分支自身 properties、分支初始相异），但递归清洗会剪掉嵌套组的悬空 required 并注入空 properties，把初始相异的两支坍缩成相同裸对象 → 不可满足恰一；且经父 MCP catalog 直达本 helper 的 raw-MCP 工具不带 registry 预剪，输入级假设不成立。

修法（按处方：判定落在有效归一化形状上）：pass-through 决策前先在克隆上演练完全相同的变换（drop_dependent_keywords + 递归 sanitize），仅当归一化后分支两两深等检查通过才对原参数提交 pass-through；否则原始未变形参数走 flatten 回落（共享根 properties 保留、{"inner":{"a":"x"}} 等有效实例保持有效）。该归一化后判定子sume输入级查同（registry 预剪形状天然被覆盖，5799828607 回归测试保持绿），union_branches_keep_required 保守带原样保留（混合裸 required 分支仍按既有行为回落 flatten）。

测试：新增 xai_sanitize_flattens_oneof_distinct_only_before_recursive_normalization——评审原样反例（嵌套 required a/b + 根共享 inner 声明）断言 union 不上线、根 inner 存活、jsonschema 断言 {"inner":{"a":"x"}} 有效；对照臂=嵌套组自含（required 落在嵌套对象自身 properties）归一化后仍相异 → 保持 pass-through。既有全部正例（自含 flat、anyOf 重复、registry 预剪）保持绿。schema_sanitize 80 passed / 0 failed（前头 79 + 新增 1）、xAI wire 45 / 0、rustfmt --check 干净。

[JensenChen28]

Standards

Request changes at c6084478a9ae7e02f84f14eba365cddeff057bae against 11cba403. The clone rehearsal correctly fixes the previous annotation-free recursive-collapse example. One P2 remains in the same normalization root cause.

[P2] Ignore annotation-only differences when guarding cleanup-induced oneOf collapse (crates/tui/src/tools/schema_sanitize.rs:114,144). Take the valid nested schema from my previous review and add "title":"A alternative" to the first outer branch and "title":"B alternative" to the second. Both immediate required checks still pass. Rehearsal removes the nested a/b requirements and injects empty inner properties, but keeps the different titles. Consequently left == right is false and pass-through is admitted, although both resulting branches impose exactly the same validation constraints: an object with an object-valued inner property. Every otherwise valid argument matches both; no argument satisfies exactly one. {"inner":{"a":"x"}} satisfies the original schema and the target's flattening fallback, but fails the head-derived output.

JSON Schema's annotation reference confirms that title/description do not distinguish validation constraints. The unchanged raw-MCP catalog-to-chat path reaches this example without a registry prepass. The registry path also preserves annotations when pruning required groups, so the rehearsal does not establish closure there either.

Spec

Please protect the validation-relevant normalized shape, or conservatively use the compatible fallback for these lossy cases; a general schema solver is not requested. Add annotation-bearing regression coverage through the actual transformation, retaining the self-contained object-union and anyOf controls. Schema-aware treatment matters: a business property named title must remain a property. The new test meaningfully covers the unannotated case, which is now fixed.

Actual validation: cumulative/delta source review along independent Standards and Spec axes, exact-head diff check (exit 0), and a restricted source-transcribed transformation with an independent object/required/exactly-one evaluator (original true, normalized false, fallback true). No Rust/Cargo, complete validator or provider request was run locally; author/peer execution counts are not adopted. This reports a conditional tool-contract regression, with no claim of an observed HTTP 400. Prior body/documentation advisory remains.

[asto18089]

Thank you for the careful variant analysis — both the minProperties:1 witness and the minProperties:0 reading of §6.5.2 are accepted, and the new head addresses all three asks of this review.

[P2] minProperties presence no longer admits a union. empty_matching_object_branch now binds the floor: an object branch with no required and minProperties < 2 counts as empty-matching (0 is equivalent to omitting the keyword per draft 2020-12 §6.5.2, and a floor of 1 still admits {}), so both of your variants fall out of pass-through and take the flattening fallback, keeping {"a":"x"} valid through the shared root properties. The regression xai_sanitize_registry_path_flattens_minproperties_floored_cross_required_union starts from your original valid schema, walks the exact registry sanitize/canonicalize rehearsal, and asserts the union never reaches the wire; re-verified red under the previous presence-only predicate, green under the floor binding.

Author/trailer identity. Both mismatches are corrected by chain rewrite: 3a3378357 aligns the author name with its existing trailer, b6bf68707 re-signs the trailer to match its author; every commit in the branch now has author == Signed-off-by: == asto <asto18089@126.com> byte-exact (the correction path — no maintainer disposition needed).

PR body. The guard description (Changes, Testing, residual edge 2) is updated to the semantic-admission state: R1/R2/R3 with the <2 floor binding; the residual paragraph now discloses only the mixed-group dangling-prune note loss plus the MCP verbatim-clone route.

New head: d13b51b23 (force-pushed rewrite; trees and the cumulative patch are identical to the previously reviewed content except the two identity fixes and the new floor commit). Local evidence: schema_sanitize 89/89 (including the red-green check above), xai wire 51/51, cargo fmt --check clean. CI has re-triggered on the new head.

[JensenChen28]

Standards

Request changes at d13b51b2367363366ade40382d076f79d696ec51 against 11cba403dc6bd6ca5c4dca55042785b03449ac8e. The earlier minProperties 0/1 examples are fixed, and all ten current branch commits now have matching author and Signed-off-by identities. The same registered-tool P2 remains at the new threshold.

[P2] A floor of two does not restore the erased branch distinction (crates/tui/src/tools/schema_sanitize.rs:344, floor < 2). Use the same valid original cross-required schema, with minProperties 2 on both branches:

{"type":"object","properties":{"a":{"type":"string"},"b":{"type":"string"}},"oneOf":[{"type":"object","properties":{"a":{"type":"string"}},"required":["b"],"minProperties":2},{"type":"object","properties":{"b":{"type":"string"}},"required":["a"],"minProperties":2}]}

{"a":"x","extra":"y"} originally matches exactly the second branch; additional properties are allowed. Actual Registry sanitize/canonicalize removes the cross-required groups. The optional-property maps still differ (R1 does not fire), floor 2 excludes both from R2, and there is no nested union for R3. The union therefore stays on the wire.

Every object satisfying the shared root constraints with at least two fields matches both resulting alternatives; objects with fewer fields match neither. No legal argument remains. The target fallback retains the root fields and accepts the original witness. Raising the threshold to cover a fixed property count does not repair this preprocessing regression. Preserve the compatible fallback for the affected required-free shape, or otherwise establish that the original operation contract survives. A general schema solver is not requested.

Spec

The known duplicate/annotation/nested examples and the minProperties 0/1 subcases remain credited as fixed; confirmed annotation-position P3 cases stay closed. Overall semantic usability is still partial for the same root cause. Add a regression from the original valid schema through the actual registration order, with enough permitted extra properties to satisfy the floor. The new test uses canonicalize then sanitize whereas production uses sanitize then canonicalize; those orders are equivalent for its present fixture, but the test should mirror the stated production sequence.

Please correct the explanation that minProperties 1 admits the empty object: it excludes it. Its failure here is that shared-root-valid nonempty objects match both alternatives, not that the empty object satisfies that floor.

Actual verification: full current cumulative/delta source, new test bodies and producer/consumer chain, ten author/trailer identities, diff check (exit 0), and independently derived restricted normalization with object/string/required/minProperties/exactly-one semantics. Witness original valid/head invalid/target fallback valid, with an all-instance argument for the normalized shape. No Rust/Cargo, complete validator, native or provider execution; external counts are not adopted and no observed HTTP400 is claimed. This is a source blocker, independent of CI completion.

[asto18089]

Accepted — the floor-2 witness closes the threshold family, and the fix removes the threshold idea entirely. New head e6423afe1.

[P2] R2 no longer consults minProperties at all. A required-free object branch is now non-discriminating on its own: the required group was the branch's only cross-branch discriminator, and whatever survives the prune — the optional property map, any floor — admits a common tail of the shared root's instances with every sibling required-free branch while rejecting the rest, so exactly-one holds for no instance. Your floor-2 schema now flattens through the full registry path with {"a":"x","extra":"y"} staying valid, exactly like the floor-0/1 variants before it.

Two findings from implementing it, both disclosed plainly:

1. The original-comparison detour was tried and abandoned. I first implemented a gutted-branch trigger comparing pre-rehearsal and projected trees (declared-required-then-erased), but it cannot work at this layer: sanitize_for_xai_parameters runs after the registry's own sanitize/canonicalize (registry.rs:278-279), so the cross-required groups are already gone from the only "original" it can see — the floor-1 variant passed through under that shape. Whether a group was never declared or erased upstream is indistinguishable here, which is why the rule is now simply "required-free pair → flattening fallback". The cost is bounded over-flattening of never-required floored pairs (a sound minProperties:1 union over disjoint property maps now also flattens) — fail-closed, compatible fallback retained, disclosed rather than solved with a general solver.

2. The floor-1 wording you flagged was wrong and is corrected (PR body and in-code docs): minProperties: 1 excludes {}; its failure mode is the shared-root-valid non-empty overlap, not empty-object admission.

The regression now walks the exact production order (sanitize THEN canonicalize_schema, mirroring registry.rs:278-279 — the previous test's order was canonicalize-then-sanitize) for floors 1 and 2 from the original valid schemas. Mutation red-checks: the test fails under both the presence-only exemption and the floor-2 exemption, green under the required-free rule. Local totals: schema_sanitize 89/89 (floor unit pins 0/1/2 all non-discriminating), xai wire 52/52, cargo fmt --check clean. CI re-triggered on e6423afe1.

[JensenChen28]

Approve e6423afe1a1f5f001ffbf89e41f390985fd22e55, independently checked out against fetched maintenance 11cba403dc6bd6ca5c4dca55042785b03449ac8e.

Standards

The known registered-tool P2 is closed. R2 now sends the affected required-free branch pair through the compatible flattening fallback without a minProperties exemption. Current regression tests use the actual sanitize-then-canonicalize production order, and all eleven current author/trailer identities match exactly.

Two nonblocking P3 accuracy boundaries remain: the conservative R2 policy is broader than a universal proof that every required-free union is unusable (different floors or disjoint optional constraints can allow exclusive instances), and a raw MCP clone can still reach the chat-level xAI helper, so its body description should not promise unconditional unchanged wire bytes. Qualify those explanations without requiring a general schema solver.

Spec

Independent restricted controls confirm the original, current and target fallback retain the valid floor-0/1/2 witnesses, including the floor-2 extra-property case. The old threshold control still rejects that witness, which distinguishes the repair. Earlier flat/nested/annotation-position closures remain credited. No additional blocking regression was established in the cumulative two-file change.

Validation: cumulative diff check, source producer/consumer and regression-order inspection, restricted semantic controls and fresh DCO inventory. No local Cargo, complete validator, native/provider execution or author test-count adoption. Current-head CI was still running when first observed; required checks and parent-consumption fork registration remain unwaived. No merge or queue operation.

Standards: P2 closed, bounded P3s. Spec: confirmed unusable-schema cases closed.

[asto18089]

Thank you — the P2 closure is confirmed on record. The two accuracy qualifications are accepted as stated, with the corrected wording recorded here so the next revision lands it without churning an approved head:

1. R2 scope qualification: the required-free rule is a conservative admission policy, not a universal unusability proof — unions whose branches carry different floors or disjoint optional-only constraints can still hold exclusive instances for some arguments, and R2 flattens them anyway. The flattening fallback keeps those schemas usable; what is given up is branch-level expressiveness, and the in-code doc comment will say exactly that rather than implying a proof.

2. MCP route qualification: McpPool::to_api_tools clones input_schema verbatim at pool level, but a raw clone can still reach the chat-level xAI helper downstream, so the PR body's "nothing changes the shipped bytes" for that route must not read as unconditional — the body wording will be narrowed to the pool-level clone behavior.

Both ride the next revision on this branch. Required checks and the parent-consumption fork registration remain unwaived.

[JensenChen28]

Standards

Request changes at e6423afe1a1f5f001ffbf89e41f390985fd22e55 against 11cba403dc6bd6ca5c4dca55042785b03449ac8e. Removing the floor exemption fixes the confirmed minProperties family. However, approval 5811560941 overstates overall closure: an independent registered-input variant of the same P2 still loses all legal arguments.

[P2] Retaining a common required field still bypasses R2 (crates/tui/src/tools/schema_sanitize.rs:352–358, required-free predicate). Start with:

{"type":"object","properties":{"a":{"type":"string"},"b":{"type":"string"},"c":{"type":"string"}},"required":["c"],"oneOf":[{"type":"object","properties":{"a":{"type":"string"},"c":{"type":"string"}},"required":["b","c"]},{"type":"object","properties":{"b":{"type":"string"},"c":{"type":"string"}},"required":["a","c"]}]}

{"a":"x","c":"z"} originally matches exactly the second branch. In the actual registry sanitize/canonicalize sequence, the cross-required b/a entries are removed but the locally declared common c remains. Both branches therefore retain required:["c"], so neither counts toward the required-free R2 pair. Their optional-property maps differ (R1 false), there is no inner union (R3 false), and the later keep-required check sees only the already-retained c and passes.

Every root-valid object contains c and satisfies both resulting alternatives: shared root string constraints already cover the branch property constraints. No argument can satisfy exactly one. The target fallback retains shared a/b/c and required c and accepts the original witness. The direct raw-input belt rejects the original cross-required shape correctly; it cannot recover the distinction already erased by registration.

Please protect the actual constraint-loss path, including partially pruned groups with a shared surviving requirement, rather than only the fully required-free case. Preserving preprocessing information or choosing a conservative supported admission domain are possible directions; a general JSON Schema solver is not requested.

Spec

The known floor-0/1/2, earlier nested and annotation-position cases remain fixed and credited. Production regression order is now correct, and all eleven current author/trailer identities match. Overall usable-contract preservation remains partial for the same root cause. Add the original valid shared-required schema through the actual registry path. The previously acknowledged conservative-policy/MCP wording P3s remain separate and nonblocking.

Actual validation: independent Standards and Spec source review, full latest delta/test bodies plus cumulative producer/consumer requalification, cumulative diff check (exit 0), DCO inventory, and a restricted source-transcribed normalizer with independent object/string/required/exactly-one semantics. New witness original valid/head invalid/target fallback valid, with an all-root-valid-instance argument. No local Cargo/Rust, full validator, native or provider execution; author/other-review native counts are not adopted. No observed HTTP400 or deployed-frequency claim. This correction is a source finding, not a CI-only hold.

[asto18089]

Accepted — the shared-surviving-requirement variant is real and fixed on the new head 7d1d255ee.

[P2] R2 now covers equally-constrained surviving groups. The witness is reproduced exactly: registry preprocess (sanitize then canonicalize) drops the cross-branch discriminators (b from branch one, a from branch two — neither declared in the branch's own properties) while the locally declared common c survives on both; the required-free predicate is silent, R1 is silent (the optional maps differ), R3 is silent, and the keep-required belt sees only the retained c. The new R2 arm fires when two object branches carry the same non-empty surviving required group and describe the shared fields identically: every root-valid instance carries the group, each branch's property map admits the sibling's fields as additional, and the identical shared-field constraints accept the same values — so no instance matches exactly one and the union takes the flattening fallback, keeping {"a":"x","c":"z"} valid through the shared root (root required: ["c"] preserved).

One design boundary worth stating plainly, because it decided the implementation shape: the first cut fired on equal groups alone and three existing pass-through pins caught it immediately — const-differentiated branches (config pinned to different consts under a shared required: ["config"]) and nested-subconstraint-differentiated branches (inner required on both, discriminated by each branch's own nested required group) both stayed genuinely discriminating with equal top-level groups. The arm therefore requires the branches' property entries for the shared fields to be deep-equal; diverging signatures (const/enum pins, nested constraints) keep exactly-one meaningful for the instances they differentiate, and comparing subschema equivalence beyond deep-equality would be the general solver neither of us wants. The residual is the mirror of the disclosed one: a shared group whose fields are unconstrained in one branch but constrained in the other stays admitted (conservative pass-through, fail-open on that bounded shape) — recorded here so it is on the record, not hidden.

Regression xai_sanitize_registry_path_flattens_shared_required_cross_branch_union walks the exact production order from your original valid schema and pins the flattening outcome plus the surviving root requirement; a unit pin covers the shared-group reason and the different-groups-untouched control. Mutation red-check: both new tests fail with the arm disabled, green with it. Local totals: schema_sanitize 90/90, xai wire 53/53, cargo fmt --check clean. CI re-triggered on 7d1d255ee. The two wording P3s from the approval round remain recorded for the next revision.

[JensenChen28]

Standards

Request changes at 7d1d255ee16a2ba85f19b8caf82db7cbaab0c2df against 11cba403dc6bd6ca5c4dca55042785b03449ac8e. The new shared-group arm fixes the previous identical-c-schema witness. The disclosed differing-signature case still contains a concrete instance of the same P2.

[P2] Root constraints can make unequal shared-field signatures equivalent (crates/tui/src/tools/schema_sanitize.rs:387, signature grouping). Start from:

{"type":"object","properties":{"a":{"type":"string"},"b":{"type":"string"},"c":{"type":"string"}},"required":["c"],"oneOf":[{"type":"object","properties":{"a":{"type":"string"},"c":{}},"required":["b","c"]},{"type":"object","properties":{"b":{"type":"string"},"c":{"type":"string"}},"required":["a","c"]}]}

{"a":"x","c":"z"} originally matches exactly the second branch. Actual registry sanitize/canonicalize removes b/a from the respective required groups and retains c. Both branches now require c, but the new signatures differ (c:{} versus c:{"type":"string"}), so the shared-group arm does not fire. Neither branch is required-free, their full property maps differ, and there is no nested union; all admission checks pass.

The root already requires c to be a string. Thus every root-valid object satisfies both resulting alternatives, despite the unequal branch-local signatures. No legal argument remains, whereas the target fallback preserves shared a/b/c and required c and accepts the original witness. This is the same actual preprocessing regression; recording it as bounded fail-open does not preserve the previously usable tool contract.

Please protect constraint erasure through the real registration path, including constraints already implied by the root. Retaining sufficient pre-prune information or limiting admission to a conservative supported domain can avoid inferring erased history at this late stage. No general schema-equivalence solver is requested.

Spec

The prior identical shared-c, floor-family, nested and annotation-position repairs remain credited. The new production-order regression meaningfully covers its exact case; it does not cover the root-implied-constraint variant. Genuine const/nested discriminators must remain distinguished. Overall usable-contract preservation remains partial for the same canonical finding; acknowledged wording P3s remain separate.

Actual validation: independent Standards and Spec source review, complete new 169-line delta/test bodies plus cumulative producer/consumer requalification, cumulative diff check (exit 0), and restricted source-transcribed normalization with independent object/string/required/exactly-one semantics. New witness original valid/head invalid/target fallback valid; all root-valid objects match both normalized branches. No local Rust/Cargo, complete validator, native/provider execution or adopted author run counts. No observed HTTP400 or deployed-frequency claim; conclusion is independent of CI completion.

[asto18089]

Accepted — the root-implied equivalence variant is real and fixed on the new head 8220bf4bf.

[P2] Shared-field signatures now normalize against the containing root. The witness is reproduced exactly: the registry preprocess leaves branch one with c:{} and branch two with c:{{"type":"string"}} — deep-unequal signatures, so the previous arm stayed silent — while the containing root itself requires c and declares it {{"type":"string"}}. The signature builder now treats two kinds of branch-local entries as implied by the root and normalizes them to absent: a vacuous {} (constrains nothing at all), and an entry deep-equal to the root's own schema for that field (adds nothing the root has not already demanded). Both of your branches' entries normalize to absent, the signatures compare equal, and the shared-group arm fires — the union takes the flattening fallback with root required: ["c"] preserved and {{"a":"x","c":"z"}} valid.

The boundary moves as disclosed: this decidable normalization covers vacuous and root-identical entries; a branch entry that is strictly weaker than the root's schema but neither vacuous nor identical (e.g. root {type:string, minLength:2} versus branch {type:string}) still keeps its verbatim shape and stays admitted — deciding general subsumption is the solver neither of us wants. Recorded here as the remaining bounded fail-open remainder, mirroring last round's disclosure.

Regression xai_sanitize_registry_path_flattens_root_implied_shared_required_union walks the exact production order from your original valid schema; a unit pin covers the root-implied equivalence inside the semantic-defect matrix. Mutation red-check: both new tests fail with the normalization disabled, green with it. Local totals: schema_sanitize 91/91, xai wire 54/54, cargo fmt --check clean. CI re-triggered on 8220bf4bf. The two wording P3s remain recorded for the next revision.

[JensenChen28]

Standards

Request changes at 8220bf4bfe06c7bd64c976fa2c2bcc65182224d2 against 11cba403dc6bd6ca5c4dca55042785b03449ac8e. The previous vacuous/root-identical case is fixed. The disclosed strictly-weaker branch case still includes a concrete originally-valid tool regression of the same P2.

[P2] Preserve the contract when the root is stronger than both branch constraints (crates/tui/src/tools/schema_sanitize.rs:399–410, vacuous/root-identical projection):

{"type":"object","properties":{"a":{"type":"string"},"b":{"type":"string"},"c":{"type":"string","minLength":2}},"required":["c"],"oneOf":[{"type":"object","properties":{"a":{"type":"string"},"c":{}},"required":["b","c"]},{"type":"object","properties":{"b":{"type":"string"},"c":{"type":"string"}},"required":["a","c"]}]}

{"a":"x","c":"zz"} originally matches exactly the second branch. Registry sanitize/canonicalize removes the cross-required b/a entries, leaving required c in both. The new projection removes the first branch's vacuous c but retains the second branch's string c because it differs from the stronger root schema. Signatures remain unequal; R1, both R2 arms and R3 all miss, so the unusable union stays on the wire.

Every root-valid object already has a c string of length at least two, satisfying both local c constraints and both projected alternatives. No argument can satisfy exactly one. The target fallback retains root c/minLength/required and accepts the original witness. This uses ordinary valid keywords and the actual registered-tool path. Disclosing the fail-open remainder does not remove the new loss of a previously usable call.

Please address constraint erasure through a conservative admission domain or preserve sufficient pre-prune information across registration. This is a request to protect a demonstrated transformation contract, not to implement arbitrary JSON Schema subsumption. The root-minLength example illustrates why increasingly narrow equality exceptions alone do not establish usability.

Spec

Prior exact-root/shared-group, floor, nested and annotation-position repairs remain credited. The new regression accurately covers its original exact-root example in production order; broader usable-contract preservation remains partial under this same finding. Genuine branch discriminators should remain intact, and prior wording P3s remain separate.

Actual validation: independent Standards and Spec review of the complete latest 119+/13− delta/test bodies and cumulative producer/consumer chain, cumulative diff check (exit 0), and a restricted source-transcribed normalizer with independent object/string/minLength/required/exactly-one semantics. Witness original valid/head invalid/target fallback valid, with an all-root-valid-object argument. No local Cargo/Rust, complete validator, native/provider execution or external test-count adoption. No observed HTTP400/deployed-frequency claim; the finding is independent of CI completion.

[zhuowp]

Request changes at 8220bf4bfe06c7bd64c976fa2c2bcc65182224d2 against 11cba403dc6bd6ca5c4dca55042785b03449ac8e. I reviewed the complete two-file change and the registry-to-xAI request path.

• [P2] Account for the enclosing constraints when admitting a normalized oneOf. In schema_sanitize.rs:298–415, the guard can retain branches whose surviving requirements fall into different groups even though the root guarantees both branches. After registry preprocessing, every root-valid argument matches both alternatives. The outgoing schema admits no argument for the affected tool, while the target's flattening path accepts a previously valid argument. Please preserve a usable contract for this case and cover it in production preprocessing order.
• [P2] Visit each branch once during semantic inspection. The recursive check in schema_sanitize.rs:246–278 and 418–425 repeats work on healthy branches. This work runs synchronously during xAI request preparation and can stall a tool-bearing request before it is sent. Use a single traversal and verify that the work stays bounded for supported schemas.

The fixed Linux CI tree recorded 15,030 passing tests and 15 skips, including passes for all 29 affected test bodies. The blockers above follow from the source and its real callers. Provider behavior and elapsed time for the affected requests remain unmeasured.

[JensenChen28]

补充固定 head 8220bf4bfe06c7bd64c976fa2c2bcc65182224d2 / base 11cba403dc6bd6ca5c4dca55042785b03449ac8e 的实质问题。已有 CHANGES_REQUESTED 5812488975 保持；本条记录额外的性能 P2，避免重复提交同一不可用-schema 审核。

Standards

[P2] 语义检查应只遍历每个子树一次。 schema_sanitize.rs:246–278 调用 oneof_node_defect，其 R3 循环在 418–425 递归检查各分支。健康分支返回无缺陷后，外层又通过普通 oneOf 数组遍历进入同一批分支；每增加一层健康 union，后代子树就重复一倍，形成线性大小输入上的指数工作。

具体健康结构：每层两个 object 分支均要求自己声明的 kind，分别用 const A/B 区分；仅 A 有一个可选 child 属性承载下一层。两支签名不同，R1/R2 不提前拒绝，所有内层 union 可满足，R3 不提前返回。此形状不需要无效 schema、引用或不可满足分支，现有注册/清理及原 MCP 通路可将它送到检查器。

独立的源码转写计数模型中，4/8/12 层分别约 769/1505/2241 字节，对应 377/6377/102377 次 walker 调用。12 层 JSON 值深度为 50；这些是受限控制流模型的操作计数，不是 Rust 执行次数实测或请求耗时。递归调用关系直接说明更深输入的倍增。

Spec

该 helper 在 xAI build_chat_wire_body 的每工具参数准备中同步运行，并由 prepare_outbound_request 在网络发送前调用；因此这项重复工作落在真实请求准备路径。建议单次子树遍历并向上复用判定，保留 schema-position 与 fallback 行为，并加入操作次数/深链覆盖以验证增长界限。此前“重复遍历”建议在有具体小型合法输入和复杂度证据后，升级为独立 P2。

原有不可用-schema P2 也未关闭。同行指出的不同 surviving required 组、但根保证两组都存在的形状，已用原先合法且带 cross-required 区分的输入独立复核：原参数有效、head 无效、target fallback 有效；属于同一约束丢失根因，不另开第三项问题。

实际验证：固定源码、实际调用链、diff check、独立 Standards/Spec 复核及标注清楚的受限遍历/语义模型。未运行 Cargo/Rust、真实 MCP/provider 请求、原生性能实验，未测量卡顿持续时间，也未采用同行 CI 计数。没有已观察到的冻结、HTTP400 或普遍受影响声明。

[asto18089]

Addressed the three open findings at head b7ab69b6e (one commit on top of the reviewed 8220bf4bf; the review-level findings decompose into two root causes, which the two reviewers had already cross-confirmed as shared).

1. Constraint erasure: root stronger than both branches / root requires both groups (reviews 5812488975, 5814012140 P2)

Both witnesses reproduced red on 8220bf4bf exactly as demonstrated:

• xai_sanitize_registry_path_flattens_root_stronger_shared_required_union — the minLength: 2 root dominating one branch's {} and the other's {"type":"string"}; unequal signatures let the equal-group arm spare the union; production order (sanitize THEN canonicalize, registry.rs:278-279); original witness {"a":"x","c":"zz"} stays valid after the fallback.
• xai_sanitize_flattens_union_whose_groups_the_root_already_requires — groups [a]/[b] under root required: ["a","b"]; every arm stayed silent and the union shipped; the flatten fallback keeps the "Exactly one of these parameter groups: a | b" note and the original witness valid.

Fix — a conservative pairwise admission arm (unions_lack_pairwise_discrimination), not another equality exception: an all-object union ships only when EVERY ordered branch pair stays discriminable over the root-valid domain by one of three bounded proofs, and the first unproven pair takes the flattening fallback:

1. group discrimination — the right branch requires a name the left branch does not and the enclosing required does not either, so a root-valid instance may omit it (this arm's enclosing check is what kills the root-requires-both-groups form; mutation-verified: dropping just that check turns the witness red again);
2. pin discrimination — disjoint const/enum pins on a shared field with a candidate the enclosing entry admits (this keeps the reviewed const/forkguard/chat-wire pass-throughs shipping; mutation-verified: disabling the arm turns xai_sanitize_passes_const_differentiated_union_through_without_touching_consts red);
3. nested group discrimination (depth-bounded) — a shared field's entries are object schemas whose own surviving groups differ by a name neither entry requires nor the enclosing entry requires (keeps the reviewed self-contained nested-discriminator control shipping; mutation-verified red when disabled).

The fail-closed default is deliberate per the review guidance: the guard no longer tries to enumerate signature equivalences (which produced the round-by-round narrowing); anything the arms cannot prove flattens, and the flattening fallback keeps the shared root properties usable and re-derives the parameter-groups note.

2. Exponential re-traversal (reviews 5814012140 P2-b, 5814046611)

Confirmed by inspection: the R3 recursion and the outer keyword walk visited the same branch subtrees independently, doubling work per healthy nesting level. The walk now memoizes per node (stable addresses on an unmutated serde_json tree; single-parent nodes keep cached reasons path-exact, so a cached reason is path-identical to what a fresh visit would produce). xai_union_defect_walk_computes_each_subtree_once builds a 10-level deep healthy pinned-branch union, requires the walk to report no defect, and asserts the computation count stays at the document's node count — red when the memo is removed (reproducing the reported doubling). The judge remains synchronous per request; the work is now linear in schema size.

Also tightened while closing #1

The arms' witnesses are omission witnesses, and they are only accepted as proven when both shapes carry structural keys and every carried field's entry is value-permissive; a restrictive additionalProperties on the left branch or a minLength on a carried field leaves the pair unproven → flatten (xai_sanitize_flattens_union_whose_witness_fields_carry_further_constraints, red when the gate is disabled). This closes the residual fail-open the reviews had not yet reached rather than disclosing it.

Verification

• Local at b7ab69b6e: 58/58 xai_-filtered lib tests, 95/95 schema_sanitize, cargo fmt --all -- --check clean, clippy clean for this PR's files (the 8 local lints are the pre-existing stable-1.99.1 toolchain drift in unrelated files, as previously disclosed).
• Full local codewhale-tui lib battery at b7ab69b6e: 12,439 passed / 0 failed / 13 ignored (run with --test-threads=1 RUST_MIN_STACK=16M: on this machine the default parallel runner deadlocks in the core::engine forkguard integration tests and three runtime_api::tests integration tests (create_thread_accepts_dynamic_tools_and_environments, create_thread_normalizes_and_persists_named_permission_posture, decide_approval_400s_on_bad_decision) overflow the default 8 MB test-thread stack — all reproduced identically at the reviewed base 11cba403d via a throwaway worktree, so both are local-environment artifacts, pre-existing and untouched by this PR; CI's nextest was green through 15,030 tests at the prior head).
• Full mutation ledger re-run on the final commit (six targeted mutations, each turning its pin red; restored state green).
• PR body updated (head, changes, red-green evidence incl. the mutation ledger, narrowed-behavior disclosure, bounded-edges section).

Re-requesting review from both.

[JensenChen28]

Reviewed the latest head dbf757316320720bad2130bbf0a8d61a5f8078e4 against base 11cba403dc6bd6ca5c4dca55042785b03449ac8e. I found one P2 issue in the bounded witness proof; see the inline comment. git diff --check passed. I could not run Rust tests locally because Cargo is unavailable; the review worktree created for this cycle also appeared with unrelated modifications, so I preserved it and did not use it for validation.

[JensenChen28]

[P2] Reject contradictory pin/type combinations before treating this field as witnessable. This helper accepts any combination of type, const, and enum based only on keyword names, so e.g. {"type":"string","const":42} is deemed to admit a concrete value even though no instance satisfies it. With an object root oneOf whose branches respectively require a and b, and whose a/b entries each use that contradictory schema, the pairwise omission proof succeeds for both directions and the union passes through with no valid branch. This breaks the stated semantic-usability guarantee and can ship an unusable tool argument contract. Check type/pin compatibility (and enum non-emptiness/intersection as applicable) when proving the witness, or fail closed to the flattening path; add a regression case.

[JensenChen28]

Additional finding on the current head; see the inline P2 below for a documented object-union case that the current guard flattens and narrows.

[JensenChen28]

[P2] Preserve property discriminators on required-free branches. This early return counts every pair of object branches without required as a dead union before the later const/enum pin proof can run. For oneOf: [{type:"object",properties:{kind:{const:"email"}}},{type:"object",properties:{kind:{const:"sms"}}}], each discriminator value matches exactly one branch, so the documented object union has usable arguments. The fallback then merges root properties from the first branch and removes oneOf, rejecting the sms variant. Please account for disjoint property constraints before taking this fallback and cover both variants.

[JensenChen28]

A further correctness issue surfaced during follow-up of the newly submitted review. The existing CHANGES_REQUESTED remains active; this comment records a distinct narrowing case in the required-free branch rule.

[JensenChen28]

[P2] Do not classify every pair of required-free branches as non-discriminating. A valid documented object union can discriminate through property pins without required, e.g. oneOf: [{type:"object",properties:{kind:{const:"email"}}},{type:"object",properties:{kind:{const:"sms"}}}]. Each input matches exactly one branch, but this early count sends the union to flattening before the later pairwise const/enum proof can run. merge_root_composition_properties keeps the first kind schema (or_insert), then removes oneOf, so the resulting root rejects the sms variant. Preserve such pairs when their property constraints prove disjoint, or otherwise make fallback retain all alternatives; add regression coverage for both variants.

[zhuowp]

Request changes for dbf757316320720bad2130bbf0a8d61a5f8078e4 against 11cba403. I reviewed all seven cumulative hunks, the current repair, and the registry-to-xAI request path.

[P2] Pairwise witnesses do not establish a usable multi-branch oneOf. A valid registered tool can lose its cross-field discriminators during preprocessing and reach this check with overlapping enum branches. Each ordered pair passes, yet every remaining value matches two branches, leaving the tool with no schema-valid arguments. Please require a common witness that excludes all other branches, or use the existing fallback when that cannot be established. The inline comment gives the finite counterexample and the production preprocessing boundary.

The earlier root-constraint cases now fall back, and memoization fixes the previously reported repeated subtree computations. Conservative fallback can still discard a valid later variant when its proof is unsupported. That remains a nonblocking compatibility limit relative to the target's existing fallback. Some new test assertions also label an originally rejected object as an original valid witness, so they establish fallback usability without establishing preservation of that witness.

The exact CI merge tree has 13 successful checks and one skip. All 37 reviewed test bodies have named PASS records within the 15,034-pass, 15-skip workspace run. The multi-branch counterexample follows from source and a finite value-domain analysis; it was not executed against xAI. The fixed combination is clean, while the confirmed source issue remains unresolved.

[zhuowp]

[P2] The witnesses chosen for different pairs need not be the same value. Consider registered branches whose kind enums are {a,b}, {b,c}, and {a,c}. Before registry preprocessing, each branch also requires its own x/y/z field declared in the root, so {kind:a,x:"x"} matches only the first branch. Registry sanitize removes those branch-local cross-field requirements. At this point all branches require only kind, each ordered pair passes the pin arm, and the walker retains oneOf. But a matches branches 0/2, b matches 0/1, and c matches 1/2; all other values match none. The tool has no valid arguments, whereas the target's flattening path accepts the original witness. Please prove a witness against all other branches together, or fall back for this overlapping multi-branch case, and cover the actual sanitize→canonicalize→wire order.

[JensenChen28]

Confirmed. The guard checks ordered pairs independently: each pair may find a different left-side enum value outside only that one right branch. With the three sets {a,b}, {b,c}, and {a,c}, every pair passes while each value matches two branches, so oneOf has no accepted value after the cross-branch required entries are pruned. This is a distinct multi-branch gap from my two comments on this head; I have not run Rust tests locally.