let5see/clawdbot
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

Merge pull request #1372 from zerone0x/fix/openrouter-tool-call-id-alphanumeric

Browse Source 
fix(agents): use alphanumeric-only tool call IDs for OpenRouter compatibility
This commit is contained in:
Peter Steinberger
2026-01-22 01:17:16 +00:00
committed by GitHub
parent 758f30eb7d f5cec1dd8b
commit 9780748bbb
14 changed files with 529 additions and 160 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -29,6 +29,7 @@ Docs: https://docs.clawd.bot
### Fixes
- Nodes/macOS: prompt on allowlist miss for node exec approvals, persist allowlist decisions, and flatten node invoke errors. (#1394) Thanks @ngutman.
- Gateway: keep auto bind loopback-first and add explicit tailnet binding to avoid Tailscale taking over local UI. (#1380)
- Agents: enforce 9-char alphanumeric tool call ids for Mistral providers. (#1372) Thanks @zerone0x.
- Embedded runner: persist injected history images so attachments aren’t reloaded each turn. (#1374) Thanks @Nicell.
- Nodes tool: include agent/node/gateway context in tool failure logs to speed approval debugging.
- macOS: exec approvals now respect wildcard agent allowlists (`*`).

56
src/agents/pi-embedded-helpers.sanitize-session-messages-images.keeps-tool-call-tool-result-ids-unchanged.test.ts
View File

@@ -1,15 +1,7 @@
import type { AgentMessage } from "@mariozechner/pi-agent-core";
import { describe, expect, it } from "vitest";
import { sanitizeSessionMessagesImages } from "./pi-embedded-helpers.js";
import { DEFAULT_AGENTS_FILENAME } from "./workspace.js";
const _makeFile = (overrides: Partial<WorkspaceBootstrapFile>): WorkspaceBootstrapFile => ({
name: DEFAULT_AGENTS_FILENAME,
path: "/tmp/AGENTS.md",
content: "",
missing: false,
...overrides,
});
describe("sanitizeSessionMessagesImages", () => {
it("keeps tool call + tool result IDs unchanged by default", async () => {
const input = [
@@ -50,7 +42,8 @@ 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 in standard mode (preserves underscores)", async () => {
const input = [
{
role: "assistant",
@@ -82,6 +75,7 @@ describe("sanitizeSessionMessagesImages", () => {
const toolCall = (assistant.content as Array<{ type?: string; id?: string }>).find(
(b) => b.type === "toolCall",
);
// Standard mode preserves underscores for readability, replaces invalid chars
expect(toolCall?.id).toBe("call_123_fc_456");
const toolResult = out[1] as unknown as {
@@ -91,6 +85,50 @@ describe("sanitizeSessionMessagesImages", () => {
expect(toolResult.role).toBe("toolResult");
expect(toolResult.toolCallId).toBe("call_123_fc_456");
});
it("sanitizes tool call + tool result IDs in strict mode (alphanumeric only)", async () => {
const input = [
{
role: "assistant",
content: [
{
type: "toolCall",
id: "call_123|fc_456",
name: "read",
arguments: { path: "package.json" },
},
],
},
{
role: "toolResult",
toolCallId: "call_123|fc_456",
toolName: "read",
content: [{ type: "text", text: "ok" }],
isError: false,
},
] satisfies AgentMessage[];
const out = await sanitizeSessionMessagesImages(input, "test", {
sanitizeToolCallIds: true,
toolCallIdMode: "strict",
});
const assistant = out[0] as unknown as { role?: string; content?: unknown };
expect(assistant.role).toBe("assistant");
expect(Array.isArray(assistant.content)).toBe(true);
const toolCall = (assistant.content as Array<{ type?: string; id?: string }>).find(
(b) => b.type === "toolCall",
);
// Strict mode strips all non-alphanumeric characters
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("call123fc456");
});
it("drops assistant blocks after a tool call when enforceToolCallLast is enabled", async () => {
const input = [
{

47
src/agents/pi-embedded-helpers.sanitize-session-messages-images.removes-empty-assistant-text-blocks-but-preserves.test.ts
View File

@@ -1,15 +1,7 @@
import type { AgentMessage } from "@mariozechner/pi-agent-core";
import { describe, expect, it } from "vitest";
import { sanitizeSessionMessagesImages } from "./pi-embedded-helpers.js";
import { DEFAULT_AGENTS_FILENAME } from "./workspace.js";
const _makeFile = (overrides: Partial<WorkspaceBootstrapFile>): WorkspaceBootstrapFile => ({
name: DEFAULT_AGENTS_FILENAME,
path: "/tmp/AGENTS.md",
content: "",
missing: false,
...overrides,
});
describe("sanitizeSessionMessagesImages", () => {
it("removes empty assistant text blocks but preserves tool calls", async () => {
const input = [
@@ -30,7 +22,8 @@ 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 in standard mode (preserves underscores)", async () => {
const input = [
{
role: "assistant",
@@ -55,6 +48,7 @@ describe("sanitizeSessionMessagesImages", () => {
sanitizeToolCallIds: true,
});
// Standard mode preserves underscores for readability
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");
@@ -62,6 +56,41 @@ describe("sanitizeSessionMessagesImages", () => {
const toolResult = out[1] as { toolUseId?: string };
expect(toolResult.toolUseId).toBe("call_abc_item_123");
});
it("sanitizes tool ids in strict mode (alphanumeric only)", async () => {
const input = [
{
role: "assistant",
content: [
{ type: "toolUse", id: "call_abc|item:123", name: "test", input: {} },
{
type: "toolCall",
id: "call_abc|item:456",
name: "exec",
arguments: {},
},
],
},
{
role: "toolResult",
toolUseId: "call_abc|item:123",
content: [{ type: "text", text: "ok" }],
},
] satisfies AgentMessage[];
const out = await sanitizeSessionMessagesImages(input, "test", {
sanitizeToolCallIds: true,
toolCallIdMode: "strict",
});
// Strict mode strips all non-alphanumeric characters
const assistant = out[0] as { content?: Array<{ id?: string }> };
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("callabcitem123");
});
it("filters whitespace-only assistant text blocks", async () => {
const input = [
{

49
src/agents/pi-embedded-helpers.sanitizetoolcallid.test.ts
View File

@@ -1,22 +1,43 @@
import { describe, expect, it } from "vitest";
import { sanitizeToolCallId } from "./pi-embedded-helpers.js";
import { DEFAULT_AGENTS_FILENAME } from "./workspace.js";
const _makeFile = (overrides: Partial<WorkspaceBootstrapFile>): WorkspaceBootstrapFile => ({
name: DEFAULT_AGENTS_FILENAME,
path: "/tmp/AGENTS.md",
content: "",
missing: false,
...overrides,
});
describe("sanitizeToolCallId", () => {
it("keeps valid tool call IDs", () => {
expect(sanitizeToolCallId("call_abc-123")).toBe("call_abc-123");
describe("standard mode (default)", () => {
it("keeps valid alphanumeric tool call IDs", () => {
expect(sanitizeToolCallId("callabc123")).toBe("callabc123");
});
it("keeps underscores and hyphens for readability", () => {
expect(sanitizeToolCallId("call_abc-123")).toBe("call_abc-123");
expect(sanitizeToolCallId("call_abc_def")).toBe("call_abc_def");
});
it("replaces invalid characters with underscores", () => {
expect(sanitizeToolCallId("call_abc|item:456")).toBe("call_abc_item_456");
});
it("returns default for empty IDs", () => {
expect(sanitizeToolCallId("")).toBe("default_tool_id");
});
});
it("replaces invalid characters with underscores", () => {
expect(sanitizeToolCallId("call_abc|item:456")).toBe("call_abc_item_456");
describe("strict mode (alphanumeric only)", () => {
it("strips all non-alphanumeric characters", () => {
expect(sanitizeToolCallId("call_abc-123", "strict")).toBe("callabc123");
expect(sanitizeToolCallId("call_abc|item:456", "strict")).toBe("callabcitem456");
expect(sanitizeToolCallId("whatsapp_login_1768799841527_1", "strict")).toBe(
"whatsapplogin17687998415271",
);
});
it("returns default for empty IDs", () => {
expect(sanitizeToolCallId("", "strict")).toBe("defaulttoolid");
});
});
it("returns default for empty IDs", () => {
expect(sanitizeToolCallId("")).toBe("default_tool_id");
describe("strict9 mode (Mistral tool call IDs)", () => {
it("returns alphanumeric IDs with length 9", () => {
const out = sanitizeToolCallId("call_abc|item:456", "strict9");
expect(out).toMatch(/^[a-zA-Z0-9]{9}$/);
});
it("returns default for empty IDs", () => {
expect(sanitizeToolCallId("", "strict9")).toMatch(/^[a-zA-Z0-9]{9}$/);
});
});
});

1
src/agents/pi-embedded-helpers.ts
View File

@@ -50,4 +50,5 @@ export {
} from "./pi-embedded-helpers/turns.js";
export type { EmbeddedContextFile, FailoverReason } from "./pi-embedded-helpers/types.js";
export type { ToolCallIdMode } from "./tool-call-id.js";
export { isValidCloudCodeAssistToolId, sanitizeToolCallId } from "./tool-call-id.js";

10
src/agents/pi-embedded-helpers/images.ts
View File

@@ -1,5 +1,6 @@
import type { AgentMessage, AgentToolResult } from "@mariozechner/pi-agent-core";
import type { ToolCallIdMode } from "../tool-call-id.js";
import { sanitizeToolCallIdsForCloudCodeAssist } from "../tool-call-id.js";
import { sanitizeContentBlocksImages } from "../tool-images.js";
import { stripThoughtSignatures } from "./bootstrap.js";
@@ -32,6 +33,13 @@ export async function sanitizeSessionMessagesImages(
label: string,
options?: {
sanitizeToolCallIds?: boolean;
/**
* Mode for tool call ID sanitization:
* - "standard" (default, preserves _-)
* - "strict" (alphanumeric only)
* - "strict9" (alphanumeric only, length 9)
*/
toolCallIdMode?: ToolCallIdMode;
enforceToolCallLast?: boolean;
preserveSignatures?: boolean;
sanitizeThoughtSignatures?: {
@@ -43,7 +51,7 @@ export async function sanitizeSessionMessagesImages(
// We sanitize historical session messages because Anthropic can reject a request
// if the transcript contains oversized base64 images (see MAX_IMAGE_DIMENSION_PX).
const sanitizedIds = options?.sanitizeToolCallIds
? sanitizeToolCallIdsForCloudCodeAssist(messages)
? sanitizeToolCallIdsForCloudCodeAssist(messages, options.toolCallIdMode)
: messages;
const out: AgentMessage[] = [];
for (const msg of sanitizedIds) {

19
src/agents/pi-embedded-runner.sanitize-session-history.test.ts
View File

@@ -54,6 +54,25 @@ describe("sanitizeSessionHistory", () => {
);
});
it("sanitizes tool call ids with strict9 for Mistral models", async () => {
vi.mocked(helpers.isGoogleModelApi).mockReturnValue(false);
await sanitizeSessionHistory({
messages: mockMessages,
modelApi: "openai-responses",
provider: "openrouter",
modelId: "mistralai/devstral-2512:free",
sessionManager: mockSessionManager,
sessionId: "test-session",
});
expect(helpers.sanitizeSessionMessagesImages).toHaveBeenCalledWith(
mockMessages,
"session:history",
expect.objectContaining({ sanitizeToolCallIds: true, toolCallIdMode: "strict9" }),
);
});
it("does not sanitize tool call ids for non-Google, non-OpenAI APIs", async () => {
vi.mocked(helpers.isGoogleModelApi).mockReturnValue(false);

27
src/agents/pi-embedded-runner/google.ts
View File

@@ -14,6 +14,8 @@ import { log } from "./logger.js";
import { describeUnknownError } from "./utils.js";
import { isAntigravityClaude } from "../pi-embedded-helpers/google.js";
import { cleanToolSchemaForGemini } from "../pi-tools.schema.js";
import { normalizeProviderId } from "../model-selection.js";
import type { ToolCallIdMode } from "../tool-call-id.js";
const GOOGLE_TURN_ORDERING_CUSTOM_TYPE = "google-turn-ordering-bootstrap";
const GOOGLE_SCHEMA_UNSUPPORTED_KEYWORDS = new Set([
@@ -44,12 +46,29 @@ const OPENAI_TOOL_CALL_ID_APIS = new Set([
"openai-responses",
"openai-codex-responses",
]);
const MISTRAL_MODEL_HINTS = [
"mistral",
"mixtral",
"codestral",
"pixtral",
"devstral",
"ministral",
"mistralai",
];
function shouldSanitizeToolCallIds(modelApi?: string | null): boolean {
if (!modelApi) return false;
return isGoogleModelApi(modelApi) || OPENAI_TOOL_CALL_ID_APIS.has(modelApi);
}
function isMistralModel(params: { provider?: string | null; modelId?: string | null }): boolean {
const provider = normalizeProviderId(params.provider ?? "");
if (provider === "mistral") return true;
const modelId = (params.modelId ?? "").toLowerCase();
if (!modelId) return false;
return MISTRAL_MODEL_HINTS.some((hint) => modelId.includes(hint));
}
function findUnsupportedSchemaKeywords(schema: unknown, path: string): string[] {
if (!schema || typeof schema !== "object") return [];
if (Array.isArray(schema)) {
@@ -191,12 +210,16 @@ export async function sanitizeSessionHistory(params: {
sessionId: string;
}): Promise<AgentMessage[]> {
const isAntigravityClaudeModel = isAntigravityClaude(params.modelApi, params.modelId);
const provider = (params.provider ?? "").toLowerCase();
const provider = normalizeProviderId(params.provider ?? "");
const modelId = (params.modelId ?? "").toLowerCase();
const isOpenRouterGemini =
(provider === "openrouter" || provider === "opencode") && modelId.includes("gemini");
const isMistral = isMistralModel({ provider, modelId });
const toolCallIdMode: ToolCallIdMode | undefined = isMistral ? "strict9" : undefined;
const sanitizeToolCallIds = shouldSanitizeToolCallIds(params.modelApi) || isMistral;
const sanitizedImages = await sanitizeSessionMessagesImages(params.messages, "session:history", {
sanitizeToolCallIds: shouldSanitizeToolCallIds(params.modelApi),
sanitizeToolCallIds,
toolCallIdMode,
enforceToolCallLast: params.modelApi === "anthropic-messages",
preserveSignatures: params.modelApi === "google-antigravity" && isAntigravityClaudeModel,
sanitizeThoughtSignatures: isOpenRouterGemini

336
src/agents/tool-call-id.test.ts
View File

@@ -7,106 +7,262 @@ import {
} from "./tool-call-id.js";
describe("sanitizeToolCallIdsForCloudCodeAssist", () => {
it("is a no-op for already-valid non-colliding IDs", () => {
const input = [
{
role: "assistant",
content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }],
},
{
role: "toolResult",
toolCallId: "call_1",
toolName: "read",
content: [{ type: "text", text: "ok" }],
},
] satisfies AgentMessage[];
describe("standard mode (default)", () => {
it("is a no-op for already-valid non-colliding IDs", () => {
const input = [
{
role: "assistant",
content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }],
},
{
role: "toolResult",
toolCallId: "call_1",
toolName: "read",
content: [{ type: "text", text: "ok" }],
},
] satisfies AgentMessage[];
const out = sanitizeToolCallIdsForCloudCodeAssist(input);
expect(out).toBe(input);
const out = sanitizeToolCallIdsForCloudCodeAssist(input);
expect(out).toBe(input);
});
it("replaces invalid characters with underscores (preserves readability)", () => {
const input = [
{
role: "assistant",
content: [{ type: "toolCall", id: "call|item:123", name: "read", arguments: {} }],
},
{
role: "toolResult",
toolCallId: "call|item:123",
toolName: "read",
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 };
// Standard mode preserves underscores for readability
expect(toolCall.id).toBe("call_item_123");
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 = [
{
role: "assistant",
content: [
{ type: "toolCall", id: "call_a|b", name: "read", arguments: {} },
{ type: "toolCall", id: "call_a:b", name: "read", arguments: {} },
],
},
{
role: "toolResult",
toolCallId: "call_a|b",
toolName: "read",
content: [{ type: "text", text: "one" }],
},
{
role: "toolResult",
toolCallId: "call_a:b",
toolName: "read",
content: [{ type: "text", text: "two" }],
},
] satisfies AgentMessage[];
const out = sanitizeToolCallIdsForCloudCodeAssist(input);
expect(out).not.toBe(input);
const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
const a = assistant.content?.[0] as { id?: string };
const b = assistant.content?.[1] as { id?: string };
expect(typeof a.id).toBe("string");
expect(typeof b.id).toBe("string");
expect(a.id).not.toBe(b.id);
expect(isValidCloudCodeAssistToolId(a.id as string)).toBe(true);
expect(isValidCloudCodeAssistToolId(b.id as string)).toBe(true);
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }>;
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }>;
expect(r1.toolCallId).toBe(a.id);
expect(r2.toolCallId).toBe(b.id);
});
it("caps tool call IDs at 40 chars while preserving uniqueness", () => {
const longA = `call_${"a".repeat(60)}`;
const longB = `call_${"a".repeat(59)}b`;
const input = [
{
role: "assistant",
content: [
{ type: "toolCall", id: longA, name: "read", arguments: {} },
{ type: "toolCall", id: longB, name: "read", arguments: {} },
],
},
{
role: "toolResult",
toolCallId: longA,
toolName: "read",
content: [{ type: "text", text: "one" }],
},
{
role: "toolResult",
toolCallId: longB,
toolName: "read",
content: [{ type: "text", text: "two" }],
},
] satisfies AgentMessage[];
const out = sanitizeToolCallIdsForCloudCodeAssist(input);
const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
const a = assistant.content?.[0] as { id?: string };
const b = assistant.content?.[1] as { id?: string };
expect(typeof a.id).toBe("string");
expect(typeof b.id).toBe("string");
expect(a.id).not.toBe(b.id);
expect(a.id?.length).toBeLessThanOrEqual(40);
expect(b.id?.length).toBeLessThanOrEqual(40);
expect(isValidCloudCodeAssistToolId(a.id as string)).toBe(true);
expect(isValidCloudCodeAssistToolId(b.id as string)).toBe(true);
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }>;
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }>;
expect(r1.toolCallId).toBe(a.id);
expect(r2.toolCallId).toBe(b.id);
});
});
it("avoids collisions when sanitization would produce duplicate IDs", () => {
const input = [
{
role: "assistant",
content: [
{ type: "toolCall", id: "call_a|b", name: "read", arguments: {} },
{ type: "toolCall", id: "call_a:b", name: "read", arguments: {} },
],
},
{
role: "toolResult",
toolCallId: "call_a|b",
toolName: "read",
content: [{ type: "text", text: "one" }],
},
{
role: "toolResult",
toolCallId: "call_a:b",
toolName: "read",
content: [{ type: "text", text: "two" }],
},
] satisfies AgentMessage[];
describe("strict mode (alphanumeric only)", () => {
it("strips underscores and hyphens from tool call IDs", () => {
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 out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict");
expect(out).not.toBe(input);
const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
const a = assistant.content?.[0] as { id?: string };
const b = assistant.content?.[1] as { id?: string };
expect(typeof a.id).toBe("string");
expect(typeof b.id).toBe("string");
expect(a.id).not.toBe(b.id);
expect(isValidCloudCodeAssistToolId(a.id as string)).toBe(true);
expect(isValidCloudCodeAssistToolId(b.id as string)).toBe(true);
const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
const toolCall = assistant.content?.[0] as { id?: string };
// Strict mode strips all non-alphanumeric characters
expect(toolCall.id).toBe("whatsapplogin17687998415271");
expect(isValidCloudCodeAssistToolId(toolCall.id as string, "strict")).toBe(true);
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }>;
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }>;
expect(r1.toolCallId).toBe(a.id);
expect(r2.toolCallId).toBe(b.id);
const result = out[1] as Extract<AgentMessage, { role: "toolResult" }>;
expect(result.toolCallId).toBe(toolCall.id);
});
it("avoids collisions with alphanumeric-only suffixes", () => {
const input = [
{
role: "assistant",
content: [
{ type: "toolCall", id: "call_a|b", name: "read", arguments: {} },
{ type: "toolCall", id: "call_a:b", name: "read", arguments: {} },
],
},
{
role: "toolResult",
toolCallId: "call_a|b",
toolName: "read",
content: [{ type: "text", text: "one" }],
},
{
role: "toolResult",
toolCallId: "call_a:b",
toolName: "read",
content: [{ type: "text", text: "two" }],
},
] satisfies AgentMessage[];
const out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict");
expect(out).not.toBe(input);
const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
const a = assistant.content?.[0] as { id?: string };
const b = assistant.content?.[1] as { id?: string };
expect(typeof a.id).toBe("string");
expect(typeof b.id).toBe("string");
expect(a.id).not.toBe(b.id);
// Both should be strictly alphanumeric
expect(isValidCloudCodeAssistToolId(a.id as string, "strict")).toBe(true);
expect(isValidCloudCodeAssistToolId(b.id as string, "strict")).toBe(true);
// Should not contain underscores or hyphens
expect(a.id).not.toMatch(/[_-]/);
expect(b.id).not.toMatch(/[_-]/);
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }>;
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }>;
expect(r1.toolCallId).toBe(a.id);
expect(r2.toolCallId).toBe(b.id);
});
});
it("caps tool call IDs at 40 chars while preserving uniqueness", () => {
const longA = `call_${"a".repeat(60)}`;
const longB = `call_${"a".repeat(59)}b`;
const input = [
{
role: "assistant",
content: [
{ type: "toolCall", id: longA, name: "read", arguments: {} },
{ type: "toolCall", id: longB, name: "read", arguments: {} },
],
},
{
role: "toolResult",
toolCallId: longA,
toolName: "read",
content: [{ type: "text", text: "one" }],
},
{
role: "toolResult",
toolCallId: longB,
toolName: "read",
content: [{ type: "text", text: "two" }],
},
] satisfies AgentMessage[];
describe("strict9 mode (Mistral tool call IDs)", () => {
it("enforces alphanumeric IDs with length 9", () => {
const input = [
{
role: "assistant",
content: [
{ type: "toolCall", id: "call_abc|item:123", name: "read", arguments: {} },
{ type: "toolCall", id: "call_abc|item:456", name: "read", arguments: {} },
],
},
{
role: "toolResult",
toolCallId: "call_abc|item:123",
toolName: "read",
content: [{ type: "text", text: "one" }],
},
{
role: "toolResult",
toolCallId: "call_abc|item:456",
toolName: "read",
content: [{ type: "text", text: "two" }],
},
] satisfies AgentMessage[];
const out = sanitizeToolCallIdsForCloudCodeAssist(input);
const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
const a = assistant.content?.[0] as { id?: string };
const b = assistant.content?.[1] as { id?: string };
const out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict9");
expect(out).not.toBe(input);
expect(typeof a.id).toBe("string");
expect(typeof b.id).toBe("string");
expect(a.id).not.toBe(b.id);
expect(a.id?.length).toBeLessThanOrEqual(40);
expect(b.id?.length).toBeLessThanOrEqual(40);
expect(isValidCloudCodeAssistToolId(a.id as string)).toBe(true);
expect(isValidCloudCodeAssistToolId(b.id as string)).toBe(true);
const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
const a = assistant.content?.[0] as { id?: string };
const b = assistant.content?.[1] as { id?: string };
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }>;
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }>;
expect(r1.toolCallId).toBe(a.id);
expect(r2.toolCallId).toBe(b.id);
expect(typeof a.id).toBe("string");
expect(typeof b.id).toBe("string");
expect(a.id).not.toBe(b.id);
expect(a.id?.length).toBe(9);
expect(b.id?.length).toBe(9);
expect(isValidCloudCodeAssistToolId(a.id as string, "strict9")).toBe(true);
expect(isValidCloudCodeAssistToolId(b.id as string, "strict9")).toBe(true);
const r1 = out[1] as Extract<AgentMessage, { role: "toolResult" }>;
const r2 = out[2] as Extract<AgentMessage, { role: "toolResult" }>;
expect(r1.toolCallId).toBe(a.id);
expect(r2.toolCallId).toBe(b.id);
});
});
});

103
src/agents/tool-call-id.ts
View File

@@ -2,46 +2,96 @@ 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";
export type ToolCallIdMode = "standard" | "strict" | "strict9";
const cloudCodeAssistPatternReplacement = id.replace(/[^a-zA-Z0-9_-]/g, "_");
const trimmedInvalidStartChars = cloudCodeAssistPatternReplacement.replace(
/^[^a-zA-Z0-9_-]+/,
"",
);
const STRICT9_LEN = 9;
return trimmedInvalidStartChars.length > 0 ? trimmedInvalidStartChars : "sanitized_tool_id";
/**
* Sanitize a tool call ID to be compatible with various providers.
*
* - "standard" mode: allows [a-zA-Z0-9_-], better readability (default)
* - "strict" mode: only [a-zA-Z0-9]
* - "strict9" mode: only [a-zA-Z0-9], length 9 (Mistral tool call requirement)
*/
export function sanitizeToolCallId(id: string, mode: ToolCallIdMode = "standard"): string {
if (!id || typeof id !== "string") {
if (mode === "strict9") return "defaultid";
return mode === "strict" ? "defaulttoolid" : "default_tool_id";
}
if (mode === "strict") {
// Some providers require strictly alphanumeric tool call IDs.
const alphanumericOnly = id.replace(/[^a-zA-Z0-9]/g, "");
return alphanumericOnly.length > 0 ? alphanumericOnly : "sanitizedtoolid";
}
if (mode === "strict9") {
const alphanumericOnly = id.replace(/[^a-zA-Z0-9]/g, "");
if (alphanumericOnly.length >= STRICT9_LEN) return alphanumericOnly.slice(0, STRICT9_LEN);
if (alphanumericOnly.length > 0) return shortHash(alphanumericOnly, STRICT9_LEN);
return shortHash("sanitized", STRICT9_LEN);
}
// Standard mode: allow underscores and hyphens for better readability in logs
const sanitized = id.replace(/[^a-zA-Z0-9_-]/g, "_");
const trimmed = sanitized.replace(/^[^a-zA-Z0-9_-]+/, "");
return trimmed.length > 0 ? trimmed : "sanitized_tool_id";
}
export function isValidCloudCodeAssistToolId(id: string): boolean {
export function isValidCloudCodeAssistToolId(
id: string,
mode: ToolCallIdMode = "standard",
): boolean {
if (!id || typeof id !== "string") return false;
if (mode === "strict") {
// Strictly alphanumeric for providers with tighter tool ID constraints
return /^[a-zA-Z0-9]+$/.test(id);
}
if (mode === "strict9") {
return /^[a-zA-Z0-9]{9}$/.test(id);
}
// Standard mode allows underscores and hyphens
return /^[a-zA-Z0-9_-]+$/.test(id);
}
function shortHash(text: string): string {
return createHash("sha1").update(text).digest("hex").slice(0, 8);
function shortHash(text: string, length = 8): string {
return createHash("sha1").update(text).digest("hex").slice(0, length);
}
function makeUniqueToolId(params: { id: string; used: Set<string> }): string {
function makeUniqueToolId(params: { id: string; used: Set<string>; mode: ToolCallIdMode }): string {
if (params.mode === "strict9") {
const base = sanitizeToolCallId(params.id, params.mode);
const candidate = base.length >= STRICT9_LEN ? base.slice(0, STRICT9_LEN) : "";
if (candidate && !params.used.has(candidate)) return candidate;
for (let i = 0; i < 1000; i += 1) {
const hashed = shortHash(`${params.id}:${i}`, STRICT9_LEN);
if (!params.used.has(hashed)) return hashed;
}
return shortHash(`${params.id}:${Date.now()}`, STRICT9_LEN);
}
const MAX_LEN = 40;
const base = sanitizeToolCallId(params.id).slice(0, MAX_LEN);
const base = sanitizeToolCallId(params.id, params.mode).slice(0, MAX_LEN);
if (!params.used.has(base)) return base;
const hash = shortHash(params.id);
const maxBaseLen = MAX_LEN - 1 - hash.length;
// Use separator based on mode: underscore for standard (readable), none for strict
const separator = params.mode === "strict" ? "" : "_";
const maxBaseLen = MAX_LEN - separator.length - hash.length;
const clippedBase = base.length > maxBaseLen ? base.slice(0, maxBaseLen) : base;
const candidate = `${clippedBase}_${hash}`;
const candidate = `${clippedBase}${separator}${hash}`;
if (!params.used.has(candidate)) return candidate;
for (let i = 2; i < 1000; i += 1) {
const suffix = `_${i}`;
const suffix = params.mode === "strict" ? `x${i}` : `_${i}`;
const next = `${candidate.slice(0, MAX_LEN - suffix.length)}${suffix}`;
if (!params.used.has(next)) return next;
}
const ts = `_${Date.now()}`;
const ts = params.mode === "strict" ? `t${Date.now()}` : `_${Date.now()}`;
return `${candidate.slice(0, MAX_LEN - ts.length)}${ts}`;
}
@@ -100,9 +150,20 @@ function rewriteToolResultIds(params: {
} as Extract<AgentMessage, { role: "toolResult" }>;
}
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`).
/**
* Sanitize tool call IDs for provider compatibility.
*
* @param messages - The messages to sanitize
* @param mode - "standard" (default, allows _-), "strict" (alphanumeric only), or "strict9" (alphanumeric length 9)
*/
export function sanitizeToolCallIdsForCloudCodeAssist(
messages: AgentMessage[],
mode: ToolCallIdMode = "standard",
): AgentMessage[] {
// Standard mode: allows [a-zA-Z0-9_-] for better readability in session logs
// 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` -> `a_b` or `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>();
@@ -110,7 +171,7 @@ export function sanitizeToolCallIdsForCloudCodeAssist(messages: AgentMessage[]):
const resolve = (id: string) => {
const existing = map.get(id);
if (existing) return existing;
const next = makeUniqueToolId({ id, used });
const next = makeUniqueToolId({ id, used, mode });
map.set(id, next);
used.add(next);
return next;

13
src/auto-reply/reply.directive.directive-behavior.lists-allowlisted-models-model-list.test.ts
View File

@@ -60,7 +60,7 @@ describe("directive behavior", () => {
vi.restoreAllMocks();
});
it("lists allowlisted models on /model list", async () => {
it("moves /model list to /models", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
@@ -90,7 +90,7 @@ describe("directive behavior", () => {
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("falls back to configured models when catalog is unavailable", async () => {
it("shows summary on /model when catalog is unavailable", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
vi.mocked(loadModelCatalog).mockResolvedValueOnce([]);
@@ -116,12 +116,13 @@ describe("directive behavior", () => {
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Current: anthropic/claude-opus-4-5");
expect(text).toContain("Switch: /model <provider/model>");
expect(text).toContain("Browse: /models (providers) or /models <provider> (models)");
expect(text).toContain("More: /model status");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("includes catalog models when no allowlist is set", async () => {
it("moves /model list to /models even when no allowlist is set", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
vi.mocked(loadModelCatalog).mockResolvedValueOnce([
@@ -156,7 +157,7 @@ describe("directive behavior", () => {
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("merges config allowlist models even when catalog is present", async () => {
it("moves /model list to /models even when catalog is present", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
// Catalog present but missing custom providers: /model should still include
@@ -207,7 +208,7 @@ describe("directive behavior", () => {
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("does not repeat missing auth labels on /model list", async () => {
it("moves /model list to /models without listing auth labels", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
@@ -231,6 +232,8 @@ describe("directive behavior", () => {
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model listing moved.");
expect(text).toContain("Use: /models (providers) or /models <provider> (models)");
expect(text).toContain("Switch: /model <provider/model>");
expect(text).not.toContain("missing (missing)");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});

6
src/auto-reply/reply.directive.directive-behavior.prefers-alias-matches-fuzzy-selection-is-ambiguous.test.ts
View File

@@ -67,7 +67,7 @@ describe("directive behavior", () => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
await getReplyFromConfig(
const res = await getReplyFromConfig(
{ Body: "/model ki", From: "+1222", To: "+1222", CommandAuthorized: true },
{},
{
@@ -103,9 +103,11 @@ describe("directive behavior", () => {
},
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model set to Kimi (moonshot/kimi-k2-0905-preview).");
assertModelSelection(storePath, {
model: "kimi-k2-0905-preview",
provider: "moonshot",
model: "kimi-k2-0905-preview",
});
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});

18
src/auto-reply/reply.directive.directive-behavior.supports-fuzzy-model-matches-model-directive.test.ts
View File

@@ -65,7 +65,7 @@ describe("directive behavior", () => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
await getReplyFromConfig(
const res = await getReplyFromConfig(
{ Body: "/model kimi", From: "+1222", To: "+1222", CommandAuthorized: true },
{},
{
@@ -94,9 +94,11 @@ describe("directive behavior", () => {
},
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model set to moonshot/kimi-k2-0905-preview.");
assertModelSelection(storePath, {
model: "kimi-k2-0905-preview",
provider: "moonshot",
model: "kimi-k2-0905-preview",
});
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
@@ -106,7 +108,7 @@ describe("directive behavior", () => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
await getReplyFromConfig(
const res = await getReplyFromConfig(
{
Body: "/model kimi-k2-0905-preview",
From: "+1222",
@@ -140,9 +142,11 @@ describe("directive behavior", () => {
},
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model set to moonshot/kimi-k2-0905-preview.");
assertModelSelection(storePath, {
model: "kimi-k2-0905-preview",
provider: "moonshot",
model: "kimi-k2-0905-preview",
});
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
@@ -152,7 +156,7 @@ describe("directive behavior", () => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
await getReplyFromConfig(
const res = await getReplyFromConfig(
{ Body: "/model moonshot/kimi", From: "+1222", To: "+1222", CommandAuthorized: true },
{},
{
@@ -181,9 +185,11 @@ describe("directive behavior", () => {
},
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model set to moonshot/kimi-k2-0905-preview.");
assertModelSelection(storePath, {
model: "kimi-k2-0905-preview",
provider: "moonshot",
model: "kimi-k2-0905-preview",
});
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});

3
src/auto-reply/reply.directive.directive-behavior.updates-tool-verbose-during-flight-run-toggle.test.ts
View File

@@ -187,7 +187,7 @@ describe("directive behavior", () => {
expect(runEmbeddedPiAgent).toHaveBeenCalledOnce();
});
});
it("lists allowlisted models on /model", async () => {
it("shows summary on /model", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
@@ -212,6 +212,7 @@ describe("directive behavior", () => {
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Current: anthropic/claude-opus-4-5");
expect(text).toContain("Switch: /model <provider/model>");
expect(text).toContain("Browse: /models (providers) or /models <provider> (models)");
expect(text).toContain("More: /model status");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();