mirror of https://github.com/openclaw/openclaw.git
Deduplicate repeated tool call IDs for OpenAI-compatible APIs (#40996)
Merged via squash.
Prepared head SHA: 38d8048359
Co-authored-by: xaeon2026 <264572156+xaeon2026@users.noreply.github.com>
Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com>
Reviewed-by: @frankekn
This commit is contained in:
parent
9d3e653ec9
commit
5c5c64b612
|
|
@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Models/openai-completions: default non-native OpenAI-compatible providers to omit tool-definition `strict` fields unless users explicitly opt back in, so tool calling keeps working on providers that reject that option. (#45497) Thanks @sahancava.
|
||||
- WhatsApp/reconnect: restore the append recency filter in the extension inbox monitor and handle protobuf `Long` timestamps correctly, so fresh post-reconnect append messages are processed while stale history sync stays suppressed. (#42588) thanks @MonkeyLeeT.
|
||||
- WhatsApp/login: wait for pending creds writes before reopening after Baileys `515` pairing restarts in both QR login and `channels login` flows, and keep the restart coverage pinned to the real wrapped error shape plus per-account creds queues. (#27910) Thanks @asyncjason.
|
||||
- Agents/openai-compatible tool calls: deduplicate repeated tool call ids across live assistant messages and replayed history so OpenAI-compatible backends no longer reject duplicate `tool_call_id` values with HTTP 400. (#40996) Thanks @xaeon2026.
|
||||
|
||||
### Fixes
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core";
|
|||
import type { AssistantMessage, UserMessage, Usage } from "@mariozechner/pi-ai";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
expectOpenAIResponsesStrictSanitizeCall,
|
||||
loadSanitizeSessionHistoryWithCleanMocks,
|
||||
makeMockSessionManager,
|
||||
makeInMemorySessionManager,
|
||||
|
|
@ -247,7 +248,24 @@ describe("sanitizeSessionHistory", () => {
|
|||
expect(result).toEqual(mockMessages);
|
||||
});
|
||||
|
||||
it("passes simple user-only history through for openai-completions", async () => {
|
||||
it("sanitizes tool call ids for OpenAI-compatible responses providers", async () => {
|
||||
setNonGoogleModelApi();
|
||||
|
||||
await sanitizeSessionHistory({
|
||||
messages: mockMessages,
|
||||
modelApi: "openai-responses",
|
||||
provider: "custom",
|
||||
sessionManager: mockSessionManager,
|
||||
sessionId: TEST_SESSION_ID,
|
||||
});
|
||||
|
||||
expectOpenAIResponsesStrictSanitizeCall(
|
||||
mockedHelpers.sanitizeSessionMessagesImages,
|
||||
mockMessages,
|
||||
);
|
||||
});
|
||||
|
||||
it("sanitizes tool call ids for openai-completions", async () => {
|
||||
setNonGoogleModelApi();
|
||||
|
||||
const result = await sanitizeSessionHistory({
|
||||
|
|
|
|||
|
|
@ -702,6 +702,26 @@ describe("wrapStreamFnTrimToolCallNames", () => {
|
|||
expect(finalToolCall.name).toBe("read");
|
||||
expect(finalToolCall.id).toBe("call_42");
|
||||
});
|
||||
|
||||
it("reassigns duplicate tool call ids within a message to unique fallbacks", async () => {
|
||||
const finalToolCallA = { type: "toolCall", name: " read ", id: " edit:22 " };
|
||||
const finalToolCallB = { type: "toolCall", name: " write ", id: "edit:22" };
|
||||
const finalMessage = { role: "assistant", content: [finalToolCallA, finalToolCallB] };
|
||||
const baseFn = vi.fn(() =>
|
||||
createFakeStream({
|
||||
events: [],
|
||||
resultMessage: finalMessage,
|
||||
}),
|
||||
);
|
||||
|
||||
const stream = await invokeWrappedStream(baseFn);
|
||||
await stream.result();
|
||||
|
||||
expect(finalToolCallA.name).toBe("read");
|
||||
expect(finalToolCallB.name).toBe("write");
|
||||
expect(finalToolCallA.id).toBe("edit:22");
|
||||
expect(finalToolCallB.id).toBe("call_auto_1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("wrapStreamFnRepairMalformedToolCallArguments", () => {
|
||||
|
|
|
|||
|
|
@ -667,6 +667,7 @@ function normalizeToolCallIdsInMessage(message: unknown): void {
|
|||
}
|
||||
|
||||
let fallbackIndex = 1;
|
||||
const assignedIds = new Set<string>();
|
||||
for (const block of content) {
|
||||
if (!block || typeof block !== "object") {
|
||||
continue;
|
||||
|
|
@ -678,20 +679,23 @@ function normalizeToolCallIdsInMessage(message: unknown): void {
|
|||
if (typeof typedBlock.id === "string") {
|
||||
const trimmedId = typedBlock.id.trim();
|
||||
if (trimmedId) {
|
||||
if (typedBlock.id !== trimmedId) {
|
||||
typedBlock.id = trimmedId;
|
||||
if (!assignedIds.has(trimmedId)) {
|
||||
if (typedBlock.id !== trimmedId) {
|
||||
typedBlock.id = trimmedId;
|
||||
}
|
||||
assignedIds.add(trimmedId);
|
||||
continue;
|
||||
}
|
||||
usedIds.add(trimmedId);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
let fallbackId = "";
|
||||
while (!fallbackId || usedIds.has(fallbackId)) {
|
||||
while (!fallbackId || usedIds.has(fallbackId) || assignedIds.has(fallbackId)) {
|
||||
fallbackId = `call_auto_${fallbackIndex++}`;
|
||||
}
|
||||
typedBlock.id = fallbackId;
|
||||
usedIds.add(fallbackId);
|
||||
assignedIds.add(fallbackId);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -29,6 +29,54 @@ const buildDuplicateIdCollisionInput = () =>
|
|||
},
|
||||
]);
|
||||
|
||||
const buildRepeatedRawIdInput = () =>
|
||||
castAgentMessages([
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "toolCall", id: "edit:22", name: "edit", arguments: {} },
|
||||
{ type: "toolCall", id: "edit:22", name: "edit", arguments: {} },
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "toolResult",
|
||||
toolCallId: "edit:22",
|
||||
toolName: "edit",
|
||||
content: [{ type: "text", text: "one" }],
|
||||
},
|
||||
{
|
||||
role: "toolResult",
|
||||
toolCallId: "edit:22",
|
||||
toolName: "edit",
|
||||
content: [{ type: "text", text: "two" }],
|
||||
},
|
||||
]);
|
||||
|
||||
const buildRepeatedSharedToolResultIdInput = () =>
|
||||
castAgentMessages([
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "toolCall", id: "edit:22", name: "edit", arguments: {} },
|
||||
{ type: "toolCall", id: "edit:22", name: "edit", arguments: {} },
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "toolResult",
|
||||
toolCallId: "edit:22",
|
||||
toolUseId: "edit:22",
|
||||
toolName: "edit",
|
||||
content: [{ type: "text", text: "one" }],
|
||||
},
|
||||
{
|
||||
role: "toolResult",
|
||||
toolCallId: "edit:22",
|
||||
toolUseId: "edit:22",
|
||||
toolName: "edit",
|
||||
content: [{ type: "text", text: "two" }],
|
||||
},
|
||||
]);
|
||||
|
||||
function expectCollisionIdsRemainDistinct(
|
||||
out: AgentMessage[],
|
||||
mode: "strict" | "strict9",
|
||||
|
|
@ -111,6 +159,26 @@ describe("sanitizeToolCallIdsForCloudCodeAssist", () => {
|
|||
expectCollisionIdsRemainDistinct(out, "strict");
|
||||
});
|
||||
|
||||
it("reuses one rewritten id when a tool result carries matching toolCallId and toolUseId", () => {
|
||||
const input = buildRepeatedSharedToolResultIdInput();
|
||||
|
||||
const out = sanitizeToolCallIdsForCloudCodeAssist(input);
|
||||
expect(out).not.toBe(input);
|
||||
const { aId, bId } = expectCollisionIdsRemainDistinct(out, "strict");
|
||||
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }> & { toolUseId?: string };
|
||||
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }> & { toolUseId?: string };
|
||||
expect(r1.toolUseId).toBe(aId);
|
||||
expect(r2.toolUseId).toBe(bId);
|
||||
});
|
||||
|
||||
it("assigns distinct IDs when identical raw tool call ids repeat", () => {
|
||||
const input = buildRepeatedRawIdInput();
|
||||
|
||||
const out = sanitizeToolCallIdsForCloudCodeAssist(input);
|
||||
expect(out).not.toBe(input);
|
||||
expectCollisionIdsRemainDistinct(out, "strict");
|
||||
});
|
||||
|
||||
it("caps tool call IDs at 40 chars while preserving uniqueness", () => {
|
||||
const longA = `call_${"a".repeat(60)}`;
|
||||
const longB = `call_${"a".repeat(59)}b`;
|
||||
|
|
@ -181,6 +249,16 @@ describe("sanitizeToolCallIdsForCloudCodeAssist", () => {
|
|||
expect(aId).not.toMatch(/[_-]/);
|
||||
expect(bId).not.toMatch(/[_-]/);
|
||||
});
|
||||
|
||||
it("assigns distinct strict IDs when identical raw tool call ids repeat", () => {
|
||||
const input = buildRepeatedRawIdInput();
|
||||
|
||||
const out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict");
|
||||
expect(out).not.toBe(input);
|
||||
const { aId, bId } = expectCollisionIdsRemainDistinct(out, "strict");
|
||||
expect(aId).not.toMatch(/[_-]/);
|
||||
expect(bId).not.toMatch(/[_-]/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("strict9 mode (Mistral tool call IDs)", () => {
|
||||
|
|
@ -231,5 +309,27 @@ describe("sanitizeToolCallIdsForCloudCodeAssist", () => {
|
|||
expect(aId.length).toBe(9);
|
||||
expect(bId.length).toBe(9);
|
||||
});
|
||||
|
||||
it("assigns distinct strict9 IDs when identical raw tool call ids repeat", () => {
|
||||
const input = buildRepeatedRawIdInput();
|
||||
|
||||
const out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict9");
|
||||
expect(out).not.toBe(input);
|
||||
const { aId, bId } = expectCollisionIdsRemainDistinct(out, "strict9");
|
||||
expect(aId.length).toBe(9);
|
||||
expect(bId.length).toBe(9);
|
||||
});
|
||||
|
||||
it("reuses one rewritten strict9 id when a tool result carries matching toolCallId and toolUseId", () => {
|
||||
const input = buildRepeatedSharedToolResultIdInput();
|
||||
|
||||
const out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict9");
|
||||
expect(out).not.toBe(input);
|
||||
const { aId, bId } = expectCollisionIdsRemainDistinct(out, "strict9");
|
||||
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }> & { toolUseId?: string };
|
||||
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }> & { toolUseId?: string };
|
||||
expect(r1.toolUseId).toBe(aId);
|
||||
expect(r2.toolUseId).toBe(bId);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -144,9 +144,55 @@ function makeUniqueToolId(params: { id: string; used: Set<string>; mode: ToolCal
|
|||
return `${candidate.slice(0, MAX_LEN - ts.length)}${ts}`;
|
||||
}
|
||||
|
||||
function createOccurrenceAwareResolver(mode: ToolCallIdMode): {
|
||||
resolveAssistantId: (id: string) => string;
|
||||
resolveToolResultId: (id: string) => string;
|
||||
} {
|
||||
const used = new Set<string>();
|
||||
const assistantOccurrences = new Map<string, number>();
|
||||
const orphanToolResultOccurrences = new Map<string, number>();
|
||||
const pendingByRawId = new Map<string, string[]>();
|
||||
|
||||
const allocate = (seed: string): string => {
|
||||
const next = makeUniqueToolId({ id: seed, used, mode });
|
||||
used.add(next);
|
||||
return next;
|
||||
};
|
||||
|
||||
const resolveAssistantId = (id: string): string => {
|
||||
const occurrence = (assistantOccurrences.get(id) ?? 0) + 1;
|
||||
assistantOccurrences.set(id, occurrence);
|
||||
const next = allocate(occurrence === 1 ? id : `${id}:${occurrence}`);
|
||||
const pending = pendingByRawId.get(id);
|
||||
if (pending) {
|
||||
pending.push(next);
|
||||
} else {
|
||||
pendingByRawId.set(id, [next]);
|
||||
}
|
||||
return next;
|
||||
};
|
||||
|
||||
const resolveToolResultId = (id: string): string => {
|
||||
const pending = pendingByRawId.get(id);
|
||||
if (pending && pending.length > 0) {
|
||||
const next = pending.shift()!;
|
||||
if (pending.length === 0) {
|
||||
pendingByRawId.delete(id);
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
const occurrence = (orphanToolResultOccurrences.get(id) ?? 0) + 1;
|
||||
orphanToolResultOccurrences.set(id, occurrence);
|
||||
return allocate(`${id}:tool_result:${occurrence}`);
|
||||
};
|
||||
|
||||
return { resolveAssistantId, resolveToolResultId };
|
||||
}
|
||||
|
||||
function rewriteAssistantToolCallIds(params: {
|
||||
message: Extract<AgentMessage, { role: "assistant" }>;
|
||||
resolve: (id: string) => string;
|
||||
resolveId: (id: string) => string;
|
||||
}): Extract<AgentMessage, { role: "assistant" }> {
|
||||
const content = params.message.content;
|
||||
if (!Array.isArray(content)) {
|
||||
|
|
@ -168,7 +214,7 @@ function rewriteAssistantToolCallIds(params: {
|
|||
) {
|
||||
return block;
|
||||
}
|
||||
const nextId = params.resolve(id);
|
||||
const nextId = params.resolveId(id);
|
||||
if (nextId === id) {
|
||||
return block;
|
||||
}
|
||||
|
|
@ -184,7 +230,7 @@ function rewriteAssistantToolCallIds(params: {
|
|||
|
||||
function rewriteToolResultIds(params: {
|
||||
message: Extract<AgentMessage, { role: "toolResult" }>;
|
||||
resolve: (id: string) => string;
|
||||
resolveId: (id: string) => string;
|
||||
}): Extract<AgentMessage, { role: "toolResult" }> {
|
||||
const toolCallId =
|
||||
typeof params.message.toolCallId === "string" && params.message.toolCallId
|
||||
|
|
@ -192,9 +238,14 @@ function rewriteToolResultIds(params: {
|
|||
: undefined;
|
||||
const toolUseId = (params.message as { toolUseId?: unknown }).toolUseId;
|
||||
const toolUseIdStr = typeof toolUseId === "string" && toolUseId ? toolUseId : undefined;
|
||||
const sharedRawId =
|
||||
toolCallId && toolUseIdStr && toolCallId === toolUseIdStr ? toolCallId : undefined;
|
||||
|
||||
const nextToolCallId = toolCallId ? params.resolve(toolCallId) : undefined;
|
||||
const nextToolUseId = toolUseIdStr ? params.resolve(toolUseIdStr) : undefined;
|
||||
const sharedResolvedId = sharedRawId ? params.resolveId(sharedRawId) : undefined;
|
||||
const nextToolCallId =
|
||||
sharedResolvedId ?? (toolCallId ? params.resolveId(toolCallId) : undefined);
|
||||
const nextToolUseId =
|
||||
sharedResolvedId ?? (toolUseIdStr ? params.resolveId(toolUseIdStr) : undefined);
|
||||
|
||||
if (nextToolCallId === toolCallId && nextToolUseId === toolUseIdStr) {
|
||||
return params.message;
|
||||
|
|
@ -219,21 +270,11 @@ export function sanitizeToolCallIdsForCloudCodeAssist(
|
|||
): AgentMessage[] {
|
||||
// Strict mode: only [a-zA-Z0-9]
|
||||
// Strict9 mode: only [a-zA-Z0-9], length 9 (Mistral tool call requirement)
|
||||
// Sanitization can introduce collisions (e.g. `a|b` and `a:b` -> `ab`).
|
||||
// Fix by applying a stable, transcript-wide mapping and de-duping via suffix.
|
||||
const map = new Map<string, string>();
|
||||
const used = new Set<string>();
|
||||
|
||||
const resolve = (id: string) => {
|
||||
const existing = map.get(id);
|
||||
if (existing) {
|
||||
return existing;
|
||||
}
|
||||
const next = makeUniqueToolId({ id, used, mode });
|
||||
map.set(id, next);
|
||||
used.add(next);
|
||||
return next;
|
||||
};
|
||||
// Sanitization can introduce collisions, and some providers also reject raw
|
||||
// duplicate tool-call IDs. Track assistant occurrences in-order so repeated
|
||||
// raw IDs receive distinct rewritten IDs, while matching tool results consume
|
||||
// the same rewritten IDs in encounter order.
|
||||
const { resolveAssistantId, resolveToolResultId } = createOccurrenceAwareResolver(mode);
|
||||
|
||||
let changed = false;
|
||||
const out = messages.map((msg) => {
|
||||
|
|
@ -244,7 +285,7 @@ export function sanitizeToolCallIdsForCloudCodeAssist(
|
|||
if (role === "assistant") {
|
||||
const next = rewriteAssistantToolCallIds({
|
||||
message: msg as Extract<AgentMessage, { role: "assistant" }>,
|
||||
resolve,
|
||||
resolveId: resolveAssistantId,
|
||||
});
|
||||
if (next !== msg) {
|
||||
changed = true;
|
||||
|
|
@ -254,7 +295,7 @@ export function sanitizeToolCallIdsForCloudCodeAssist(
|
|||
if (role === "toolResult") {
|
||||
const next = rewriteToolResultIds({
|
||||
message: msg as Extract<AgentMessage, { role: "toolResult" }>,
|
||||
resolve,
|
||||
resolveId: resolveToolResultId,
|
||||
});
|
||||
if (next !== msg) {
|
||||
changed = true;
|
||||
|
|
|
|||
|
|
@ -78,7 +78,10 @@ export function resolveTranscriptPolicy(params: {
|
|||
provider,
|
||||
modelId,
|
||||
});
|
||||
const requiresOpenAiCompatibleToolIdSanitization = params.modelApi === "openai-completions";
|
||||
const requiresOpenAiCompatibleToolIdSanitization =
|
||||
params.modelApi === "openai-completions" ||
|
||||
(!isOpenAi &&
|
||||
(params.modelApi === "openai-responses" || params.modelApi === "openai-codex-responses"));
|
||||
|
||||
// Anthropic Claude endpoints can reject replayed `thinking` blocks unless the
|
||||
// original signatures are preserved byte-for-byte. Drop them at send-time to
|
||||
|
|
|
|||
Loading…
Reference in New Issue