diff --git a/src/agents/pi-embedded-helpers.sanitize-session-messages-images.keeps-tool-call-tool-result-ids-unchanged.test.ts b/src/agents/pi-embedded-helpers.sanitize-session-messages-images.keeps-tool-call-tool-result-ids-unchanged.test.ts index f8822feea..22a885e9f 100644 --- a/src/agents/pi-embedded-helpers.sanitize-session-messages-images.keeps-tool-call-tool-result-ids-unchanged.test.ts +++ b/src/agents/pi-embedded-helpers.sanitize-session-messages-images.keeps-tool-call-tool-result-ids-unchanged.test.ts @@ -50,7 +50,7 @@ describe("sanitizeSessionMessagesImages", () => { expect(toolResult.role).toBe("toolResult"); expect(toolResult.toolCallId).toBe("call_123|fc_456"); }); - it("sanitizes tool call + tool result IDs when enabled", async () => { + it("sanitizes tool call + tool result IDs when enabled (alphanumeric only)", async () => { const input = [ { role: "assistant", @@ -82,14 +82,15 @@ describe("sanitizeSessionMessagesImages", () => { const toolCall = (assistant.content as Array<{ type?: string; id?: string }>).find( (b) => b.type === "toolCall", ); - expect(toolCall?.id).toBe("call_123_fc_456"); + // Sanitization strips all non-alphanumeric characters for Mistral/OpenRouter compatibility + expect(toolCall?.id).toBe("call123fc456"); const toolResult = out[1] as unknown as { role?: string; toolCallId?: string; }; expect(toolResult.role).toBe("toolResult"); - expect(toolResult.toolCallId).toBe("call_123_fc_456"); + expect(toolResult.toolCallId).toBe("call123fc456"); }); it("drops assistant blocks after a tool call when enforceToolCallLast is enabled", async () => { const input = [ diff --git a/src/agents/pi-embedded-helpers.sanitize-session-messages-images.removes-empty-assistant-text-blocks-but-preserves.test.ts b/src/agents/pi-embedded-helpers.sanitize-session-messages-images.removes-empty-assistant-text-blocks-but-preserves.test.ts index bf2be6ef5..ff14577bf 100644 --- a/src/agents/pi-embedded-helpers.sanitize-session-messages-images.removes-empty-assistant-text-blocks-but-preserves.test.ts +++ b/src/agents/pi-embedded-helpers.sanitize-session-messages-images.removes-empty-assistant-text-blocks-but-preserves.test.ts @@ -30,7 +30,7 @@ describe("sanitizeSessionMessagesImages", () => { expect(content).toHaveLength(1); expect((content as Array<{ type?: string }>)[0]?.type).toBe("toolCall"); }); - it("sanitizes tool ids for assistant blocks and tool results when enabled", async () => { + it("sanitizes tool ids for assistant blocks and tool results when enabled (alphanumeric only)", async () => { const input = [ { role: "assistant", @@ -55,12 +55,13 @@ describe("sanitizeSessionMessagesImages", () => { sanitizeToolCallIds: true, }); + // Sanitization strips all non-alphanumeric characters for Mistral/OpenRouter compatibility const assistant = out[0] as { content?: Array<{ id?: string }> }; - expect(assistant.content?.[0]?.id).toBe("call_abc_item_123"); - expect(assistant.content?.[1]?.id).toBe("call_abc_item_456"); + expect(assistant.content?.[0]?.id).toBe("callabcitem123"); + expect(assistant.content?.[1]?.id).toBe("callabcitem456"); const toolResult = out[1] as { toolUseId?: string }; - expect(toolResult.toolUseId).toBe("call_abc_item_123"); + expect(toolResult.toolUseId).toBe("callabcitem123"); }); it("filters whitespace-only assistant text blocks", async () => { const input = [ diff --git a/src/agents/pi-embedded-helpers.sanitizetoolcallid.test.ts b/src/agents/pi-embedded-helpers.sanitizetoolcallid.test.ts index 19fec5aaa..b7fe91210 100644 --- a/src/agents/pi-embedded-helpers.sanitizetoolcallid.test.ts +++ b/src/agents/pi-embedded-helpers.sanitizetoolcallid.test.ts @@ -10,13 +10,15 @@ const _makeFile = (overrides: Partial): WorkspaceBootstr ...overrides, }); describe("sanitizeToolCallId", () => { - it("keeps valid tool call IDs", () => { - expect(sanitizeToolCallId("call_abc-123")).toBe("call_abc-123"); + it("keeps valid alphanumeric tool call IDs", () => { + expect(sanitizeToolCallId("callabc123")).toBe("callabc123"); }); - it("replaces invalid characters with underscores", () => { - expect(sanitizeToolCallId("call_abc|item:456")).toBe("call_abc_item_456"); + it("strips non-alphanumeric characters (Mistral/OpenRouter compatibility)", () => { + expect(sanitizeToolCallId("call_abc-123")).toBe("callabc123"); + expect(sanitizeToolCallId("call_abc|item:456")).toBe("callabcitem456"); + expect(sanitizeToolCallId("whatsapp_login_1768799841527_1")).toBe("whatsapplogin17687998415271"); }); it("returns default for empty IDs", () => { - expect(sanitizeToolCallId("")).toBe("default_tool_id"); + expect(sanitizeToolCallId("")).toBe("defaulttoolid"); }); }); diff --git a/src/agents/tool-call-id.test.ts b/src/agents/tool-call-id.test.ts index 0d9fb1341..a6e85d327 100644 --- a/src/agents/tool-call-id.test.ts +++ b/src/agents/tool-call-id.test.ts @@ -7,15 +7,15 @@ import { } from "./tool-call-id.js"; describe("sanitizeToolCallIdsForCloudCodeAssist", () => { - it("is a no-op for already-valid non-colliding IDs", () => { + it("is a no-op for already-valid non-colliding alphanumeric IDs", () => { const input = [ { role: "assistant", - content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }], + content: [{ type: "toolCall", id: "call1", name: "read", arguments: {} }], }, { role: "toolResult", - toolCallId: "call_1", + toolCallId: "call1", toolName: "read", content: [{ type: "text", text: "ok" }], }, @@ -25,6 +25,35 @@ describe("sanitizeToolCallIdsForCloudCodeAssist", () => { expect(out).toBe(input); }); + it("strips underscores from tool call IDs (Mistral/OpenRouter compatibility)", () => { + const input = [ + { + role: "assistant", + content: [ + { type: "toolCall", id: "whatsapp_login_1768799841527_1", name: "login", arguments: {} }, + ], + }, + { + role: "toolResult", + toolCallId: "whatsapp_login_1768799841527_1", + toolName: "login", + content: [{ type: "text", text: "ok" }], + }, + ] satisfies AgentMessage[]; + + const out = sanitizeToolCallIdsForCloudCodeAssist(input); + expect(out).not.toBe(input); + + const assistant = out[0] as Extract; + const toolCall = assistant.content?.[0] as { id?: string }; + // ID should be alphanumeric only, no underscores + expect(toolCall.id).toBe("whatsapplogin17687998415271"); + expect(isValidCloudCodeAssistToolId(toolCall.id as string)).toBe(true); + + const result = out[1] as Extract; + expect(result.toolCallId).toBe(toolCall.id); + }); + it("avoids collisions when sanitization would produce duplicate IDs", () => { const input = [ { diff --git a/src/agents/tool-call-id.ts b/src/agents/tool-call-id.ts index c4aec7e5b..736454536 100644 --- a/src/agents/tool-call-id.ts +++ b/src/agents/tool-call-id.ts @@ -3,20 +3,19 @@ import { createHash } from "node:crypto"; import type { AgentMessage } from "@mariozechner/pi-agent-core"; export function sanitizeToolCallId(id: string): string { - if (!id || typeof id !== "string") return "default_tool_id"; + if (!id || typeof id !== "string") return "defaulttoolid"; - const cloudCodeAssistPatternReplacement = id.replace(/[^a-zA-Z0-9_-]/g, "_"); - const trimmedInvalidStartChars = cloudCodeAssistPatternReplacement.replace( - /^[^a-zA-Z0-9_-]+/, - "", - ); + // Some providers (e.g. Mistral via OpenRouter) require strictly alphanumeric tool call IDs. + // Strip all non-alphanumeric characters to ensure maximum compatibility. + const alphanumericOnly = id.replace(/[^a-zA-Z0-9]/g, ""); - return trimmedInvalidStartChars.length > 0 ? trimmedInvalidStartChars : "sanitized_tool_id"; + return alphanumericOnly.length > 0 ? alphanumericOnly : "sanitizedtoolid"; } export function isValidCloudCodeAssistToolId(id: string): boolean { if (!id || typeof id !== "string") return false; - return /^[a-zA-Z0-9_-]+$/.test(id); + // Strictly alphanumeric for maximum provider compatibility (e.g. Mistral via OpenRouter). + return /^[a-zA-Z0-9]+$/.test(id); } function shortHash(text: string): string { @@ -29,19 +28,20 @@ function makeUniqueToolId(params: { id: string; used: Set }): string { const base = sanitizeToolCallId(params.id).slice(0, MAX_LEN); if (!params.used.has(base)) return base; + // Use alphanumeric-only suffixes to maintain strict compatibility. const hash = shortHash(params.id); - const maxBaseLen = MAX_LEN - 1 - hash.length; + const maxBaseLen = MAX_LEN - hash.length; const clippedBase = base.length > maxBaseLen ? base.slice(0, maxBaseLen) : base; - const candidate = `${clippedBase}_${hash}`; + const candidate = `${clippedBase}${hash}`; if (!params.used.has(candidate)) return candidate; for (let i = 2; i < 1000; i += 1) { - const suffix = `_${i}`; + const suffix = `x${i}`; const next = `${candidate.slice(0, MAX_LEN - suffix.length)}${suffix}`; if (!params.used.has(next)) return next; } - const ts = `_${Date.now()}`; + const ts = `t${Date.now()}`; return `${candidate.slice(0, MAX_LEN - ts.length)}${ts}`; } @@ -101,9 +101,10 @@ function rewriteToolResultIds(params: { } export function sanitizeToolCallIdsForCloudCodeAssist(messages: AgentMessage[]): AgentMessage[] { - // Cloud Code Assist requires tool IDs matching ^[a-zA-Z0-9_-]+$. - // Sanitization can introduce collisions (e.g. `a|b` and `a:b` -> `a_b`). - // Fix by applying a stable, transcript-wide mapping and de-duping via suffix. + // Some providers (e.g. Mistral via OpenRouter) require strictly alphanumeric tool IDs. + // Use ^[a-zA-Z0-9]+$ pattern for maximum compatibility across all providers. + // Sanitization can introduce collisions (e.g. `a|b` and `a:b` -> `ab`). + // Fix by applying a stable, transcript-wide mapping and de-duping via hash suffix. const map = new Map(); const used = new Set();