diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index c076f8ce48..0464af5709 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -543,6 +543,24 @@ function isXaiSchemaTarget(provider: CodexCommanderProviderConfig): boolean { } } +// Moonshot's strict validator ("moonshot flavored json schema") 400s any schema +// node that carries a `$ref` next to sibling keys: +// At path '$defs.__schema20': when using $ref, type should be defined in the +// referenced schema instead of the parent schema +// Codex emits exactly that shape for deferred/dynamic tools with recursive +// parameter schemas (zod-style serialization: `$defs.__schemaN` with a `$ref` +// plus a sibling `type`/`description`). Bare `$ref`s are accepted, so the +// sanitizer below only rewrites ref-with-siblings nodes. +const MOONSHOT_SCHEMA_HOSTNAMES = new Set(["api.moonshot.ai", "api.moonshot.cn", "api.kimi.com"]); + +function isMoonshotSchemaTarget(provider: CodexCommanderProviderConfig): boolean { + try { + return MOONSHOT_SCHEMA_HOSTNAMES.has(new URL(provider.baseUrl).hostname); + } catch { + return false; + } +} + // Volcengine Ark regional endpoints. Ark validates an assistant message's text field as a // REQUIRED parameter and treats "" as absent, so a tool-call-only assistant in history 400s with // `MissingParameter: input.content.text` (#796). Every other OpenAI-compatible provider accepts @@ -599,7 +617,9 @@ function ensureRootObjectType(parameters: unknown): Record { return { ...obj, type: "object" }; } -function resolveXaiLocalSchemaRef( +/** Resolve a local `#/...` JSON Pointer against a schema document. Shared by the + * xAI root expansion and the Moonshot `$ref`-sibling inlining. */ +function resolveLocalSchemaRef( ref: string, document: Record, ): Record | undefined { @@ -616,6 +636,81 @@ function resolveXaiLocalSchemaRef( : undefined; } +/** + * Merge `$ref` siblings into the resolved target. Object-valued `properties` + * merge per key and `required` unions so inlined constraints are not lost; + * every other sibling key wins over the target's value. + */ +function mergeRefSiblings( + target: Record, + siblings: Record, +): Record { + const merged: Record = { ...target }; + for (const [key, value] of Object.entries(siblings)) { + const existing = merged[key]; + if (key === "properties" && existing && typeof existing === "object" && !Array.isArray(existing) + && value && typeof value === "object" && !Array.isArray(value)) { + merged[key] = { ...existing as Record, ...value as Record }; + } else if (key === "required" && Array.isArray(existing) && Array.isArray(value)) { + merged[key] = [...new Set([...existing, ...value])]; + } else { + merged[key] = value; + } + } + return merged; +} + +/** + * Rewrite every schema node that carries a `$ref` next to sibling keys so the + * emitted document never mixes the two (Moonshot 400s that shape). A bare + * `$ref` is left untouched, which keeps recursive schemas intact. + * + * A sibling-carrying ref is inlined from its local target when that is safe: + * resolvable, acyclic (`expanding` tracks the refs on the current path), and + * the target itself fully resolves. Anything else — external refs, missing + * targets, cycles — collapses to the bare `$ref`, dropping the siblings; that + * keeps the document valid at the cost of the sibling annotations. + */ +function resolveMoonshotRefSiblings( + value: unknown, + document: Record, + expanding: ReadonlySet, +): unknown { + if (Array.isArray(value)) { + return value.map(entry => resolveMoonshotRefSiblings(entry, document, expanding)); + } + if (!value || typeof value !== "object") return value; + const obj = value as Record; + const ref = obj.$ref; + if (typeof ref === "string") { + const siblings = Object.fromEntries(Object.entries(obj).filter(([key]) => key !== "$ref")); + if (Object.keys(siblings).length === 0) return { $ref: ref }; + const target = resolveLocalSchemaRef(ref, document); + if (target && !expanding.has(ref)) { + const next = new Set(expanding); + next.add(ref); + const walkedTarget = resolveMoonshotRefSiblings(target, document, next) as Record; + if (typeof walkedTarget.$ref !== "string") { + const walkedSiblings = resolveMoonshotRefSiblings(siblings, document, next) as Record; + return mergeRefSiblings(walkedTarget, walkedSiblings); + } + } + return { $ref: ref }; + } + const out: Record = {}; + for (const [key, child] of Object.entries(obj)) { + out[key] = resolveMoonshotRefSiblings(child, document, expanding); + } + return out; +} + +function normalizeMoonshotToolParameters(parameters: unknown): Record { + const document = parameters && typeof parameters === "object" && !Array.isArray(parameters) + ? parameters as Record + : {}; + return ensureRootObjectType(resolveMoonshotRefSiblings(parameters, document, new Set())); +} + function expandXaiRootObjectSchemas( schema: unknown, document: Record, @@ -625,7 +720,7 @@ function expandXaiRootObjectSchemas( const obj = schema as Record; if (obj.$ref !== undefined) { if (typeof obj.$ref !== "string" || seenRefs.has(obj.$ref)) return undefined; - const target = resolveXaiLocalSchemaRef(obj.$ref, document); + const target = resolveLocalSchemaRef(obj.$ref, document); if (!target) return undefined; const nextSeen = new Set(seenRefs); nextSeen.add(obj.$ref); @@ -676,10 +771,13 @@ function toolsToChatFormat(parsed: CodexCommanderParsedRequest, provider: CodexC : parsed.context.tools; if (tools.length === 0) return undefined; const xaiTarget = isXaiSchemaTarget(provider); + const moonshotTarget = isMoonshotSchemaTarget(provider); const formatted = tools.flatMap(t => { const parameters = xaiTarget ? normalizeXaiToolParameters(t.parameters) - : ensureRootObjectType(t.parameters); + : moonshotTarget + ? normalizeMoonshotToolParameters(t.parameters) + : ensureRootObjectType(t.parameters); if (parameters === undefined) return []; return [{ diff --git a/tests/moonshot-tool-schema.test.ts b/tests/moonshot-tool-schema.test.ts new file mode 100644 index 0000000000..14f1e8751e --- /dev/null +++ b/tests/moonshot-tool-schema.test.ts @@ -0,0 +1,151 @@ +import { describe, expect, test } from "bun:test"; +import { createOpenAIChatAdapter as createOpenAIChatAdapterProduction } from "../src/adapters/openai-chat"; +import type { CodexCommanderParsedRequest, CodexCommanderProviderConfig } from "../src/types"; +import { withTestTranslatorBudget } from "./helpers/translator-budget"; + +const createOpenAIChatAdapter = (...args: Parameters) => + withTestTranslatorBudget(createOpenAIChatAdapterProduction(...args)); + +function provider(overrides: Partial = {}): CodexCommanderProviderConfig { + return { + adapter: "openai-chat", + baseUrl: "https://api.moonshot.ai/v1", + apiKey: "sk-test", + authMode: "key", + ...overrides, + }; +} + +function parsedWithParameters(parameters: Record): CodexCommanderParsedRequest { + return { + modelId: "kimi-k3", + context: { + messages: [{ role: "user", content: "hi", timestamp: 0 }], + tools: [{ name: "create_thread", description: "make a thread", parameters }], + }, + stream: false, + options: {}, + }; +} + +function emittedParameters(req: CodexCommanderParsedRequest, prov = provider()): Record { + const built = createOpenAIChatAdapter(prov).buildRequest(req) as { body: string }; + const body = JSON.parse(built.body) as { tools: { function: { parameters: Record } }[] }; + return body.tools[0]!.function.parameters; +} + +/** Collect every node in the schema tree that carries a `$ref` key. */ +function refNodes(value: unknown, found: Record[] = []): Record[] { + if (Array.isArray(value)) { + for (const entry of value) refNodes(entry, found); + return found; + } + if (!value || typeof value !== "object") return found; + const obj = value as Record; + if (typeof obj.$ref === "string") found.push(obj); + for (const child of Object.values(obj)) refNodes(child, found); + return found; +} + +describe("moonshot tool schema sanitization", () => { + // Reproduces the exact 400 shape seen from api.moonshot.ai: + // At path '$defs.__schema20': when using $ref, type should be defined + // in the referenced schema instead of the parent schema + const REPORTED_SHAPE = { + type: "object", + properties: { + brief: { $ref: "#/$defs/__schema10", description: "what to do" }, + }, + $defs: { + __schema10: { type: "object", properties: { title: { type: "string" } } }, + __schema20: { $ref: "#/$defs/__schema10", type: "object" }, + }, + }; + + test("no emitted $ref node carries sibling keys (the reported $defs.__schema20 case)", () => { + const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE)); + for (const node of refNodes(parameters)) { + expect(Object.keys(node)).toEqual(["$ref"]); + } + }); + + test("a $defs entry that is $ref + type is inlined from its target", () => { + const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE)); + const defs = parameters.$defs as Record>; + expect(defs.__schema20).toMatchObject({ + type: "object", + properties: { title: { type: "string" } }, + }); + }); + + test("a property-level $ref with a description sibling is resolved", () => { + const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE)); + const props = parameters.properties as Record>; + expect(props.brief).toMatchObject({ + type: "object", + description: "what to do", + properties: { title: { type: "string" } }, + }); + }); + + test.each(["https://api.moonshot.ai/v1", "https://api.moonshot.cn/v1", "https://api.kimi.com/coding/v1"])( + "applies to %s", + baseUrl => { + const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE), provider({ baseUrl })); + for (const node of refNodes(parameters)) { + expect(Object.keys(node)).toEqual(["$ref"]); + } + }, + ); + + test("recursive schemas stay referenced and terminate", () => { + const parameters = emittedParameters(parsedWithParameters({ + type: "object", + properties: { root: { $ref: "#/$defs/node", type: "object" } }, + $defs: { + node: { + type: "object", + properties: { + children: { type: "array", items: { $ref: "#/$defs/node" } }, + }, + }, + }, + })); + const props = parameters.properties as Record>; + // The sibling-carrying root ref is inlined once; the inner bare $ref keeps the recursion intact. + expect(props.root).toMatchObject({ type: "object" }); + const node = (parameters.$defs as Record>).node; + const items = (node.properties as Record>).children + .items as Record; + expect(items).toEqual({ $ref: "#/$defs/node" }); + }); + + test("unresolvable or external refs keep the $ref and drop siblings", () => { + const parameters = emittedParameters(parsedWithParameters({ + type: "object", + properties: { + external: { $ref: "https://example.com/schema.json", type: "object" }, + missing: { $ref: "#/$defs/nope", description: "gone" }, + }, + })); + const props = parameters.properties as Record>; + expect(props.external).toEqual({ $ref: "https://example.com/schema.json" }); + expect(props.missing).toEqual({ $ref: "#/$defs/nope" }); + }); + + test("non-moonshot providers pass the shape through untouched", () => { + const parameters = emittedParameters( + parsedWithParameters(REPORTED_SHAPE), + provider({ baseUrl: "https://api.deepseek.com/v1" }), + ); + const defs = parameters.$defs as Record>; + expect(defs.__schema20).toEqual({ $ref: "#/$defs/__schema10", type: "object" }); + }); + + test("root still gains type: object when missing", () => { + const parameters = emittedParameters(parsedWithParameters({ + properties: { title: { type: "string" } }, + })); + expect(parameters.type).toBe("object"); + }); +});