mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-11 18:03:44 +00:00
fix(agents): harden openai ws tool call id handling
This commit is contained in:
@@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
|
- OpenAI/Responses WebSocket tool-call id hygiene: normalize blank/whitespace streamed tool-call ids before persistence, and block empty `function_call_output.call_id` payloads in the WS conversion path to avoid OpenAI 400 errors (`Invalid 'input[n].call_id': empty string`), with regression coverage for both inbound stream normalization and outbound payload guards.
|
||||||
- Gateway/Control UI basePath webhook passthrough: let non-read methods under configured `controlUiBasePath` fall through to plugin routes (instead of returning Control UI 405), restoring webhook handlers behind basePath mounts. (#32311) Thanks @ademczuk.
|
- Gateway/Control UI basePath webhook passthrough: let non-read methods under configured `controlUiBasePath` fall through to plugin routes (instead of returning Control UI 405), restoring webhook handlers behind basePath mounts. (#32311) Thanks @ademczuk.
|
||||||
- Models/config env propagation: apply `config.env.vars` before implicit provider discovery in models bootstrap so config-scoped credentials are visible to implicit provider resolution paths. (#32295) Thanks @hsiaoa.
|
- Models/config env propagation: apply `config.env.vars` before implicit provider discovery in models bootstrap so config-scoped credentials are visible to implicit provider resolution paths. (#32295) Thanks @hsiaoa.
|
||||||
- Hooks/runtime stability: keep the internal hook handler registry on a `globalThis` singleton so hook registration/dispatch remains consistent when bundling emits duplicate module copies. (#32292) Thanks @Drickon.
|
- Hooks/runtime stability: keep the internal hook handler registry on a `globalThis` singleton so hook registration/dispatch remains consistent when bundling emits duplicate module copies. (#32292) Thanks @Drickon.
|
||||||
|
|||||||
@@ -424,6 +424,41 @@ describe("convertMessagesToInputItems", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("drops tool result messages with empty tool call id", () => {
|
||||||
|
const msg = {
|
||||||
|
role: "toolResult" as const,
|
||||||
|
toolCallId: " ",
|
||||||
|
toolName: "test_tool",
|
||||||
|
content: [{ type: "text", text: "output" }],
|
||||||
|
isError: false,
|
||||||
|
timestamp: 0,
|
||||||
|
};
|
||||||
|
const items = convertMessagesToInputItems([msg] as Parameters<
|
||||||
|
typeof convertMessagesToInputItems
|
||||||
|
>[0]);
|
||||||
|
expect(items).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to toolUseId when toolCallId is missing", () => {
|
||||||
|
const msg = {
|
||||||
|
role: "toolResult" as const,
|
||||||
|
toolUseId: "call_from_tool_use",
|
||||||
|
toolName: "test_tool",
|
||||||
|
content: [{ type: "text", text: "ok" }],
|
||||||
|
isError: false,
|
||||||
|
timestamp: 0,
|
||||||
|
};
|
||||||
|
const items = convertMessagesToInputItems([msg] as Parameters<
|
||||||
|
typeof convertMessagesToInputItems
|
||||||
|
>[0]);
|
||||||
|
expect(items).toHaveLength(1);
|
||||||
|
expect(items[0]).toMatchObject({
|
||||||
|
type: "function_call_output",
|
||||||
|
call_id: "call_from_tool_use",
|
||||||
|
output: "ok",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("converts a full multi-turn conversation", () => {
|
it("converts a full multi-turn conversation", () => {
|
||||||
const messages: FakeMessage[] = [
|
const messages: FakeMessage[] = [
|
||||||
userMsg("Run ls"),
|
userMsg("Run ls"),
|
||||||
@@ -454,6 +489,14 @@ describe("convertMessagesToInputItems", () => {
|
|||||||
expect(items[0]?.type).toBe("function_call");
|
expect(items[0]?.type).toBe("function_call");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("drops assistant tool calls with empty ids", () => {
|
||||||
|
const msg = assistantMsg([], [{ id: " ", name: "read", args: { path: "/tmp/a" } }]);
|
||||||
|
const items = convertMessagesToInputItems([msg] as Parameters<
|
||||||
|
typeof convertMessagesToInputItems
|
||||||
|
>[0]);
|
||||||
|
expect(items).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
it("skips thinking blocks in assistant messages", () => {
|
it("skips thinking blocks in assistant messages", () => {
|
||||||
const msg = {
|
const msg = {
|
||||||
role: "assistant" as const,
|
role: "assistant" as const,
|
||||||
|
|||||||
@@ -101,6 +101,14 @@ export function hasWsSession(sessionId: string): boolean {
|
|||||||
|
|
||||||
type AnyMessage = Message & { role: string; content: unknown };
|
type AnyMessage = Message & { role: string; content: unknown };
|
||||||
|
|
||||||
|
function toNonEmptyString(value: unknown): string | null {
|
||||||
|
if (typeof value !== "string") {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
const trimmed = value.trim();
|
||||||
|
return trimmed.length > 0 ? trimmed : null;
|
||||||
|
}
|
||||||
|
|
||||||
/** Convert pi-ai content (string | ContentPart[]) to plain text. */
|
/** Convert pi-ai content (string | ContentPart[]) to plain text. */
|
||||||
function contentToText(content: unknown): string {
|
function contentToText(content: unknown): string {
|
||||||
if (typeof content === "string") {
|
if (typeof content === "string") {
|
||||||
@@ -211,11 +219,16 @@ export function convertMessagesToInputItems(messages: Message[]): InputItem[] {
|
|||||||
});
|
});
|
||||||
textParts.length = 0;
|
textParts.length = 0;
|
||||||
}
|
}
|
||||||
|
const callId = toNonEmptyString(block.id);
|
||||||
|
const toolName = toNonEmptyString(block.name);
|
||||||
|
if (!callId || !toolName) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
// Push function_call item
|
// Push function_call item
|
||||||
items.push({
|
items.push({
|
||||||
type: "function_call",
|
type: "function_call",
|
||||||
call_id: typeof block.id === "string" ? block.id : `call_${randomUUID()}`,
|
call_id: callId,
|
||||||
name: block.name ?? "",
|
name: toolName,
|
||||||
arguments:
|
arguments:
|
||||||
typeof block.arguments === "string"
|
typeof block.arguments === "string"
|
||||||
? block.arguments
|
? block.arguments
|
||||||
@@ -245,14 +258,19 @@ export function convertMessagesToInputItems(messages: Message[]): InputItem[] {
|
|||||||
|
|
||||||
if (m.role === "toolResult") {
|
if (m.role === "toolResult") {
|
||||||
const tr = m as unknown as {
|
const tr = m as unknown as {
|
||||||
toolCallId: string;
|
toolCallId?: string;
|
||||||
|
toolUseId?: string;
|
||||||
content: unknown;
|
content: unknown;
|
||||||
isError: boolean;
|
isError: boolean;
|
||||||
};
|
};
|
||||||
|
const callId = toNonEmptyString(tr.toolCallId) ?? toNonEmptyString(tr.toolUseId);
|
||||||
|
if (!callId) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
const outputText = contentToText(tr.content);
|
const outputText = contentToText(tr.content);
|
||||||
items.push({
|
items.push({
|
||||||
type: "function_call_output",
|
type: "function_call_output",
|
||||||
call_id: tr.toolCallId,
|
call_id: callId,
|
||||||
output: outputText,
|
output: outputText,
|
||||||
});
|
});
|
||||||
continue;
|
continue;
|
||||||
@@ -280,10 +298,14 @@ export function buildAssistantMessageFromResponse(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if (item.type === "function_call") {
|
} else if (item.type === "function_call") {
|
||||||
|
const toolName = toNonEmptyString(item.name);
|
||||||
|
if (!toolName) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
content.push({
|
content.push({
|
||||||
type: "toolCall",
|
type: "toolCall",
|
||||||
id: item.call_id,
|
id: toNonEmptyString(item.call_id) ?? `call_${randomUUID()}`,
|
||||||
name: item.name,
|
name: toolName,
|
||||||
arguments: (() => {
|
arguments: (() => {
|
||||||
try {
|
try {
|
||||||
return JSON.parse(item.arguments) as Record<string, unknown>;
|
return JSON.parse(item.arguments) as Record<string, unknown>;
|
||||||
|
|||||||
@@ -244,6 +244,54 @@ describe("wrapStreamFnTrimToolCallNames", () => {
|
|||||||
expect(finalToolCall.name).toBe("\t ");
|
expect(finalToolCall.name).toBe("\t ");
|
||||||
expect(baseFn).toHaveBeenCalledTimes(1);
|
expect(baseFn).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("assigns fallback ids to missing/blank tool call ids in streamed and final messages", async () => {
|
||||||
|
const partialToolCall = { type: "toolCall", name: " read ", id: " " };
|
||||||
|
const finalToolCallA = { type: "toolCall", name: " exec ", id: "" };
|
||||||
|
const finalToolCallB = { type: "toolCall", name: " write " };
|
||||||
|
const event = {
|
||||||
|
type: "toolcall_delta",
|
||||||
|
partial: { role: "assistant", content: [partialToolCall] },
|
||||||
|
};
|
||||||
|
const finalMessage = { role: "assistant", content: [finalToolCallA, finalToolCallB] };
|
||||||
|
const baseFn = vi.fn(() =>
|
||||||
|
createFakeStream({
|
||||||
|
events: [event],
|
||||||
|
resultMessage: finalMessage,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
const stream = await invokeWrappedStream(baseFn);
|
||||||
|
for await (const _item of stream) {
|
||||||
|
// drain
|
||||||
|
}
|
||||||
|
const result = await stream.result();
|
||||||
|
|
||||||
|
expect(partialToolCall.name).toBe("read");
|
||||||
|
expect(partialToolCall.id).toBe("call_auto_1");
|
||||||
|
expect(finalToolCallA.name).toBe("exec");
|
||||||
|
expect(finalToolCallA.id).toBe("call_auto_1");
|
||||||
|
expect(finalToolCallB.name).toBe("write");
|
||||||
|
expect(finalToolCallB.id).toBe("call_auto_2");
|
||||||
|
expect(result).toBe(finalMessage);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("trims surrounding whitespace on tool call ids", async () => {
|
||||||
|
const finalToolCall = { type: "toolCall", name: " read ", id: " call_42 " };
|
||||||
|
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).toBe("read");
|
||||||
|
expect(finalToolCall.id).toBe("call_42");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("isOllamaCompatProvider", () => {
|
describe("isOllamaCompatProvider", () => {
|
||||||
|
|||||||
@@ -259,6 +259,64 @@ function normalizeToolCallNameForDispatch(rawName: string, allowedToolNames?: Se
|
|||||||
return caseInsensitiveMatch ?? trimmed;
|
return caseInsensitiveMatch ?? trimmed;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isToolCallBlockType(type: unknown): boolean {
|
||||||
|
return type === "toolCall" || type === "toolUse" || type === "functionCall";
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeToolCallIdsInMessage(message: unknown): void {
|
||||||
|
if (!message || typeof message !== "object") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const content = (message as { content?: unknown }).content;
|
||||||
|
if (!Array.isArray(content)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const usedIds = new Set<string>();
|
||||||
|
for (const block of content) {
|
||||||
|
if (!block || typeof block !== "object") {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const typedBlock = block as { type?: unknown; id?: unknown };
|
||||||
|
if (!isToolCallBlockType(typedBlock.type) || typeof typedBlock.id !== "string") {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const trimmedId = typedBlock.id.trim();
|
||||||
|
if (!trimmedId) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
usedIds.add(trimmedId);
|
||||||
|
}
|
||||||
|
|
||||||
|
let fallbackIndex = 1;
|
||||||
|
for (const block of content) {
|
||||||
|
if (!block || typeof block !== "object") {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const typedBlock = block as { type?: unknown; id?: unknown };
|
||||||
|
if (!isToolCallBlockType(typedBlock.type)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (typeof typedBlock.id === "string") {
|
||||||
|
const trimmedId = typedBlock.id.trim();
|
||||||
|
if (trimmedId) {
|
||||||
|
if (typedBlock.id !== trimmedId) {
|
||||||
|
typedBlock.id = trimmedId;
|
||||||
|
}
|
||||||
|
usedIds.add(trimmedId);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let fallbackId = "";
|
||||||
|
while (!fallbackId || usedIds.has(fallbackId)) {
|
||||||
|
fallbackId = `call_auto_${fallbackIndex++}`;
|
||||||
|
}
|
||||||
|
typedBlock.id = fallbackId;
|
||||||
|
usedIds.add(fallbackId);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function resolveOllamaBaseUrlForRun(params: {
|
export function resolveOllamaBaseUrlForRun(params: {
|
||||||
modelBaseUrl?: string;
|
modelBaseUrl?: string;
|
||||||
providerBaseUrl?: string;
|
providerBaseUrl?: string;
|
||||||
@@ -298,6 +356,7 @@ function trimWhitespaceFromToolCallNamesInMessage(
|
|||||||
typedBlock.name = normalized;
|
typedBlock.name = normalized;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
normalizeToolCallIdsInMessage(message);
|
||||||
}
|
}
|
||||||
|
|
||||||
function wrapStreamTrimToolCallNames(
|
function wrapStreamTrimToolCallNames(
|
||||||
|
|||||||
Reference in New Issue
Block a user