fix(agents): use alphanumeric-only tool call IDs for OpenRouter compatibility
Some providers like Mistral via OpenRouter require strictly alphanumeric tool call IDs. The error message indicates: "Tool call id was whatsapp_login_1768799841527_1 but must be a-z, A-Z, 0-9, with a length of 9." Changes: - Update sanitizeToolCallId to strip all non-alphanumeric characters (previously allowed underscores and hyphens) - Update makeUniqueToolId to use alphanumeric suffixes (x2, x3, etc.) instead of underscores - Update isValidCloudCodeAssistToolId to validate alphanumeric-only IDs - Update tests to reflect stricter sanitization Fixes #1359 Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
committed by
Peter Steinberger
parent
39b375e32b
commit
d0f9e22a4b
@@ -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 = [
|
||||
|
||||
@@ -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 = [
|
||||
|
||||
@@ -10,13 +10,15 @@ const _makeFile = (overrides: Partial<WorkspaceBootstrapFile>): 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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<AgentMessage, { role: "assistant" }>;
|
||||
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<AgentMessage, { role: "toolResult" }>;
|
||||
expect(result.toolCallId).toBe(toolCall.id);
|
||||
});
|
||||
|
||||
it("avoids collisions when sanitization would produce duplicate IDs", () => {
|
||||
const input = [
|
||||
{
|
||||
|
||||
@@ -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> }): 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<string, string>();
|
||||
const used = new Set<string>();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user