diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index f9ce20e37..bd3c9e2e4 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -35,7 +35,7 @@ - Stopped runaway exact response cycles across model providers and kept persistent loops fail-closed after bounded guarded retries ([#8669](https://github.com/can1357/oh-my-pi/pull/8669) by [@pstarkgit](https://github.com/pstarkgit)). ### Fixed -- Fixed OpenAI Responses / Completions and xAI tool conversion 400ing the whole turn on MCP schemas whose root is an object plus a typeless exclusive-required `anyOf` (e.g. codebase-memory `check_index_coverage`). Flatten only the **tool root** — nested unions (e.g. `task.outputSchema`) stay intact so Grok still sees valid `task`/`edit` schemas. +- Fixed xAI 400ing the whole turn on MCP schemas whose tool root is an object plus a typeless exclusive-required `anyOf` (e.g. codebase-memory `check_index_coverage`). Flatten only that root fragment — nested unions and branch property/`additionalProperties` constraints stay intact. Leftover object-root unions quarantine that one tool on paid xAI Completions and xAI OAuth Responses, not on OpenAI/Azure/Codex. ## [17.3.4] - 2026-08-14 diff --git a/packages/ai/src/providers/openai-completions.ts b/packages/ai/src/providers/openai-completions.ts index 4375b0bce..78c3f39ca 100644 --- a/packages/ai/src/providers/openai-completions.ts +++ b/packages/ai/src/providers/openai-completions.ts @@ -3,7 +3,7 @@ import { isKimiModelId } from "@oh-my-pi/pi-catalog/identity"; import { resolveWireModelId } from "@oh-my-pi/pi-catalog/model-thinking"; import { calculateCost } from "@oh-my-pi/pi-catalog/models"; import type { ResolvedOpenAICompat } from "@oh-my-pi/pi-catalog/types"; -import { $env, parseStreamingJson, parseStreamingJsonThrottled } from "@oh-my-pi/pi-utils"; +import { $env, logger, parseStreamingJson, parseStreamingJsonThrottled } from "@oh-my-pi/pi-utils"; import { renderDemotedThinking } from "../dialect/demotion"; import * as AIError from "../error"; import { getKimiCommonHeaders } from "../registry/oauth/kimi"; @@ -44,6 +44,7 @@ import { notifyProviderResponse } from "../utils/provider-response"; import { callWithCopilotModelRetry } from "../utils/retry"; import { adaptSchemaForStrict, + findStrictToolSchemaViolation, NO_STRICT, normalizeSchemaForMoonshot, sanitizeSchemaForGrammar, @@ -1604,7 +1605,7 @@ function buildParams( applyOpenAIServiceTier(params, options?.serviceTier, model); if (context.tools?.length) { - const builtTools = convertTools(context.tools, initialCompat, toolStrictModeOverride); + const builtTools = convertTools(context.tools, initialCompat, toolStrictModeOverride, model.provider); params.tools = builtTools.tools; toolStrictMode = builtTools.toolStrictMode; strictToolsApplied = builtTools.strictToolsApplied; @@ -1665,7 +1666,10 @@ function buildParams( params.tool_choice = "auto"; } - if (params.tool_choice === "none" && (!Array.isArray(params.tools) || params.tools.length === 0)) { + if ( + (!Array.isArray(params.tools) || params.tools.length === 0) && + (params.tool_choice === "none" || isForcedToolChoice(params.tool_choice)) + ) { // `tool_choice: "none"` with no tools to gate is redundant and also // trips LiteLLM → Bedrock: the proxy serializes the directive into a // `toolConfig` block, and Bedrock requires `toolConfig.tools` to be @@ -1674,6 +1678,8 @@ function buildParams( // Side-channel turns hit this: `/btw` and IRC background replies route // through `AgentSession.runEphemeralTurn`, which sets `context.tools = []` // and `toolChoice: "none"` (see packages/coding-agent/src/session/agent-session.ts). + // The same empty-tools case applies after leftover-union quarantine: a + // leftover `"required"` / named force would 400 just like the bad schema. delete params.tool_choice; } @@ -2300,6 +2306,7 @@ function convertTools( tools: Tool[], compat: ResolvedOpenAICompat, toolStrictModeOverride?: ToolStrictModeOverride, + provider?: string, ): BuiltOpenAICompletionTools { const adaptedTools = tools.map(tool => { const strict = !NO_STRICT && compat.supportsStrictMode !== false && tool.strict !== false; @@ -2322,48 +2329,57 @@ function convertTools( ? "all_strict" : "none" : "mixed"; + const rejectXaiRootObjectUnion = provider === "xai" || provider === "xai-oauth"; + + const wireTools: ChatCompletionTool[] = []; + let anyStrictEmitted = false; + for (const { tool, baseParameters, parameters, strict } of adaptedTools) { + const includeStrict = toolStrictMode === "all_strict" || (toolStrictMode === "mixed" && strict); + // `strict: false` is semantically distinct from omitted `strict` on some + // backends: with it absent, optional properties may be over-filled with + // placeholder values (#4336). Preserve the author's explicit `false`, + // but only in "mixed" mode against a provider that understands the + // field — the `all_strict → none` collapse and `supportsStrictMode: + // false` paths deliberately keep the wire flag uniformly absent. + const includeExplicitFalse = + !includeStrict && tool.strict === false && toolStrictMode === "mixed" && compat.supportsStrictMode !== false; + const wireParameters = includeStrict ? parameters : baseParameters; + // Moonshot/Kimi native hosts validate against the stricter MFJS subset + // (const→enum, typed enums, no validators) and 400 otherwise. + // Grammar-constrained local backends (llama.cpp, LM Studio, vLLM) + // build a GBNF grammar from the schema and 400 with + // `Unrecognized schema: true` on the bare boolean subschema + // `toolWireSchema` emits for open fields (issue #5914). + const emittedParameters = + compat.toolSchemaFlavor === "moonshot-mfjs" + ? (normalizeSchemaForMoonshot(wireParameters) as Record) + : compat.toolSchemaFlavor === "grammar" + ? sanitizeSchemaForGrammar(wireParameters) + : wireParameters; + const violation = findStrictToolSchemaViolation(emittedParameters, "#", { rejectXaiRootObjectUnion }); + if (violation) { + logger.warn( + `Tool "${tool.name}" omitted from the openai-completions request: its parameter schema is invalid for this provider at ${violation} (an enum/const value cannot match its declared type, or leftover xAI object-root union). Other tools are unaffected.`, + ); + continue; + } + if (includeStrict) anyStrictEmitted = true; + wireTools.push({ + type: "function", + function: { + name: tool.name, + description: tool.description || "", + parameters: emittedParameters, + // Only include strict if provider supports it. Some reject unknown fields. + ...(includeStrict ? { strict: true } : includeExplicitFalse ? { strict: false } : {}), + }, + }); + } return { - tools: adaptedTools.map(({ tool, baseParameters, parameters, strict }) => { - const includeStrict = toolStrictMode === "all_strict" || (toolStrictMode === "mixed" && strict); - // `strict: false` is semantically distinct from omitted `strict` on some - // backends: with it absent, optional properties may be over-filled with - // placeholder values (#4336). Preserve the author's explicit `false`, - // but only in "mixed" mode against a provider that understands the - // field — the `all_strict → none` collapse and `supportsStrictMode: - // false` paths deliberately keep the wire flag uniformly absent. - const includeExplicitFalse = - !includeStrict && - tool.strict === false && - toolStrictMode === "mixed" && - compat.supportsStrictMode !== false; - const wireParameters = includeStrict ? parameters : baseParameters; - return { - type: "function", - function: { - name: tool.name, - description: tool.description || "", - // Moonshot/Kimi native hosts validate against the stricter MFJS subset - // (const→enum, typed enums, no validators) and 400 otherwise. - // Grammar-constrained local backends (llama.cpp, LM Studio, vLLM) - // build a GBNF grammar from the schema and 400 with - // `Unrecognized schema: true` on the bare boolean subschema - // `toolWireSchema` emits for open fields (issue #5914). - parameters: - compat.toolSchemaFlavor === "moonshot-mfjs" - ? (normalizeSchemaForMoonshot(wireParameters) as Record) - : compat.toolSchemaFlavor === "grammar" - ? sanitizeSchemaForGrammar(wireParameters) - : wireParameters, - // Only include strict if provider supports it. Some reject unknown fields. - ...(includeStrict ? { strict: true } : includeExplicitFalse ? { strict: false } : {}), - }, - }; - }), + tools: wireTools, toolStrictMode, - strictToolsApplied: - tools.length > 0 && - (toolStrictMode === "all_strict" || (toolStrictMode === "mixed" && adaptedTools.some(tool => tool.strict))), + strictToolsApplied: wireTools.length > 0 && anyStrictEmitted, }; } diff --git a/packages/ai/src/providers/openai-responses.ts b/packages/ai/src/providers/openai-responses.ts index 61055b2ab..d84107eed 100644 --- a/packages/ai/src/providers/openai-responses.ts +++ b/packages/ai/src/providers/openai-responses.ts @@ -1427,8 +1427,11 @@ export function convertTools( // Quarantine a tool whose emitted schema carries a provider-rejecting // enum/const-vs-type contradiction: dropping just that tool keeps the rest // of the request valid instead of letting one bad MCP schema 400 the whole - // turn (#2652). Other tools and built-ins are unaffected. - const violation = findStrictToolSchemaViolation(parameters); + // turn (#2652). Other tools and built-ins are unaffected. Leftover + // object-root unions are an xAI-only 400; OpenAI/Azure/Codex keep them. + const violation = findStrictToolSchemaViolation(parameters, "#", { + rejectXaiRootObjectUnion: model.provider === "xai" || model.provider === "xai-oauth", + }); if (violation) { onQuarantine(tool.name, violation); continue; diff --git a/packages/ai/src/utils/schema/normalize.ts b/packages/ai/src/utils/schema/normalize.ts index 9cc224397..794ddad41 100644 --- a/packages/ai/src/utils/schema/normalize.ts +++ b/packages/ai/src/utils/schema/normalize.ts @@ -1579,7 +1579,6 @@ function normalizeOpenAIResponsesSchemaNode(value: unknown, cache: WeakMap typeof name === "string" && name.length > 0)) return false; for (const key in branch) { if (!Object.hasOwn(branch, key)) continue; - if (key === "required" || key === "properties" || key === "additionalProperties" || key === "description" || key === "title") { - continue; - } + if (key === "required" || key === "description" || key === "title") continue; return false; } - return ( - Object.hasOwn(branch, "required") || - Object.hasOwn(branch, "properties") || - Object.hasOwn(branch, "additionalProperties") - ); + return true; } function flattenOpenAIResponsesObjectConstraintUnion(output: JsonObject): boolean { @@ -1622,19 +1616,7 @@ function flattenOpenAIResponsesObjectConstraintUnion(output: JsonObject): boolea const union = output[unionKey]; if (!Array.isArray(union) || union.length === 0) return false; if (!declaresObjectType(output.type) && !isJsonObject(output.properties)) return false; - if (!union.every(isTypelessObjectConstraintBranch)) return false; - - const properties = isJsonObject(output.properties) ? output.properties : {}; - for (const branch of union) { - if (!isJsonObject(branch) || !isJsonObject(branch.properties)) continue; - for (const name in branch.properties) { - if (!Object.hasOwn(branch.properties, name)) continue; - if (!Object.hasOwn(properties, name)) properties[name] = branch.properties[name]; - } - } - if (!isJsonObject(output.properties) && Object.keys(properties).length > 0) { - output.properties = properties; - } + if (!union.every(isExclusiveRequiredBranch)) return false; delete output[unionKey]; return true; } diff --git a/packages/ai/src/utils/schema/strict-tool-validation.ts b/packages/ai/src/utils/schema/strict-tool-validation.ts index d20a50469..77fa50287 100644 --- a/packages/ai/src/utils/schema/strict-tool-validation.ts +++ b/packages/ai/src/utils/schema/strict-tool-validation.ts @@ -2,14 +2,23 @@ * Detects tool-parameter schemas that pass structural JSON-Schema validation * (so {@link isValidJsonSchema} accepts them) yet make OpenAI-style providers * reject the whole request with HTTP 400 — namely an `enum`/`const` whose - * value(s) cannot satisfy the node's declared `type`, or a root `anyOf`/`oneOf` - * whose branches are not objects (xAI: "tool parameter root must be an object - * type"). MCP servers emit these when a nullable/array branch is built - * incorrectly, or when exclusive-required is encoded as a typeless union. - * One such tool 400s the entire turn, so callers quarantine just the offending - * tool. See issue #2652. + * value(s) cannot satisfy the node's declared `type`. MCP servers emit these + * when a nullable/array branch is built incorrectly (e.g. a non-null `enum` + * copied onto a `type: "null"` branch, or an `enum` placed on an `array` + * schema instead of its `items`). One such tool 400s the entire turn, so + * callers quarantine just the offending tool. See issue #2652. + * + * xAI additionally rejects a leftover *root* `anyOf`/`oneOf` whose branches + * are not objects ("tool parameter root must be an object type"). That class + * is opt-in via {@link FindStrictToolSchemaViolationOptions.rejectXaiRootObjectUnion} + * so OpenAI/Azure/Codex keep valid object-root unions. */ +export interface FindStrictToolSchemaViolationOptions { + /** xAI (paid + OAuth) only: leftover object-root unions 400 the whole turn. */ + rejectXaiRootObjectUnion?: boolean; +} + type JsonRecord = Record; const SCHEMA_TYPE_NAMES: Record = { @@ -71,10 +80,14 @@ const CHILD_ARRAY_KEYS = ["anyOf", "oneOf", "allOf", "prefixItems"] as const; * contradictions. Returns a JSON-pointer-ish path to the first offending node, * or `null` when the schema is safe to emit. */ -export function findStrictToolSchemaViolation(schema: unknown, path = "#"): string | null { +export function findStrictToolSchemaViolation( + schema: unknown, + path = "#", + options?: FindStrictToolSchemaViolationOptions, +): string | null { if (Array.isArray(schema)) { for (let i = 0; i < schema.length; i++) { - const hit = findStrictToolSchemaViolation(schema[i], `${path}/${i}`); + const hit = findStrictToolSchemaViolation(schema[i], `${path}/${i}`, options); if (hit) return hit; } return null; @@ -95,7 +108,11 @@ export function findStrictToolSchemaViolation(schema: unknown, path = "#"): stri // xAI rejects the whole request when the *root* schema is typed as object // (or has properties) AND still carries an anyOf/oneOf with a typeless or // non-object branch. Nested unions and pure root unions are not this error. - if (path === "#" && (types.includes("object") || (node.properties !== undefined && typeof node.properties === "object"))) { + if ( + options?.rejectXaiRootObjectUnion && + path === "#" && + (types.includes("object") || (node.properties !== undefined && typeof node.properties === "object")) + ) { for (const key of ["anyOf", "oneOf"] as const) { const arr = node[key]; if (!Array.isArray(arr) || arr.length === 0) continue; @@ -108,26 +125,25 @@ export function findStrictToolSchemaViolation(schema: unknown, path = "#"): stri } } - for (const key of CHILD_MAP_KEYS) { const sub = node[key]; if (sub && typeof sub === "object" && !Array.isArray(sub)) { for (const k of Object.keys(sub as JsonRecord)) { - const hit = findStrictToolSchemaViolation((sub as JsonRecord)[k], `${path}/${key}/${k}`); + const hit = findStrictToolSchemaViolation((sub as JsonRecord)[k], `${path}/${key}/${k}`, options); if (hit) return hit; } } } for (const key of CHILD_SCHEMA_KEYS) { if (key in node) { - const hit = findStrictToolSchemaViolation(node[key], `${path}/${key}`); + const hit = findStrictToolSchemaViolation(node[key], `${path}/${key}`, options); if (hit) return hit; } } for (const key of CHILD_ARRAY_KEYS) { const arr = node[key]; if (Array.isArray(arr)) { - const hit = findStrictToolSchemaViolation(arr, `${path}/${key}`); + const hit = findStrictToolSchemaViolation(arr, `${path}/${key}`, options); if (hit) return hit; } } diff --git a/packages/ai/src/utils/schema/wire.ts b/packages/ai/src/utils/schema/wire.ts index b265e9787..f70a80409 100644 --- a/packages/ai/src/utils/schema/wire.ts +++ b/packages/ai/src/utils/schema/wire.ts @@ -207,27 +207,17 @@ function rewriteNullableScalarAnyOf(schema: Record): void { schema.type = [scalarType, "null"]; } -function isTypelessObjectConstraintBranch(branch: unknown): boolean { +function isExclusiveRequiredBranch(branch: unknown): boolean { if (!isSchemaRecord(branch)) return false; if (Object.hasOwn(branch, "type")) return false; + if (!Array.isArray(branch.required) || branch.required.length === 0) return false; + if (!branch.required.every(name => typeof name === "string" && name.length > 0)) return false; for (const key in branch) { if (!Object.hasOwn(branch, key)) continue; - if ( - key === "required" || - key === "properties" || - key === "additionalProperties" || - key === "description" || - key === "title" - ) { - continue; - } + if (key === "required" || key === "description" || key === "title") continue; return false; } - return ( - Object.hasOwn(branch, "required") || - Object.hasOwn(branch, "properties") || - Object.hasOwn(branch, "additionalProperties") - ); + return true; } function flattenExclusiveRequiredUnion(schema: Record): void { @@ -237,23 +227,10 @@ function flattenExclusiveRequiredUnion(schema: Record): void { if (!Array.isArray(union) || union.length === 0) return; const typedObject = schema.type === "object" || (Array.isArray(schema.type) && schema.type.includes("object")); if (!typedObject && !isSchemaRecord(schema.properties)) return; - if (!union.every(isTypelessObjectConstraintBranch)) return; - - const properties = isSchemaRecord(schema.properties) ? schema.properties : {}; - for (const branch of union) { - if (!isSchemaRecord(branch) || !isSchemaRecord(branch.properties)) continue; - for (const name in branch.properties) { - if (!Object.hasOwn(branch.properties, name)) continue; - if (!Object.hasOwn(properties, name)) properties[name] = branch.properties[name]; - } - } - if (!isSchemaRecord(schema.properties) && Object.keys(properties).length > 0) { - schema.properties = properties; - } + if (!union.every(isExclusiveRequiredBranch)) return; delete schema[unionKey]; } - /** Keys whose values are a single JSON Schema (not an array or map). */ const SCHEMA_VALUE_KEYS = [ "additionalProperties", diff --git a/packages/ai/test/openai-completions-xai-root-union.test.ts b/packages/ai/test/openai-completions-xai-root-union.test.ts new file mode 100644 index 000000000..a8fac5f1f --- /dev/null +++ b/packages/ai/test/openai-completions-xai-root-union.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, it } from "bun:test"; +import { type } from "@oh-my-pi/omptype"; +import { streamOpenAICompletions } from "@oh-my-pi/pi-ai/providers/openai-completions"; +import type { Context, Model, ModelSpec, Tool, ToolChoice } from "@oh-my-pi/pi-ai/types"; +import { buildModel } from "@oh-my-pi/pi-catalog/build"; + +interface ChatCompletionsPayload { + tool_choice?: unknown; + tools?: Array<{ type?: string; function?: { name?: string; parameters?: { anyOf?: unknown } } }>; +} + +const coverageTool: Tool = { + name: "mcp__codebase_memory_check_index_coverage", + description: "coverage", + parameters: { + type: "object", + properties: { + project: { type: "string" }, + paths: { type: "array", items: { type: "string" } }, + scopes: { type: "array", items: { type: "string" } }, + }, + required: ["project"], + anyOf: [{ required: ["paths"] }, { required: ["scopes"] }], + } as unknown as Tool["parameters"], +}; + +const leftoverTool: Tool = { + name: "mcp__leftover_union", + description: "union", + parameters: { + type: "object", + properties: { kind: { type: "string" } }, + anyOf: [ + { required: ["kind"], minProperties: 1 }, + { required: ["kind"], minProperties: 2 }, + ], + } as unknown as Tool["parameters"], +}; + +const goodTool: Tool = { + name: "read_file", + description: "read a file", + parameters: type({ path: type("string") }), +}; + +function makeModel(provider: "openai" | "xai"): Model<"openai-completions"> { + return buildModel({ + id: provider === "xai" ? "grok-4" : "gpt-4o-mini", + name: provider === "xai" ? "Grok 4" : "GPT-4o Mini", + api: "openai-completions", + provider, + baseUrl: provider === "xai" ? "https://api.x.ai/v1" : "https://api.openai.com/v1", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128000, + maxTokens: 4096, + } as ModelSpec<"openai-completions">); +} + +function abortedSignal(): AbortSignal { + const controller = new AbortController(); + controller.abort(); + return controller.signal; +} + +function capturePayload( + provider: "openai" | "xai", + tools: Tool[], + toolChoice?: ToolChoice, +): Promise { + const { promise, resolve } = Promise.withResolvers(); + const context: Context = { + messages: [{ role: "user", content: "check coverage", timestamp: 0 }], + tools, + }; + streamOpenAICompletions(makeModel(provider), context, { + apiKey: "test-key", + toolChoice, + signal: abortedSignal(), + onPayload: payload => resolve(payload as ChatCompletionsPayload), + }); + return promise; +} + +function toolNames(payload: ChatCompletionsPayload): Array { + return payload.tools?.map(tool => tool.function?.name) ?? []; +} + +describe("openai-completions xAI leftover-union quarantine", () => { + it("keeps an exclusive-required MCP tool after wire flatten on paid xAI", async () => { + const payload = await capturePayload("xai", [coverageTool, goodTool]); + expect(toolNames(payload)).toEqual(["mcp__codebase_memory_check_index_coverage", "read_file"]); + expect(payload.tools?.[0]?.function?.parameters?.anyOf).toBeUndefined(); + }); + + it("keeps a leftover object-root union on OpenAI Completions", async () => { + const payload = await capturePayload("openai", [leftoverTool, goodTool]); + expect(toolNames(payload)).toEqual(["mcp__leftover_union", "read_file"]); + expect(payload.tools?.[0]?.function?.parameters?.anyOf).toHaveLength(2); + }); + + it("quarantines a leftover object-root union on paid xAI only", async () => { + const payload = await capturePayload("xai", [leftoverTool, goodTool]); + expect(toolNames(payload)).toEqual(["read_file"]); + }); + + it("drops a forced tool_choice when the leftover-union tool was quarantined", async () => { + const payload = await capturePayload("xai", [leftoverTool, goodTool], { + type: "tool", + name: "mcp__leftover_union", + }); + expect(toolNames(payload)).toEqual(["read_file"]); + expect(payload.tool_choice).toBeUndefined(); + }); +}); diff --git a/packages/ai/test/openai-responses-tool-quarantine.test.ts b/packages/ai/test/openai-responses-tool-quarantine.test.ts index dbe72bbd6..4ce90e00f 100644 --- a/packages/ai/test/openai-responses-tool-quarantine.test.ts +++ b/packages/ai/test/openai-responses-tool-quarantine.test.ts @@ -5,13 +5,13 @@ import type { Context, Model, ModelSpec, Tool } from "@oh-my-pi/pi-ai/types"; import { findStrictToolSchemaViolation } from "@oh-my-pi/pi-ai/utils/schema"; import { buildModel } from "@oh-my-pi/pi-catalog/build"; -function makeModel(): Model<"openai-responses"> { +function makeModel(provider: "openai" | "xai-oauth" = "openai"): Model<"openai-responses"> { return buildModel({ - id: "gpt-5", - name: "GPT-5", + id: provider === "xai-oauth" ? "grok-4" : "gpt-5", + name: provider === "xai-oauth" ? "Grok 4" : "GPT-5", api: "openai-responses", - provider: "openai", - baseUrl: "https://api.openai.com/v1", + provider, + baseUrl: provider === "xai-oauth" ? "https://api.x.ai/v1" : "https://api.openai.com/v1", reasoning: true, input: ["text"], cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, @@ -20,6 +20,15 @@ function makeModel(): Model<"openai-responses"> { } as ModelSpec<"openai-responses">); } +const leftoverRootUnion = { + type: "object", + properties: { kind: { type: "string" } }, + anyOf: [ + { required: ["kind"], minProperties: 1 }, + { required: ["kind"], minProperties: 2 }, + ], +} as const; + describe("findStrictToolSchemaViolation (#2652)", () => { test("flags a non-null enum on a null-typed node (nullable-enum shape)", () => { expect(findStrictToolSchemaViolation({ enum: ["A", "B"], type: "null" })).toBe("#/enum"); @@ -55,27 +64,25 @@ describe("findStrictToolSchemaViolation (#2652)", () => { expect(findStrictToolSchemaViolation({ enum: ["x"] })).toBeNull(); }); - test("flags a root anyOf whose branches are not objects (xAI root-union 400)", () => { - expect( - findStrictToolSchemaViolation({ - type: "object", - properties: { project: { type: "string" } }, - anyOf: [{ required: ["paths"] }, { required: ["scopes"] }], - }), - ).toBe("#/anyOf"); + test("flags a leftover xAI root anyOf only when the xAI option is on", () => { + expect(findStrictToolSchemaViolation(leftoverRootUnion)).toBeNull(); + expect(findStrictToolSchemaViolation(leftoverRootUnion, "#", { rejectXaiRootObjectUnion: true })).toBe("#/anyOf"); }); - test("accepts a root anyOf of typed object branches", () => { + test("accepts a root anyOf of typed object branches even for xAI", () => { expect( - findStrictToolSchemaViolation({ - anyOf: [ - { type: "object", properties: { a: { type: "string" } } }, - { type: "object", properties: { b: { type: "number" } } }, - ], - }), + findStrictToolSchemaViolation( + { + anyOf: [ + { type: "object", properties: { a: { type: "string" } } }, + { type: "object", properties: { b: { type: "number" } } }, + ], + }, + "#", + { rejectXaiRootObjectUnion: true }, + ), ).toBeNull(); }); - }); const badTool: Tool = { @@ -136,7 +143,43 @@ describe("convertTools quarantine (#2652)", () => { expect(out[0]?.parameters.anyOf).toBeUndefined(); }); + test("keeps a leftover object-root union on OpenAI Responses", () => { + const leftoverTool: Tool = { + name: "mcp__leftover_union", + description: "union", + parameters: { + type: "object", + properties: { kind: { type: "string" } }, + anyOf: [ + { required: ["kind"], minProperties: 1 }, + { required: ["kind"], minProperties: 2 }, + ], + } as unknown as Tool["parameters"], + }; + const out = convertTools([leftoverTool, goodTool], true, makeModel()) as Array<{ + name: string; + parameters: { anyOf?: unknown }; + }>; + expect(out.map(t => t.name)).toEqual(["mcp__leftover_union", "read_file"]); + expect(out[0]?.parameters.anyOf).toHaveLength(2); + }); + test("quarantines a leftover object-root union on xAI Responses only", () => { + const leftoverTool: Tool = { + name: "mcp__leftover_union", + description: "union", + parameters: { + type: "object", + properties: { kind: { type: "string" } }, + anyOf: [ + { required: ["kind"], minProperties: 1 }, + { required: ["kind"], minProperties: 2 }, + ], + } as unknown as Tool["parameters"], + }; + const out = convertTools([leftoverTool, goodTool], true, makeModel("xai-oauth")) as Array<{ name: string }>; + expect(out.map(t => t.name)).toEqual(["read_file"]); + }); test("reports the hidden tool name and the offending schema path", () => { const dropped: Array<{ name: string; path: string }> = []; convertTools([badTool], true, makeModel(), (name, path) => dropped.push({ name, path })); diff --git a/packages/ai/test/schema-normalization.test.ts b/packages/ai/test/schema-normalization.test.ts index 60a5d79a3..5bb4ec80c 100644 --- a/packages/ai/test/schema-normalization.test.ts +++ b/packages/ai/test/schema-normalization.test.ts @@ -651,7 +651,17 @@ describe("sanitizeSchemaForOpenAIResponses", () => { expect(outputSchema.anyOf).toEqual([{ required: ["paths"] }, { required: ["scopes"] }]); }); - + it("does not flatten a root union that constrains existing properties", () => { + const schema = { + type: "object", + properties: { kind: { type: "string" } }, + anyOf: [{ properties: { kind: { const: "a" } } }, { properties: { kind: { const: "b" } } }], + }; + expect(sanitizeSchemaForOpenAIResponses(schema).anyOf).toEqual([ + { properties: { kind: { const: "a" } } }, + { properties: { kind: { const: "b" } } }, + ]); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/ai/test/schema-wire.test.ts b/packages/ai/test/schema-wire.test.ts index 17f92c3f7..3565c791e 100644 --- a/packages/ai/test/schema-wire.test.ts +++ b/packages/ai/test/schema-wire.test.ts @@ -127,7 +127,22 @@ describe("toolWireSchema — raw JSON Schema normalization", () => { expect(outputSchema.anyOf).toEqual([{ required: ["paths"] }, { required: ["scopes"] }]); }); - + it("does not flatten a root union that constrains existing properties", () => { + const wire = toolWireSchema( + jsonTool({ + type: "object", + properties: { kind: { type: "string" } }, + anyOf: [{ properties: { kind: { const: "a" } } }, { properties: { kind: { const: "b" } } }], + }), + ); + expect(wire.anyOf).toEqual([{ properties: { kind: { const: "a" } } }, { properties: { kind: { const: "b" } } }]); + const properties = wire.properties; + expect( + properties && typeof properties === "object" && "kind" in properties ? properties.kind : undefined, + ).toEqual({ + type: "string", + }); + }); it("preserves raw JSON Schema required defaults and safe-integer bounds", () => { const wire = toolWireSchema(