fix(ai): scoped xAI root-union flatten and quarantine
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<string, unknown>)
|
||||
: 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<string, unknown>)
|
||||
: 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,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -1579,7 +1579,6 @@ function normalizeOpenAIResponsesSchemaNode(value: unknown, cache: WeakMap<JsonO
|
||||
changed = true;
|
||||
}
|
||||
|
||||
|
||||
// Safe to overwrite the seed: any cyclic re-entry above already observed
|
||||
// the seeded partial and set `changed = true` for that node, so a node
|
||||
// that finishes with `changed === false` is provably non-cyclic and
|
||||
@@ -1589,7 +1588,6 @@ function normalizeOpenAIResponsesSchemaNode(value: unknown, cache: WeakMap<JsonO
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* xAI (and some OpenAI-compat hosts) reject a tool whose *root* schema is an
|
||||
* `anyOf`/`oneOf` with a typeless or non-object branch — even when the node
|
||||
@@ -1599,21 +1597,17 @@ function normalizeOpenAIResponsesSchemaNode(value: unknown, cache: WeakMap<JsonO
|
||||
* stays on the wire; exclusive-required becomes both-optional (server still
|
||||
* enforces the OR).
|
||||
*/
|
||||
function isTypelessObjectConstraintBranch(branch: unknown): boolean {
|
||||
function isExclusiveRequiredBranch(branch: unknown): boolean {
|
||||
if (!isJsonObject(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 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;
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
|
||||
const SCHEMA_TYPE_NAMES: Record<string, true> = {
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -207,27 +207,17 @@ function rewriteNullableScalarAnyOf(schema: Record<string, unknown>): 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<string, unknown>): void {
|
||||
@@ -237,23 +227,10 @@ function flattenExclusiveRequiredUnion(schema: Record<string, unknown>): 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",
|
||||
|
||||
@@ -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<ChatCompletionsPayload> {
|
||||
const { promise, resolve } = Promise.withResolvers<ChatCompletionsPayload>();
|
||||
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<string | undefined> {
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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 }));
|
||||
|
||||
@@ -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" } } },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user