mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-02 13:22:11 +03:00
Integrated into release/v3.8.36 (fixes #4714)
This commit is contained in:
committed by
GitHub
parent
de19b3b087
commit
b1b3069cd1
@@ -55,7 +55,9 @@ function convertClaudeServerWebSearchTool(tool: JsonRecord): JsonRecord {
|
||||
return {
|
||||
type: "web_search",
|
||||
...(Object.keys(filters).length > 0 ? { filters } : {}),
|
||||
...(tool.user_location && typeof tool.user_location === "object" && !Array.isArray(tool.user_location)
|
||||
...(tool.user_location &&
|
||||
typeof tool.user_location === "object" &&
|
||||
!Array.isArray(tool.user_location)
|
||||
? { user_location: tool.user_location }
|
||||
: {}),
|
||||
};
|
||||
@@ -133,32 +135,15 @@ export function claudeToOpenAIRequest(model, body, stream, credentials: unknown
|
||||
}
|
||||
}
|
||||
|
||||
// Fix missing tool responses - OpenAI requires every tool_call to have a response
|
||||
fixMissingToolResponses(result.messages);
|
||||
// #4714 / #4385: re-group every role:"tool" message immediately after the
|
||||
// assistant turn whose tool_calls issued it (dropping genuine orphans), then
|
||||
// fill placeholders for any tool_call left unanswered.
|
||||
result.messages = regroupToolMessages(result.messages);
|
||||
|
||||
// #4385: drop orphan tool results — a role:"tool" message whose tool_call_id has no
|
||||
// matching assistant.tool_calls (e.g. history truncation / compression removed the
|
||||
// assistant turn that issued the call but kept the tool_result). OpenAI-compatible
|
||||
// upstreams reject these with 502 "Messages with role 'tool' must be a response to a
|
||||
// preceding message with 'tool_calls'". Mirrors the filter already applied on the
|
||||
// Responses->Chat path in openai-responses.ts (#2893). Run after fixMissingToolResponses
|
||||
// so the inserted "[No response received]" placeholders (which DO match a tool_call)
|
||||
// are kept, and only genuinely orphaned results are removed.
|
||||
const assistantToolCallIds = new Set<string>();
|
||||
for (const msg of result.messages) {
|
||||
const calls = (msg as JsonRecord).tool_calls;
|
||||
if (Array.isArray(calls)) {
|
||||
for (const tc of calls as { id?: string }[]) {
|
||||
if (tc.id) assistantToolCallIds.add(String(tc.id));
|
||||
}
|
||||
}
|
||||
}
|
||||
result.messages = result.messages.filter((msg) => {
|
||||
if ((msg as JsonRecord).role === "tool") {
|
||||
return assistantToolCallIds.has(String((msg as JsonRecord).tool_call_id ?? ""));
|
||||
}
|
||||
return true;
|
||||
});
|
||||
// Fix missing tool responses - OpenAI requires every tool_call to have a response.
|
||||
// Runs after regrouping so real results are already adjacent and only a truly
|
||||
// unanswered tool_call receives a "[No response received]" placeholder.
|
||||
fixMissingToolResponses(result.messages);
|
||||
|
||||
const useNativeResponsesWebSearch = shouldUseNativeResponsesWebSearch(credentials);
|
||||
|
||||
@@ -223,6 +208,62 @@ export function claudeToOpenAIRequest(model, body, stream, credentials: unknown
|
||||
return result;
|
||||
}
|
||||
|
||||
// #4714: Re-group tool result messages so every role:"tool" message sits
|
||||
// immediately after the assistant message whose tool_calls issued it, in
|
||||
// tool_calls order. Claude Code can issue parallel tool_use in one assistant turn
|
||||
// but have their tool_result blocks arrive across SEPARATE user turns with
|
||||
// interleaved text; the naive conversion then left a role:"tool" stranded after a
|
||||
// user message, which OpenAI-compatible upstreams reject with 400 "Messages with
|
||||
// role 'tool' must be a response to a preceding message with 'tool_calls'".
|
||||
// Tool messages whose tool_call_id matches no assistant.tool_calls are dropped here
|
||||
// (supersedes the standalone #4385 orphan filter — same drop behavior, plus ordering).
|
||||
function regroupToolMessages(messages: JsonRecord[]): JsonRecord[] {
|
||||
// tool_call_id -> index of the assistant message that issued it (first wins).
|
||||
const callIdToAssistant = new Map<string, number>();
|
||||
messages.forEach((msg, idx) => {
|
||||
if (msg.role === "assistant" && Array.isArray(msg.tool_calls)) {
|
||||
for (const tc of msg.tool_calls as { id?: string }[]) {
|
||||
if (tc.id && !callIdToAssistant.has(String(tc.id))) {
|
||||
callIdToAssistant.set(String(tc.id), idx);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// Collect tool messages per assistant index, keyed by tool_call_id (first result wins).
|
||||
const toolsByAssistant = new Map<number, Map<string, JsonRecord>>();
|
||||
for (const msg of messages) {
|
||||
if (msg.role !== "tool") continue;
|
||||
const callId = String(msg.tool_call_id ?? "");
|
||||
const assistantIdx = callIdToAssistant.get(callId);
|
||||
if (assistantIdx === undefined) continue; // orphan -> drop
|
||||
let group = toolsByAssistant.get(assistantIdx);
|
||||
if (!group) {
|
||||
group = new Map();
|
||||
toolsByAssistant.set(assistantIdx, group);
|
||||
}
|
||||
if (!group.has(callId)) group.set(callId, msg);
|
||||
}
|
||||
|
||||
// Rebuild: keep non-tool messages in order; attach each assistant's tool results
|
||||
// immediately after it, ordered by the assistant's own tool_calls sequence.
|
||||
const out: JsonRecord[] = [];
|
||||
messages.forEach((msg, idx) => {
|
||||
if (msg.role === "tool") return; // moved into its assistant's group
|
||||
out.push(msg);
|
||||
if (msg.role === "assistant" && Array.isArray(msg.tool_calls)) {
|
||||
const group = toolsByAssistant.get(idx);
|
||||
if (group) {
|
||||
for (const tc of msg.tool_calls as { id?: string }[]) {
|
||||
const tool = tc.id ? group.get(String(tc.id)) : undefined;
|
||||
if (tool) out.push(tool);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
return out;
|
||||
}
|
||||
|
||||
// Fix missing tool responses - add empty responses for tool_calls without responses
|
||||
function fixMissingToolResponses(messages) {
|
||||
for (let i = 0; i < messages.length; i++) {
|
||||
|
||||
@@ -10,9 +10,8 @@ import assert from "node:assert/strict";
|
||||
// the assistant turn but kept the tool_result). OpenAI-compatible upstreams reject it.
|
||||
// This mirrors the orphan filter already on the Responses->Chat path (#2893).
|
||||
|
||||
const { claudeToOpenAIRequest } = await import(
|
||||
"../../open-sse/translator/request/claude-to-openai.ts"
|
||||
);
|
||||
const { claudeToOpenAIRequest } =
|
||||
await import("../../open-sse/translator/request/claude-to-openai.ts");
|
||||
|
||||
type Msg = { role: string; tool_call_id?: string; tool_calls?: { id?: string }[] };
|
||||
|
||||
@@ -92,3 +91,113 @@ test("#4385 keeps valid tool_results and drops orphans in the same user turn", (
|
||||
assert.equal(toolMsgs.length, 1);
|
||||
assert.equal(toolMsgs[0].tool_call_id, "tu_valid");
|
||||
});
|
||||
|
||||
// #4714: regression (v3.8.26+) — Claude Code issues parallel tool_use in one
|
||||
// assistant turn but their tool_result blocks arrive across SEPARATE user turns
|
||||
// with interleaved text. The translator emitted each role:"tool" message in the
|
||||
// position of its originating user turn, so a later tool_result ended up AFTER a
|
||||
// user message instead of adjacent to its assistant.tool_calls. The #4385 orphan
|
||||
// filter kept it (the id DID match an assistant tool_call) but never re-ordered
|
||||
// it, so OpenAI-compatible upstreams (deepseek, etc.) still rejected the request
|
||||
// with 400 "Messages with role 'tool' must be a response to a preceding message
|
||||
// with 'tool_calls'". fixMissingToolResponses also injected a bogus
|
||||
// "[No response received]" placeholder for the not-yet-adjacent tool_call.
|
||||
|
||||
// Helper: every role:"tool" must be immediately preceded by an assistant carrying
|
||||
// the matching tool_call id (or another tool message in the same group).
|
||||
function assertToolOrdering(messages: Msg[]) {
|
||||
let activeIds: Set<string> | null = null;
|
||||
for (const m of messages) {
|
||||
if (m.role === "assistant" && Array.isArray(m.tool_calls)) {
|
||||
activeIds = new Set(m.tool_calls.map((t) => String(t.id)));
|
||||
} else if (m.role === "tool") {
|
||||
assert.ok(
|
||||
activeIds && activeIds.has(String(m.tool_call_id)),
|
||||
`role:'tool' ${m.tool_call_id} is not preceded by a matching assistant tool_calls`
|
||||
);
|
||||
} else {
|
||||
activeIds = null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
test("#4714 regroups parallel tool_results split across user turns next to their assistant", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"deepseek/deepseek-v4-flash-free",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "do two things" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "calling tools" },
|
||||
{ type: "tool_use", id: "callA", name: "toolA", input: { x: 1 } },
|
||||
{ type: "tool_use", id: "callB", name: "toolB", input: { y: 2 } },
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{ type: "tool_result", tool_use_id: "callA", content: "result A" },
|
||||
{ type: "text", text: "and here's more context" },
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "callB", content: "result B" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const messages = result.messages as Msg[];
|
||||
// No upstream-rejecting ordering violations.
|
||||
assertToolOrdering(messages);
|
||||
|
||||
// Both real tool results survive, adjacent to the assistant, in tool_calls order.
|
||||
const assistantIdx = messages.findIndex(
|
||||
(m) => m.role === "assistant" && Array.isArray(m.tool_calls)
|
||||
);
|
||||
assert.equal((messages[assistantIdx + 1] as Msg).tool_call_id, "callA");
|
||||
assert.equal((messages[assistantIdx + 2] as Msg).tool_call_id, "callB");
|
||||
|
||||
// The real "result B" must NOT have been replaced by a placeholder.
|
||||
const toolB = messages.find((m) => m.role === "tool" && m.tool_call_id === "callB") as
|
||||
| (Msg & { content?: string })
|
||||
| undefined;
|
||||
assert.ok(toolB);
|
||||
assert.notEqual(toolB.content, "[No response received]");
|
||||
// Exactly one tool message per call id (no duplicate placeholder + real result).
|
||||
assert.equal(messages.filter((m) => m.role === "tool" && m.tool_call_id === "callB").length, 1);
|
||||
});
|
||||
|
||||
test("#4714 still inserts a placeholder for a genuinely unanswered parallel tool_call", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"deepseek/deepseek-v4-flash-free",
|
||||
{
|
||||
messages: [
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "tool_use", id: "ansA", name: "toolA", input: {} },
|
||||
{ type: "tool_use", id: "noAns", name: "toolB", input: {} },
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "ansA", content: "ok" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const messages = result.messages as Msg[];
|
||||
assertToolOrdering(messages);
|
||||
const placeholder = messages.find((m) => m.role === "tool" && m.tool_call_id === "noAns") as
|
||||
| (Msg & { content?: string })
|
||||
| undefined;
|
||||
assert.ok(placeholder, "missing tool_call must still get a placeholder response");
|
||||
assert.equal(placeholder.content, "[No response received]");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user