mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-18 04:42:30 +03:00
fix(sse): guard empty tool_calls[] and strip tool_choice without tools (#12901)
Two related schema-compliance fixes for upstreams that enforce the OpenAI
spec strictly (vLLM self-hosted, Kimi-K2.6):
1. Empty tool_calls[] guard: providers like Kimi-K2.6 attach an empty
tool_calls:[] array to every content delta when tools are defined. An
empty array is truthy, so guarding on 'delta.tool_calls' alone called
closeMessage() after the first content delta, closing the message item
prematurely. Subsequent content deltas arrived on a done item and were
dropped by clients (Codex: 'OutputTextDelta without active item'),
leaving only the first text fragment in the output. Guard both
translation paths on 'delta.tool_calls?.length' so closeMessage runs
only when at least one actual tool call is present:
- open-sse/translator/response/openai-responses.ts (chatCore translate path)
- open-sse/transformer/responsesTransformer.ts (/v1/responses direct path)
2. tool_choice schema guard: auxiliary/internal calls (e.g. WebSearch)
legitimately send tool_choice:'auto' with no tools array, and routing
may drop the tools array after the client sent it. vLLM rejects this
combination with a schema 400 ('When using tool_choice, tools must be
set'). Add stripToolChoiceWithoutTools() to targetRequestSanitizer.ts
that removes a dangling tool_choice lacking a usable tools array at
the common dispatch boundary. The guard is OpenAI-spec compliance, not
provider-specific, so it fires regardless of provider.
TDD: tests reproduce both bugs (Red: 5 fail → Green: 31 pass, 0 fail):
- tests/unit/translator-openai-responses-empty-tool-calls.test.ts (2 tests)
- tests/unit/responses-transformer.test.ts (+2 tests, 21 total pass)
- tests/unit/tool-choice-schema-normalization.test.ts (8 tests)
typecheck:core: 0 errors
Co-authored-by: Jihyun Son <jihyun.son@sk.com>
This commit is contained in:
@@ -48,6 +48,30 @@ function stripVerbosityForTarget(body: JsonRecord, model: string): string[] {
|
||||
return stripped;
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip a `tool_choice` control that lacks a usable `tools` array.
|
||||
*
|
||||
* Some upstreams (vLLM self-hosted) reject the combination "tool_choice set
|
||||
* without tools" with a schema 400 — "When using tool_choice, tools must be
|
||||
* set." Auxiliary/internal calls (e.g. WebSearch) legitimately send
|
||||
* `tool_choice:"auto"` with no tools array, and routing/fallback may have
|
||||
* dropped the tools array after the client sent it. This guard removes the
|
||||
* dangling `tool_choice` so the request stays schema-valid for any upstream
|
||||
* that enforces the OpenAI spec. It is OpenAI-spec compliance, not
|
||||
* provider-specific, so it fires regardless of provider.
|
||||
*/
|
||||
function stripToolChoiceWithoutTools(body: JsonRecord): boolean {
|
||||
if (!Object.hasOwn(body, "tool_choice")) return false;
|
||||
const tc = body.tool_choice;
|
||||
// A falsy/null tool_choice carries no "use tools" intent — leave as-is.
|
||||
if (!tc) return false;
|
||||
const tools = body.tools;
|
||||
const hasUsableTools = Array.isArray(tools) && tools.length > 0;
|
||||
if (hasUsableTools) return false;
|
||||
delete body.tool_choice;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize a translated request using the concrete provider/model selected by
|
||||
* routing. Returns a fresh top-level object and never mutates the caller body.
|
||||
@@ -79,10 +103,17 @@ export function sanitizeRequestForResolvedTarget<T extends JsonRecord>(
|
||||
// boundary so custom executors cannot accidentally bypass them.
|
||||
stripUnsupportedParams(options.provider, options.model, next);
|
||||
|
||||
if (stripped.length > 0) {
|
||||
// Strip a dangling tool_choice that lacks a usable tools array — upstreams
|
||||
// that enforce the OpenAI spec (vLLM) reject "tool_choice without tools"
|
||||
// with a schema 400.
|
||||
const strippedToolChoice = stripToolChoiceWithoutTools(next);
|
||||
|
||||
if (stripped.length > 0 || strippedToolChoice) {
|
||||
const parts = [...stripped];
|
||||
if (strippedToolChoice) parts.push("tool_choice (no tools)");
|
||||
options.log?.debug?.(
|
||||
"TARGET_PARAMS",
|
||||
`Stripped ${stripped.join(", ")} for resolved target ${options.provider || "unknown"}/${options.model}`
|
||||
`Stripped ${parts.join(", ")} for resolved target ${options.provider || "unknown"}/${options.model}`
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -795,7 +795,7 @@ export function createResponsesApiTransformStream(
|
||||
}
|
||||
|
||||
// Handle tool_calls
|
||||
if (delta.tool_calls) {
|
||||
if (delta.tool_calls?.length) {
|
||||
// Close reasoning first so tool calls do not collide with an
|
||||
// open reasoning item, then close the message at its real index.
|
||||
if (state.reasoningId && !state.reasoningDone) {
|
||||
|
||||
@@ -288,7 +288,7 @@ export function openaiToOpenAIResponsesResponse(chunk, state) {
|
||||
}
|
||||
|
||||
// Handle tool_calls
|
||||
if (delta.tool_calls) {
|
||||
if (delta.tool_calls?.length) {
|
||||
// Close reasoning first so tool calls do not collide with an open
|
||||
// reasoning item, then close the message at its real index.
|
||||
if (state.reasoningId && !state.reasoningDone) {
|
||||
|
||||
@@ -547,3 +547,64 @@ test("createResponsesApiTransformStream keepalive self-clears when enqueue fails
|
||||
globalThis.clearInterval = realClearInterval;
|
||||
}
|
||||
});
|
||||
|
||||
// Regression: providers (e.g. Kimi-K2.6) emit content deltas that carry an empty
|
||||
// `tool_calls:[]` array in the SAME chunk when tools are defined. The empty array is
|
||||
// truthy, so the old `if (delta.tool_calls)` guard entered the tool-call branch and
|
||||
// called closeMessage() immediately — closing the message item after only the first
|
||||
// content delta. Subsequent content deltas arrived on a done item, and Codex
|
||||
// (which clears `active_item` on `output_item.done`) dropped them with
|
||||
// "OutputTextDelta without active item", producing a one-character response.
|
||||
// The guard must ignore an empty tool_calls array so the message stays open.
|
||||
test("createResponsesApiTransformStream does not close the message on an empty tool_calls array paired with content (Kimi-K2.6 pattern)", async () => {
|
||||
const output = await runTransformStream([
|
||||
'data: {"id":"chatcmpl_1","choices":[{"index":0,"delta":{"content":"H","tool_calls":[]}}]}\n\n',
|
||||
'data: {"choices":[{"index":0,"delta":{"content":"ello","tool_calls":[]}}]}\n\n',
|
||||
'data: {"choices":[{"index":0,"delta":{"content":" world","tool_calls":[]}}]}\n\n',
|
||||
'data: {"choices":[{"index":0,"delta":{},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":2,"total_tokens":3}}\n\n',
|
||||
]);
|
||||
|
||||
const events = parseSseOutput(output);
|
||||
const textDeltas = events
|
||||
.filter((event) => event.event === "response.output_text.delta")
|
||||
.map((event) => JSON.parse(event.data).delta);
|
||||
const completed = JSON.parse(
|
||||
events.find((event) => event.event === "response.completed").data
|
||||
).response;
|
||||
|
||||
// All three content deltas must be emitted — not just the first one.
|
||||
assert.deepEqual(textDeltas, ["H", "ello", " world"]);
|
||||
// Exactly ONE assistant message item, carrying the full concatenated text.
|
||||
const messageItems = completed.output.filter((item) => item.type === "message");
|
||||
assert.equal(messageItems.length, 1, "empty tool_calls must not split/close the message");
|
||||
assert.equal(messageItems[0].content[0].text, "Hello world");
|
||||
// No function_call items should be synthesized from the empty arrays.
|
||||
const functionCallItems = completed.output.filter((item) => item.type === "function_call");
|
||||
assert.deepEqual(functionCallItems, []);
|
||||
});
|
||||
|
||||
// The same fix must not regress the real tool-call path: when tool_calls carries an
|
||||
// actual entry, the preceding content message must still close so the tool call is its
|
||||
// own output item.
|
||||
test("createResponsesApiTransformStream still closes the message and emits a real tool call when tool_calls is non-empty", async () => {
|
||||
const output = await runTransformStream([
|
||||
'data: {"id":"chatcmpl_1","choices":[{"index":0,"delta":{"content":"let me search","tool_calls":[]}}]}\n\n',
|
||||
'data: {"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"search","arguments":"{\\"q\\":\\"hi\\"}"}}]}}]}\n\n',
|
||||
'data: {"choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}],"usage":{"prompt_tokens":1,"completion_tokens":2,"total_tokens":3}}\n\n',
|
||||
]);
|
||||
|
||||
const events = parseSseOutput(output);
|
||||
const completed = JSON.parse(
|
||||
events.find((event) => event.event === "response.completed").data
|
||||
).response;
|
||||
|
||||
const messageItems = completed.output.filter((item) => item.type === "message");
|
||||
const functionCallItems = completed.output.filter((item) => item.type === "function_call");
|
||||
|
||||
// The text message closed with its full content, and the tool call is a separate item.
|
||||
assert.equal(messageItems.length, 1);
|
||||
assert.equal(messageItems[0].content[0].text, "let me search");
|
||||
assert.equal(functionCallItems.length, 1);
|
||||
assert.equal(functionCallItems[0].call_id, "call_1");
|
||||
assert.equal(functionCallItems[0].arguments, '{"q":"hi"}');
|
||||
});
|
||||
|
||||
138
tests/unit/tool-choice-schema-normalization.test.ts
Normal file
138
tests/unit/tool-choice-schema-normalization.test.ts
Normal file
@@ -0,0 +1,138 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { sanitizeRequestForResolvedTarget } =
|
||||
await import("../../open-sse/services/targetRequestSanitizer.ts");
|
||||
|
||||
const baseOpts = { provider: "skhynix", model: "DeepSeek-V4-Flash-0731" } as const;
|
||||
|
||||
test("tool_choice schema normalization: strips tool_choice when tools absent (vLLM 400 guard)", () => {
|
||||
// WebSearch-style auxiliary call: tool_choice:"auto" with NO tools array.
|
||||
// vLLM (Hosted_vllmException) rejects this: "When using tool_choice, tools must be set."
|
||||
const body = {
|
||||
model: "DeepSeek-V4-Flash-0731",
|
||||
messages: [{ role: "user", content: "search the web" }],
|
||||
tool_choice: "auto",
|
||||
} as Record<string, unknown>;
|
||||
|
||||
const out = sanitizeRequestForResolvedTarget(body, baseOpts);
|
||||
|
||||
assert.equal(
|
||||
Object.prototype.hasOwnProperty.call(out, "tool_choice"),
|
||||
false,
|
||||
"tool_choice must be removed when tools is absent"
|
||||
);
|
||||
assert.equal(
|
||||
Object.prototype.hasOwnProperty.call(out, "tools"),
|
||||
false,
|
||||
"tools key should not be introduced"
|
||||
);
|
||||
});
|
||||
|
||||
test("tool_choice schema normalization: strips tool_choice when tools is empty array", () => {
|
||||
const body = {
|
||||
model: "DeepSeek-V4-Flash-0731",
|
||||
messages: [{ role: "user", content: "hi" }],
|
||||
tools: [],
|
||||
tool_choice: "auto",
|
||||
} as Record<string, unknown>;
|
||||
|
||||
const out = sanitizeRequestForResolvedTarget(body, baseOpts);
|
||||
|
||||
assert.equal(
|
||||
Object.prototype.hasOwnProperty.call(out, "tool_choice"),
|
||||
false,
|
||||
"tool_choice must be removed when tools is an empty array"
|
||||
);
|
||||
// empty tools array itself can stay — only tool_choice is the schema violation
|
||||
});
|
||||
|
||||
test("tool_choice schema normalization: preserves tool_choice when tools present", () => {
|
||||
const body = {
|
||||
model: "DeepSeek-V4-Flash-0731",
|
||||
messages: [{ role: "user", content: "use a tool" }],
|
||||
tools: [{ type: "function", function: { name: "get_weather", parameters: {} } }],
|
||||
tool_choice: "auto",
|
||||
} as Record<string, unknown>;
|
||||
|
||||
const out = sanitizeRequestForResolvedTarget(body, baseOpts);
|
||||
|
||||
assert.equal(out.tool_choice, "auto", "tool_choice must be preserved when tools present");
|
||||
assert.equal(Array.isArray(out.tools), true, "tools array must be preserved");
|
||||
assert.equal((out.tools as unknown[]).length, 1);
|
||||
});
|
||||
|
||||
test("tool_choice schema normalization: preserves object tool_choice with tools", () => {
|
||||
const body = {
|
||||
model: "DeepSeek-V4-Flash-0731",
|
||||
messages: [{ role: "user", content: "x" }],
|
||||
tools: [{ type: "function", function: { name: "fn", parameters: {} } }],
|
||||
tool_choice: { type: "function", function: { name: "fn" } },
|
||||
} as Record<string, unknown>;
|
||||
|
||||
const out = sanitizeRequestForResolvedTarget(body, baseOpts);
|
||||
|
||||
assert.ok(typeof out.tool_choice === "object", "object tool_choice preserved when tools present");
|
||||
});
|
||||
|
||||
test("tool_choice schema normalization: no-op when tool_choice absent", () => {
|
||||
const body = {
|
||||
model: "DeepSeek-V4-Flash-0731",
|
||||
messages: [{ role: "user", content: "hi" }],
|
||||
} as Record<string, unknown>;
|
||||
|
||||
const out = sanitizeRequestForResolvedTarget(body, baseOpts);
|
||||
|
||||
assert.equal(
|
||||
Object.prototype.hasOwnProperty.call(out, "tool_choice"),
|
||||
false,
|
||||
"no tool_choice key introduced when absent"
|
||||
);
|
||||
});
|
||||
|
||||
test("tool_choice schema normalization: no-op for null tool_choice without tools", () => {
|
||||
const body = {
|
||||
model: "DeepSeek-V4-Flash-0731",
|
||||
messages: [{ role: "user", content: "hi" }],
|
||||
tool_choice: null,
|
||||
} as Record<string, unknown>;
|
||||
|
||||
const out = sanitizeRequestForResolvedTarget(body, baseOpts);
|
||||
|
||||
// null tool_choice is falsy and carries no "use tools" intent; leave as-is
|
||||
// (the guard only strips a DEFINED, truthy tool_choice lacking a tools array)
|
||||
assert.equal(out.tool_choice, null);
|
||||
});
|
||||
|
||||
test("tool_choice schema normalization: applies regardless of provider (global schema guard)", () => {
|
||||
// The guard is OpenAI-spec compliance, not provider-specific — it must fire
|
||||
// for any provider whose upstream enforces "tool_choice requires tools".
|
||||
for (const provider of ["skhynix", "openai", "nvidia", "deepseek"]) {
|
||||
const body = {
|
||||
model: "any-model",
|
||||
messages: [{ role: "user", content: "x" }],
|
||||
tool_choice: "required",
|
||||
} as Record<string, unknown>;
|
||||
|
||||
const out = sanitizeRequestForResolvedTarget(body, { provider, model: "any-model" });
|
||||
|
||||
assert.equal(
|
||||
Object.prototype.hasOwnProperty.call(out, "tool_choice"),
|
||||
false,
|
||||
`tool_choice must be stripped for provider=${provider} when tools absent`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("tool_choice schema normalization: does not mutate caller body", () => {
|
||||
const body = {
|
||||
model: "DeepSeek-V4-Flash-0731",
|
||||
messages: [{ role: "user", content: "x" }],
|
||||
tool_choice: "auto",
|
||||
} as Record<string, unknown>;
|
||||
|
||||
sanitizeRequestForResolvedTarget(body, baseOpts);
|
||||
|
||||
// the function returns a fresh object and must not mutate the caller's body
|
||||
assert.equal(body.tool_choice, "auto", "caller body must not be mutated");
|
||||
});
|
||||
146
tests/unit/translator-openai-responses-empty-tool-calls.test.ts
Normal file
146
tests/unit/translator-openai-responses-empty-tool-calls.test.ts
Normal file
@@ -0,0 +1,146 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { openaiToOpenAIResponsesResponse } =
|
||||
await import("../../open-sse/translator/response/openai-responses.ts");
|
||||
const { initState } = await import("../../open-sse/translator/index.ts");
|
||||
const { FORMATS } = await import("../../open-sse/translator/formats.ts");
|
||||
|
||||
/**
|
||||
* Reproduces the Kimi-K2.6 empty-tool_calls bug.
|
||||
*
|
||||
* Kimi-K2.6 attaches an EMPTY `tool_calls:[]` array to every content delta when
|
||||
* tools are defined. An empty array is truthy, so `if (delta.tool_calls)` is
|
||||
* true even when no actual tool call is present. The translator then calls
|
||||
* closeMessage() right after the first content delta, closing the message item
|
||||
* with only the first fragment of text. Subsequent content deltas arrive on a
|
||||
* done item and are emitted as orphan `output_text.delta` events (no
|
||||
* matching active item) — Codex CLI panics with "OutputTextDelta without
|
||||
* active item" and only the first fragment reaches the final output.
|
||||
*
|
||||
* Expected (after fix): both content fragments accumulate into ONE message
|
||||
* item, `output_text.done` carries the full text, and the final
|
||||
* `response.completed.response.output` message contains the full text.
|
||||
*/
|
||||
function collectEvents(chunks) {
|
||||
const state = initState(FORMATS.OPENAI_RESPONSES);
|
||||
const events = [];
|
||||
for (const chunk of chunks) {
|
||||
const result = openaiToOpenAIResponsesResponse(chunk, state);
|
||||
if (result) events.push(...result);
|
||||
}
|
||||
return events;
|
||||
}
|
||||
|
||||
test("Kimi-K2.6: empty tool_calls:[] on content deltas must NOT close the message item", () => {
|
||||
const events = collectEvents([
|
||||
// First content delta with an EMPTY tool_calls array attached (Kimi pattern)
|
||||
{
|
||||
id: "chatcmpl-kimi",
|
||||
model: "Kimi-K2.6",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: { content: " The", tool_calls: [] },
|
||||
finish_reason: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
// Second content delta — also carries an empty tool_calls array
|
||||
{
|
||||
id: "chatcmpl-kimi",
|
||||
model: "Kimi-K2.6",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: { content: " fix works.", tool_calls: [] },
|
||||
finish_reason: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
// Final chunk: finish_reason (no content)
|
||||
{
|
||||
id: "chatcmpl-kimi",
|
||||
model: "Kimi-K2.6",
|
||||
choices: [{ index: 0, delta: {}, finish_reason: "stop" }],
|
||||
usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 },
|
||||
},
|
||||
]);
|
||||
|
||||
const textDeltas = events
|
||||
.filter((e) => e.event === "response.output_text.delta")
|
||||
.map((e) => e.data.delta);
|
||||
const textDone = events
|
||||
.filter((e) => e.event === "response.output_text.done")
|
||||
.map((e) => e.data.text);
|
||||
|
||||
// Both content fragments must be emitted as deltas on the SAME item
|
||||
assert.deepEqual(textDeltas, [" The", " fix works."]);
|
||||
|
||||
// Exactly one output_text.done carrying the FULL accumulated text
|
||||
assert.equal(textDone.length, 1, "must emit exactly one output_text.done");
|
||||
assert.equal(textDone[0], " The fix works.", "output_text.done must carry full text");
|
||||
|
||||
// The final completed response output must contain the full text
|
||||
const completed = events.find((e) => e.event === "response.completed");
|
||||
assert.ok(completed, "must emit response.completed");
|
||||
const msgItems = completed.data.response.output.filter((o) => o.type === "message");
|
||||
assert.equal(msgItems.length, 1, "must have exactly one message item");
|
||||
assert.equal(
|
||||
msgItems[0].content[0].text,
|
||||
" The fix works.",
|
||||
"final output message must contain the full text"
|
||||
);
|
||||
});
|
||||
|
||||
test("Kimi-K2.6: a REAL tool_call (non-empty) still closes the message before the call", () => {
|
||||
const events = collectEvents([
|
||||
{
|
||||
id: "chatcmpl-kimi2",
|
||||
model: "Kimi-K2.6",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: { content: "thinking", tool_calls: [] },
|
||||
finish_reason: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id: "chatcmpl-kimi2",
|
||||
model: "Kimi-K2.6",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: {
|
||||
tool_calls: [
|
||||
{
|
||||
index: 0,
|
||||
id: "call_1",
|
||||
function: { name: "get_weather", arguments: '{"city":"NYC"}' },
|
||||
},
|
||||
],
|
||||
},
|
||||
finish_reason: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id: "chatcmpl-kimi2",
|
||||
model: "Kimi-K2.6",
|
||||
choices: [{ index: 0, delta: {}, finish_reason: "tool_calls" }],
|
||||
usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 },
|
||||
},
|
||||
]);
|
||||
|
||||
// The content "thinking" must be in the final output as a message
|
||||
const completed = events.find((e) => e.event === "response.completed");
|
||||
const msgItems = completed.data.response.output.filter((o) => o.type === "message");
|
||||
assert.equal(msgItems.length, 1);
|
||||
assert.equal(msgItems[0].content[0].text, "thinking");
|
||||
// And a function_call item must exist
|
||||
const fnItems = completed.data.response.output.filter(
|
||||
(o) => o.type === "function_call" || o.type === "custom_tool_call"
|
||||
);
|
||||
assert.ok(fnItems.length >= 1, "must have a function/custom tool call item");
|
||||
});
|
||||
Reference in New Issue
Block a user