Skip to content

Commit 3e4ad27

Browse files
author
pavelhov
authored
Merge pull request #15 from pavelhov/codex/moonshot-ref-sibling-schema
fix(openai-chat): strip $ref siblings from tool schemas for Moonshot
2 parents dd511ab + 79f8670 commit 3e4ad27

2 files changed

Lines changed: 252 additions & 3 deletions

File tree

‎src/adapters/openai-chat.ts‎

Lines changed: 101 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,24 @@ function isXaiSchemaTarget(provider: CodexCommanderProviderConfig): boolean {
543543
}
544544
}
545545

546+
// Moonshot's strict validator ("moonshot flavored json schema") 400s any schema
547+
// node that carries a `$ref` next to sibling keys:
548+
// At path '$defs.__schema20': when using $ref, type should be defined in the
549+
// referenced schema instead of the parent schema
550+
// Codex emits exactly that shape for deferred/dynamic tools with recursive
551+
// parameter schemas (zod-style serialization: `$defs.__schemaN` with a `$ref`
552+
// plus a sibling `type`/`description`). Bare `$ref`s are accepted, so the
553+
// sanitizer below only rewrites ref-with-siblings nodes.
554+
const MOONSHOT_SCHEMA_HOSTNAMES = new Set(["api.moonshot.ai", "api.moonshot.cn", "api.kimi.com"]);
555+
556+
function isMoonshotSchemaTarget(provider: CodexCommanderProviderConfig): boolean {
557+
try {
558+
return MOONSHOT_SCHEMA_HOSTNAMES.has(new URL(provider.baseUrl).hostname);
559+
} catch {
560+
return false;
561+
}
562+
}
563+
546564
// Volcengine Ark regional endpoints. Ark validates an assistant message's text field as a
547565
// REQUIRED parameter and treats "" as absent, so a tool-call-only assistant in history 400s with
548566
// `MissingParameter: input.content.text` (#796). Every other OpenAI-compatible provider accepts
@@ -599,7 +617,9 @@ function ensureRootObjectType(parameters: unknown): Record<string, unknown> {
599617
return { ...obj, type: "object" };
600618
}
601619

602-
function resolveXaiLocalSchemaRef(
620+
/** Resolve a local `#/...` JSON Pointer against a schema document. Shared by the
621+
* xAI root expansion and the Moonshot `$ref`-sibling inlining. */
622+
function resolveLocalSchemaRef(
603623
ref: string,
604624
document: Record<string, unknown>,
605625
): Record<string, unknown> | undefined {
@@ -616,6 +636,81 @@ function resolveXaiLocalSchemaRef(
616636
: undefined;
617637
}
618638

639+
/**
640+
* Merge `$ref` siblings into the resolved target. Object-valued `properties`
641+
* merge per key and `required` unions so inlined constraints are not lost;
642+
* every other sibling key wins over the target's value.
643+
*/
644+
function mergeRefSiblings(
645+
target: Record<string, unknown>,
646+
siblings: Record<string, unknown>,
647+
): Record<string, unknown> {
648+
const merged: Record<string, unknown> = { ...target };
649+
for (const [key, value] of Object.entries(siblings)) {
650+
const existing = merged[key];
651+
if (key === "properties" && existing && typeof existing === "object" && !Array.isArray(existing)
652+
&& value && typeof value === "object" && !Array.isArray(value)) {
653+
merged[key] = { ...existing as Record<string, unknown>, ...value as Record<string, unknown> };
654+
} else if (key === "required" && Array.isArray(existing) && Array.isArray(value)) {
655+
merged[key] = [...new Set([...existing, ...value])];
656+
} else {
657+
merged[key] = value;
658+
}
659+
}
660+
return merged;
661+
}
662+
663+
/**
664+
* Rewrite every schema node that carries a `$ref` next to sibling keys so the
665+
* emitted document never mixes the two (Moonshot 400s that shape). A bare
666+
* `$ref` is left untouched, which keeps recursive schemas intact.
667+
*
668+
* A sibling-carrying ref is inlined from its local target when that is safe:
669+
* resolvable, acyclic (`expanding` tracks the refs on the current path), and
670+
* the target itself fully resolves. Anything else — external refs, missing
671+
* targets, cycles — collapses to the bare `$ref`, dropping the siblings; that
672+
* keeps the document valid at the cost of the sibling annotations.
673+
*/
674+
function resolveMoonshotRefSiblings(
675+
value: unknown,
676+
document: Record<string, unknown>,
677+
expanding: ReadonlySet<string>,
678+
): unknown {
679+
if (Array.isArray(value)) {
680+
return value.map(entry => resolveMoonshotRefSiblings(entry, document, expanding));
681+
}
682+
if (!value || typeof value !== "object") return value;
683+
const obj = value as Record<string, unknown>;
684+
const ref = obj.$ref;
685+
if (typeof ref === "string") {
686+
const siblings = Object.fromEntries(Object.entries(obj).filter(([key]) => key !== "$ref"));
687+
if (Object.keys(siblings).length === 0) return { $ref: ref };
688+
const target = resolveLocalSchemaRef(ref, document);
689+
if (target && !expanding.has(ref)) {
690+
const next = new Set(expanding);
691+
next.add(ref);
692+
const walkedTarget = resolveMoonshotRefSiblings(target, document, next) as Record<string, unknown>;
693+
if (typeof walkedTarget.$ref !== "string") {
694+
const walkedSiblings = resolveMoonshotRefSiblings(siblings, document, next) as Record<string, unknown>;
695+
return mergeRefSiblings(walkedTarget, walkedSiblings);
696+
}
697+
}
698+
return { $ref: ref };
699+
}
700+
const out: Record<string, unknown> = {};
701+
for (const [key, child] of Object.entries(obj)) {
702+
out[key] = resolveMoonshotRefSiblings(child, document, expanding);
703+
}
704+
return out;
705+
}
706+
707+
function normalizeMoonshotToolParameters(parameters: unknown): Record<string, unknown> {
708+
const document = parameters && typeof parameters === "object" && !Array.isArray(parameters)
709+
? parameters as Record<string, unknown>
710+
: {};
711+
return ensureRootObjectType(resolveMoonshotRefSiblings(parameters, document, new Set()));
712+
}
713+
619714
function expandXaiRootObjectSchemas(
620715
schema: unknown,
621716
document: Record<string, unknown>,
@@ -625,7 +720,7 @@ function expandXaiRootObjectSchemas(
625720
const obj = schema as Record<string, unknown>;
626721
if (obj.$ref !== undefined) {
627722
if (typeof obj.$ref !== "string" || seenRefs.has(obj.$ref)) return undefined;
628-
const target = resolveXaiLocalSchemaRef(obj.$ref, document);
723+
const target = resolveLocalSchemaRef(obj.$ref, document);
629724
if (!target) return undefined;
630725
const nextSeen = new Set(seenRefs);
631726
nextSeen.add(obj.$ref);
@@ -676,10 +771,13 @@ function toolsToChatFormat(parsed: CodexCommanderParsedRequest, provider: CodexC
676771
: parsed.context.tools;
677772
if (tools.length === 0) return undefined;
678773
const xaiTarget = isXaiSchemaTarget(provider);
774+
const moonshotTarget = isMoonshotSchemaTarget(provider);
679775
const formatted = tools.flatMap(t => {
680776
const parameters = xaiTarget
681777
? normalizeXaiToolParameters(t.parameters)
682-
: ensureRootObjectType(t.parameters);
778+
: moonshotTarget
779+
? normalizeMoonshotToolParameters(t.parameters)
780+
: ensureRootObjectType(t.parameters);
683781

684782
if (parameters === undefined) return [];
685783
return [{

‎tests/moonshot-tool-schema.test.ts‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { createOpenAIChatAdapter as createOpenAIChatAdapterProduction } from "../src/adapters/openai-chat";
3+
import type { CodexCommanderParsedRequest, CodexCommanderProviderConfig } from "../src/types";
4+
import { withTestTranslatorBudget } from "./helpers/translator-budget";
5+
6+
const createOpenAIChatAdapter = (...args: Parameters<typeof createOpenAIChatAdapterProduction>) =>
7+
withTestTranslatorBudget(createOpenAIChatAdapterProduction(...args));
8+
9+
function provider(overrides: Partial<CodexCommanderProviderConfig> = {}): CodexCommanderProviderConfig {
10+
return {
11+
adapter: "openai-chat",
12+
baseUrl: "https://api.moonshot.ai/v1",
13+
apiKey: "sk-test",
14+
authMode: "key",
15+
...overrides,
16+
};
17+
}
18+
19+
function parsedWithParameters(parameters: Record<string, unknown>): CodexCommanderParsedRequest {
20+
return {
21+
modelId: "kimi-k3",
22+
context: {
23+
messages: [{ role: "user", content: "hi", timestamp: 0 }],
24+
tools: [{ name: "create_thread", description: "make a thread", parameters }],
25+
},
26+
stream: false,
27+
options: {},
28+
};
29+
}
30+
31+
function emittedParameters(req: CodexCommanderParsedRequest, prov = provider()): Record<string, unknown> {
32+
const built = createOpenAIChatAdapter(prov).buildRequest(req) as { body: string };
33+
const body = JSON.parse(built.body) as { tools: { function: { parameters: Record<string, unknown> } }[] };
34+
return body.tools[0]!.function.parameters;
35+
}
36+
37+
/** Collect every node in the schema tree that carries a `$ref` key. */
38+
function refNodes(value: unknown, found: Record<string, unknown>[] = []): Record<string, unknown>[] {
39+
if (Array.isArray(value)) {
40+
for (const entry of value) refNodes(entry, found);
41+
return found;
42+
}
43+
if (!value || typeof value !== "object") return found;
44+
const obj = value as Record<string, unknown>;
45+
if (typeof obj.$ref === "string") found.push(obj);
46+
for (const child of Object.values(obj)) refNodes(child, found);
47+
return found;
48+
}
49+
50+
describe("moonshot tool schema sanitization", () => {
51+
// Reproduces the exact 400 shape seen from api.moonshot.ai:
52+
// At path '$defs.__schema20': when using $ref, type should be defined
53+
// in the referenced schema instead of the parent schema
54+
const REPORTED_SHAPE = {
55+
type: "object",
56+
properties: {
57+
brief: { $ref: "#/$defs/__schema10", description: "what to do" },
58+
},
59+
$defs: {
60+
__schema10: { type: "object", properties: { title: { type: "string" } } },
61+
__schema20: { $ref: "#/$defs/__schema10", type: "object" },
62+
},
63+
};
64+
65+
test("no emitted $ref node carries sibling keys (the reported $defs.__schema20 case)", () => {
66+
const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE));
67+
for (const node of refNodes(parameters)) {
68+
expect(Object.keys(node)).toEqual(["$ref"]);
69+
}
70+
});
71+
72+
test("a $defs entry that is $ref + type is inlined from its target", () => {
73+
const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE));
74+
const defs = parameters.$defs as Record<string, Record<string, unknown>>;
75+
expect(defs.__schema20).toMatchObject({
76+
type: "object",
77+
properties: { title: { type: "string" } },
78+
});
79+
});
80+
81+
test("a property-level $ref with a description sibling is resolved", () => {
82+
const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE));
83+
const props = parameters.properties as Record<string, Record<string, unknown>>;
84+
expect(props.brief).toMatchObject({
85+
type: "object",
86+
description: "what to do",
87+
properties: { title: { type: "string" } },
88+
});
89+
});
90+
91+
test.each(["https://api.moonshot.ai/v1", "https://api.moonshot.cn/v1", "https://api.kimi.com/coding/v1"])(
92+
"applies to %s",
93+
baseUrl => {
94+
const parameters = emittedParameters(parsedWithParameters(REPORTED_SHAPE), provider({ baseUrl }));
95+
for (const node of refNodes(parameters)) {
96+
expect(Object.keys(node)).toEqual(["$ref"]);
97+
}
98+
},
99+
);
100+
101+
test("recursive schemas stay referenced and terminate", () => {
102+
const parameters = emittedParameters(parsedWithParameters({
103+
type: "object",
104+
properties: { root: { $ref: "#/$defs/node", type: "object" } },
105+
$defs: {
106+
node: {
107+
type: "object",
108+
properties: {
109+
children: { type: "array", items: { $ref: "#/$defs/node" } },
110+
},
111+
},
112+
},
113+
}));
114+
const props = parameters.properties as Record<string, Record<string, unknown>>;
115+
// The sibling-carrying root ref is inlined once; the inner bare $ref keeps the recursion intact.
116+
expect(props.root).toMatchObject({ type: "object" });
117+
const node = (parameters.$defs as Record<string, Record<string, unknown>>).node;
118+
const items = (node.properties as Record<string, Record<string, unknown>>).children
119+
.items as Record<string, unknown>;
120+
expect(items).toEqual({ $ref: "#/$defs/node" });
121+
});
122+
123+
test("unresolvable or external refs keep the $ref and drop siblings", () => {
124+
const parameters = emittedParameters(parsedWithParameters({
125+
type: "object",
126+
properties: {
127+
external: { $ref: "https://example.com/schema.json", type: "object" },
128+
missing: { $ref: "#/$defs/nope", description: "gone" },
129+
},
130+
}));
131+
const props = parameters.properties as Record<string, Record<string, unknown>>;
132+
expect(props.external).toEqual({ $ref: "https://example.com/schema.json" });
133+
expect(props.missing).toEqual({ $ref: "#/$defs/nope" });
134+
});
135+
136+
test("non-moonshot providers pass the shape through untouched", () => {
137+
const parameters = emittedParameters(
138+
parsedWithParameters(REPORTED_SHAPE),
139+
provider({ baseUrl: "https://api.deepseek.com/v1" }),
140+
);
141+
const defs = parameters.$defs as Record<string, Record<string, unknown>>;
142+
expect(defs.__schema20).toEqual({ $ref: "#/$defs/__schema10", type: "object" });
143+
});
144+
145+
test("root still gains type: object when missing", () => {
146+
const parameters = emittedParameters(parsedWithParameters({
147+
properties: { title: { type: "string" } },
148+
}));
149+
expect(parameters.type).toBe("object");
150+
});
151+
});

0 commit comments

Comments
 (0)