diff --git a/CHANGELOG.md b/CHANGELOG.md index 023d9edea79..bd2212d5174 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/agents/pi-embedded-runner.sanitize-session-history.test.ts b/src/agents/pi-embedded-runner.sanitize-session-history.test.ts index 2003523e03f..438b46bb971 100644 --- a/src/agents/pi-embedded-runner.sanitize-session-history.test.ts +++ b/src/agents/pi-embedded-runner.sanitize-session-history.test.ts @@ -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({ diff --git a/src/agents/pi-embedded-runner/run/attempt.test.ts b/src/agents/pi-embedded-runner/run/attempt.test.ts index ef88e04ef46..1953099cf7b 100644 --- a/src/agents/pi-embedded-runner/run/attempt.test.ts +++ b/src/agents/pi-embedded-runner/run/attempt.test.ts @@ -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", () => { diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index ef5a63cdcd1..b02e8a59fb8 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -667,6 +667,7 @@ function normalizeToolCallIdsInMessage(message: unknown): void { } let fallbackIndex = 1; + const assignedIds = new Set(); 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); } } diff --git a/src/agents/tool-call-id.test.ts b/src/agents/tool-call-id.test.ts index dec3d37e9d8..ced9c7ee8a5 100644 --- a/src/agents/tool-call-id.test.ts +++ b/src/agents/tool-call-id.test.ts @@ -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 & { toolUseId?: string }; + const r2 = out[2] as Extract & { 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 & { toolUseId?: string }; + const r2 = out[2] as Extract & { toolUseId?: string }; + expect(r1.toolUseId).toBe(aId); + expect(r2.toolUseId).toBe(bId); + }); }); }); diff --git a/src/agents/tool-call-id.ts b/src/agents/tool-call-id.ts index e30236e6e82..c7c68994458 100644 --- a/src/agents/tool-call-id.ts +++ b/src/agents/tool-call-id.ts @@ -144,9 +144,55 @@ function makeUniqueToolId(params: { id: string; used: Set; 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(); + const assistantOccurrences = new Map(); + const orphanToolResultOccurrences = new Map(); + const pendingByRawId = new Map(); + + 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; - resolve: (id: string) => string; + resolveId: (id: string) => string; }): Extract { 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; - resolve: (id: string) => string; + resolveId: (id: string) => string; }): Extract { 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(); - const used = new Set(); - - 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, - 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, - resolve, + resolveId: resolveToolResultId, }); if (next !== msg) { changed = true; diff --git a/src/agents/transcript-policy.ts b/src/agents/transcript-policy.ts index 46795bad1bc..784770f2e28 100644 --- a/src/agents/transcript-policy.ts +++ b/src/agents/transcript-policy.ts @@ -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