diff --git a/CHANGELOG.md b/CHANGELOG.md index 871a2c9be..d0b0e692b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -179,6 +179,8 @@ Docs: https://docs.openclaw.ai - Gateway/config errors: surface up to three validation issues in top-level `config.set`, `config.patch`, and `config.apply` error messages while preserving structured issue details. (#42664) Thanks @huntharo. - Hooks/plugin context parity followup: pass `trigger` and `channelId` through embedded `llm_input`, `agent_end`, and `llm_output` hook contexts so plugins receive the same agent metadata across hook phases. (#42362) Thanks @zhoulf1006. - Status/context windows: normalize provider-qualified override cache keys so `/status` resolves the active provider's configured context window even when `models.providers` keys use mixed case or surrounding whitespace. (#36389) Thanks @haoruilee. +- ACP/main session aliases: canonicalize `main` before ACP session lookup so restarted ACP main sessions rehydrate instead of failing closed with `Session is not ACP-enabled: main`. (#43285, fixes #25692) +- Agents/embedded runner: recover canonical allowlisted tool names from malformed `toolCallId` and malformed non-blank tool-name variants before dispatch, while failing closed on ambiguous matches. (#34485) thanks @yuweuii. ## 2026.3.8 diff --git a/changelog/fragments/toolcall-id-malformed-name-inference.md b/changelog/fragments/toolcall-id-malformed-name-inference.md new file mode 100644 index 000000000..6af2b986f --- /dev/null +++ b/changelog/fragments/toolcall-id-malformed-name-inference.md @@ -0,0 +1 @@ +- runner: infer canonical tool names from malformed `toolCallId` variants (e.g. `functionsread3`, `functionswrite4`) when allowlist is present, preventing `Tool not found` regressions in strict routers. diff --git a/src/agents/pi-embedded-runner/run/attempt.test.ts b/src/agents/pi-embedded-runner/run/attempt.test.ts index 020372122..ef88e04ef 100644 --- a/src/agents/pi-embedded-runner/run/attempt.test.ts +++ b/src/agents/pi-embedded-runner/run/attempt.test.ts @@ -358,6 +358,279 @@ describe("wrapStreamFnTrimToolCallNames", () => { expect(result).toBe(finalMessage); }); + it("infers tool names from malformed toolCallId variants when allowlist is present", async () => { + const partialToolCall = { type: "toolCall", id: "functions.read:0", name: "" }; + const finalToolCallA = { type: "toolCall", id: "functionsread3", name: "" }; + const finalToolCallB: { type: string; id: string; name?: string } = { + type: "toolCall", + id: "functionswrite4", + }; + const finalToolCallC = { type: "functionCall", id: "functions.exec2", name: "" }; + const event = { + type: "toolcall_delta", + partial: { role: "assistant", content: [partialToolCall] }, + }; + const finalMessage = { + role: "assistant", + content: [finalToolCallA, finalToolCallB, finalToolCallC], + }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [event], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write", "exec"])); + for await (const _item of stream) { + // drain + } + const result = await stream.result(); + + expect(partialToolCall.name).toBe("read"); + expect(finalToolCallA.name).toBe("read"); + expect(finalToolCallB.name).toBe("write"); + expect(finalToolCallC.name).toBe("exec"); + expect(result).toBe(finalMessage); + }); + + it("does not infer names from malformed toolCallId when allowlist is absent", async () => { + const finalToolCall: { type: string; id: string; name?: string } = { + type: "toolCall", + id: "functionsread3", + }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn); + await stream.result(); + + expect(finalToolCall.name).toBeUndefined(); + }); + + it("infers malformed non-blank tool names before dispatch", async () => { + const partialToolCall = { type: "toolCall", id: "functionsread3", name: "functionsread3" }; + const finalToolCall = { type: "toolCall", id: "functionsread3", name: "functionsread3" }; + const event = { + type: "toolcall_delta", + partial: { role: "assistant", content: [partialToolCall] }, + }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [event], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + for await (const _item of stream) { + // drain + } + await stream.result(); + + expect(partialToolCall.name).toBe("read"); + expect(finalToolCall.name).toBe("read"); + }); + + it("recovers malformed non-blank names when id is missing", async () => { + const finalToolCall = { type: "toolCall", name: "functionsread3" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("read"); + }); + + it("recovers canonical tool names from canonical ids when name is empty", async () => { + const finalToolCall = { type: "toolCall", id: "read", name: "" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("read"); + }); + + it("recovers tool names from ids when name is whitespace-only", async () => { + const finalToolCall = { type: "toolCall", id: "functionswrite4", name: " " }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("write"); + }); + + it("keeps blank names blank and assigns fallback ids when both name and id are blank", async () => { + const finalToolCall = { type: "toolCall", id: "", name: "" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe(""); + expect(finalToolCall.id).toBe("call_auto_1"); + }); + + it("assigns fallback ids when both name and id are missing", async () => { + const finalToolCall: { type: string; name?: string; id?: string } = { type: "toolCall" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBeUndefined(); + expect(finalToolCall.id).toBe("call_auto_1"); + }); + + it("prefers explicit canonical names over conflicting canonical ids", async () => { + const finalToolCall = { type: "toolCall", id: "write", name: "read" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("read"); + expect(finalToolCall.id).toBe("write"); + }); + + it("prefers explicit trimmed canonical names over conflicting malformed ids", async () => { + const finalToolCall = { type: "toolCall", id: "functionswrite4", name: " read " }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("read"); + }); + + it("does not rewrite composite names that mention multiple tools", async () => { + const finalToolCall = { type: "toolCall", id: "functionsread3", name: "read write" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("read write"); + }); + + it("fails closed for malformed non-blank names that are ambiguous", async () => { + const finalToolCall = { type: "toolCall", id: "functions.exec2", name: "functions.exec2" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["exec", "exec2"])); + await stream.result(); + + expect(finalToolCall.name).toBe("functions.exec2"); + }); + + it("matches malformed ids case-insensitively across common separators", async () => { + const finalToolCall = { type: "toolCall", id: "Functions.Read_7", name: "" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("read"); + }); + it("does not override explicit non-blank tool names with inferred ids", async () => { + const finalToolCall = { type: "toolCall", id: "functionswrite4", name: "someOtherTool" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["read", "write"])); + await stream.result(); + + expect(finalToolCall.name).toBe("someOtherTool"); + }); + + it("fails closed when malformed ids could map to multiple allowlisted tools", async () => { + const finalToolCall = { type: "toolCall", id: "functions.exec2", name: "" }; + const finalMessage = { role: "assistant", content: [finalToolCall] }; + const baseFn = vi.fn(() => + createFakeStream({ + events: [], + resultMessage: finalMessage, + }), + ); + + const stream = await invokeWrappedStream(baseFn, new Set(["exec", "exec2"])); + await stream.result(); + + expect(finalToolCall.name).toBe(""); + }); it("does not collapse whitespace-only tool names to empty strings", async () => { const partialToolCall = { type: "toolCall", name: " " }; const finalToolCall = { type: "toolCall", name: "\t " }; diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index ef36c6dea..a1a00992f 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -425,19 +425,71 @@ export function wrapOllamaCompatNumCtx(baseFn: StreamFn | undefined, numCtx: num }); } -function normalizeToolCallNameForDispatch(rawName: string, allowedToolNames?: Set): string { - const trimmed = rawName.trim(); - if (!trimmed) { - // Keep whitespace-only placeholders unchanged so they do not collapse to - // empty names (which can later surface as toolName="" loops). +function resolveCaseInsensitiveAllowedToolName( + rawName: string, + allowedToolNames?: Set, +): string | null { + if (!allowedToolNames || allowedToolNames.size === 0) { + return null; + } + const folded = rawName.toLowerCase(); + let caseInsensitiveMatch: string | null = null; + for (const name of allowedToolNames) { + if (name.toLowerCase() !== folded) { + continue; + } + if (caseInsensitiveMatch && caseInsensitiveMatch !== name) { + return null; + } + caseInsensitiveMatch = name; + } + return caseInsensitiveMatch; +} + +function resolveExactAllowedToolName( + rawName: string, + allowedToolNames?: Set, +): string | null { + if (!allowedToolNames || allowedToolNames.size === 0) { + return null; + } + if (allowedToolNames.has(rawName)) { return rawName; } - if (!allowedToolNames || allowedToolNames.size === 0) { - return trimmed; + const normalized = normalizeToolName(rawName); + if (allowedToolNames.has(normalized)) { + return normalized; + } + return ( + resolveCaseInsensitiveAllowedToolName(rawName, allowedToolNames) ?? + resolveCaseInsensitiveAllowedToolName(normalized, allowedToolNames) + ); +} + +function buildStructuredToolNameCandidates(rawName: string): string[] { + const trimmed = rawName.trim(); + if (!trimmed) { + return []; } - const candidateNames = new Set([trimmed, normalizeToolName(trimmed)]); + const candidates: string[] = []; + const seen = new Set(); + const addCandidate = (value: string) => { + const candidate = value.trim(); + if (!candidate || seen.has(candidate)) { + return; + } + seen.add(candidate); + candidates.push(candidate); + }; + + addCandidate(trimmed); + addCandidate(normalizeToolName(trimmed)); + const normalizedDelimiter = trimmed.replace(/\//g, "."); + addCandidate(normalizedDelimiter); + addCandidate(normalizeToolName(normalizedDelimiter)); + const segments = normalizedDelimiter .split(".") .map((segment) => segment.trim()) @@ -445,11 +497,23 @@ function normalizeToolCallNameForDispatch(rawName: string, allowedToolNames?: Se if (segments.length > 1) { for (let index = 1; index < segments.length; index += 1) { const suffix = segments.slice(index).join("."); - candidateNames.add(suffix); - candidateNames.add(normalizeToolName(suffix)); + addCandidate(suffix); + addCandidate(normalizeToolName(suffix)); } } + return candidates; +} + +function resolveStructuredAllowedToolName( + rawName: string, + allowedToolNames?: Set, +): string | null { + if (!allowedToolNames || allowedToolNames.size === 0) { + return null; + } + + const candidateNames = buildStructuredToolNameCandidates(rawName); for (const candidate of candidateNames) { if (allowedToolNames.has(candidate)) { return candidate; @@ -457,23 +521,116 @@ function normalizeToolCallNameForDispatch(rawName: string, allowedToolNames?: Se } for (const candidate of candidateNames) { - const folded = candidate.toLowerCase(); - let caseInsensitiveMatch: string | null = null; - for (const name of allowedToolNames) { - if (name.toLowerCase() !== folded) { - continue; - } - if (caseInsensitiveMatch && caseInsensitiveMatch !== name) { - return candidate; - } - caseInsensitiveMatch = name; - } + const caseInsensitiveMatch = resolveCaseInsensitiveAllowedToolName(candidate, allowedToolNames); if (caseInsensitiveMatch) { return caseInsensitiveMatch; } } - return trimmed; + return null; +} + +function inferToolNameFromToolCallId( + rawId: string | undefined, + allowedToolNames?: Set, +): string | null { + if (!rawId || !allowedToolNames || allowedToolNames.size === 0) { + return null; + } + const id = rawId.trim(); + if (!id) { + return null; + } + + const candidateTokens = new Set(); + const addToken = (value: string) => { + const trimmed = value.trim(); + if (!trimmed) { + return; + } + candidateTokens.add(trimmed); + candidateTokens.add(trimmed.replace(/[:._/-]\d+$/, "")); + candidateTokens.add(trimmed.replace(/\d+$/, "")); + + const normalizedDelimiter = trimmed.replace(/\//g, "."); + candidateTokens.add(normalizedDelimiter); + candidateTokens.add(normalizedDelimiter.replace(/[:._-]\d+$/, "")); + candidateTokens.add(normalizedDelimiter.replace(/\d+$/, "")); + + for (const prefixPattern of [/^functions?[._-]?/i, /^tools?[._-]?/i]) { + const stripped = normalizedDelimiter.replace(prefixPattern, ""); + if (stripped !== normalizedDelimiter) { + candidateTokens.add(stripped); + candidateTokens.add(stripped.replace(/[:._-]\d+$/, "")); + candidateTokens.add(stripped.replace(/\d+$/, "")); + } + } + }; + + const preColon = id.split(":")[0] ?? id; + for (const seed of [id, preColon]) { + addToken(seed); + } + + let singleMatch: string | null = null; + for (const candidate of candidateTokens) { + const matched = resolveStructuredAllowedToolName(candidate, allowedToolNames); + if (!matched) { + continue; + } + if (singleMatch && singleMatch !== matched) { + return null; + } + singleMatch = matched; + } + + return singleMatch; +} + +function looksLikeMalformedToolNameCounter(rawName: string): boolean { + const normalizedDelimiter = rawName.trim().replace(/\//g, "."); + return ( + /^(?:functions?|tools?)[._-]?/i.test(normalizedDelimiter) && + /(?:[:._-]\d+|\d+)$/.test(normalizedDelimiter) + ); +} + +function normalizeToolCallNameForDispatch( + rawName: string, + allowedToolNames?: Set, + rawToolCallId?: string, +): string { + const trimmed = rawName.trim(); + if (!trimmed) { + // Keep whitespace-only placeholders unchanged unless we can safely infer + // a canonical name from toolCallId and allowlist. + return inferToolNameFromToolCallId(rawToolCallId, allowedToolNames) ?? rawName; + } + if (!allowedToolNames || allowedToolNames.size === 0) { + return trimmed; + } + + const exact = resolveExactAllowedToolName(trimmed, allowedToolNames); + if (exact) { + return exact; + } + // Some providers put malformed toolCallId-like strings into `name` + // itself (for example `functionsread3`). Recover conservatively from the + // name token before consulting the separate id so explicit names like + // `someOtherTool` are preserved. + const inferredFromName = inferToolNameFromToolCallId(trimmed, allowedToolNames); + if (inferredFromName) { + return inferredFromName; + } + + // If the explicit name looks like a provider-mangled tool-call id with a + // numeric suffix, fail closed when inference is ambiguous instead of routing + // to whichever structured candidate happens to match. + if (looksLikeMalformedToolNameCounter(trimmed)) { + return trimmed; + } + + return resolveStructuredAllowedToolName(trimmed, allowedToolNames) ?? trimmed; } function isToolCallBlockType(type: unknown): boolean { @@ -549,13 +706,21 @@ function trimWhitespaceFromToolCallNamesInMessage( if (!block || typeof block !== "object") { continue; } - const typedBlock = block as { type?: unknown; name?: unknown }; - if (!isToolCallBlockType(typedBlock.type) || typeof typedBlock.name !== "string") { + const typedBlock = block as { type?: unknown; name?: unknown; id?: unknown }; + if (!isToolCallBlockType(typedBlock.type)) { continue; } - const normalized = normalizeToolCallNameForDispatch(typedBlock.name, allowedToolNames); - if (normalized !== typedBlock.name) { - typedBlock.name = normalized; + const rawId = typeof typedBlock.id === "string" ? typedBlock.id : undefined; + if (typeof typedBlock.name === "string") { + const normalized = normalizeToolCallNameForDispatch(typedBlock.name, allowedToolNames, rawId); + if (normalized !== typedBlock.name) { + typedBlock.name = normalized; + } + continue; + } + const inferred = inferToolNameFromToolCallId(rawId, allowedToolNames); + if (inferred) { + typedBlock.name = inferred; } } normalizeToolCallIdsInMessage(message);