diff --git a/src/UIMessages.combineUIMessages.test.ts b/src/UIMessages.combineUIMessages.test.ts new file mode 100644 index 00000000..f696de58 --- /dev/null +++ b/src/UIMessages.combineUIMessages.test.ts @@ -0,0 +1,239 @@ +import { describe, it, expect } from "vitest"; +import { combineUIMessages, type UIMessage } from "./UIMessages.js"; + +describe("combineUIMessages", () => { + it("should preserve all tool calls when combining messages", () => { + const messages: UIMessage[] = [ + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "streaming", + role: "assistant", + parts: [ + { + type: "tool-toolA", + toolCallId: "call_A", + state: "input-available", + input: {}, + }, + ], + text: "", + _creationTime: Date.now(), + }, + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "streaming", + role: "assistant", + parts: [ + { + type: "tool-toolB", + toolCallId: "call_B", + state: "input-available", + input: {}, + }, + ], + text: "", + _creationTime: Date.now(), + }, + ]; + + const result = combineUIMessages(messages); + + expect(result).toHaveLength(1); + expect(result[0].parts).toHaveLength(2); + + const toolCallIds = result[0].parts + .filter((p) => p.type.startsWith("tool-")) + .map((p: any) => p.toolCallId); + + expect(toolCallIds).toContain("call_A"); + expect(toolCallIds).toContain("call_B"); + }); + + it("should accumulate tool calls progressively (issue #182)", () => { + // Simulating: A(started) → B → C → A(result) + const messages: UIMessage[] = [ + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "streaming", + role: "assistant", + parts: [ + { + type: "tool-toolA", + toolCallId: "call_A", + state: "input-available", + input: {}, + }, + ], + text: "", + _creationTime: Date.now(), + }, + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "streaming", + role: "assistant", + parts: [ + { + type: "tool-toolA", + toolCallId: "call_A", + state: "input-available", + input: {}, + }, + { + type: "tool-toolB", + toolCallId: "call_B", + state: "input-available", + input: {}, + }, + ], + text: "", + _creationTime: Date.now(), + }, + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "streaming", + role: "assistant", + parts: [ + { + type: "tool-toolA", + toolCallId: "call_A", + state: "input-available", + input: {}, + }, + { + type: "tool-toolB", + toolCallId: "call_B", + state: "input-available", + input: {}, + }, + { + type: "tool-toolC", + toolCallId: "call_C", + state: "input-available", + input: {}, + }, + ], + text: "", + _creationTime: Date.now(), + }, + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "success", + role: "assistant", + parts: [ + { + type: "tool-toolA", + toolCallId: "call_A", + state: "output-available", + input: {}, + output: "success", + }, + { + type: "tool-toolB", + toolCallId: "call_B", + state: "input-available", + input: {}, + }, + { + type: "tool-toolC", + toolCallId: "call_C", + state: "input-available", + input: {}, + }, + ], + text: "", + _creationTime: Date.now(), + }, + ]; + + const result = combineUIMessages(messages); + + expect(result).toHaveLength(1); + expect(result[0].parts).toHaveLength(3); + + const toolCallIds = result[0].parts + .filter((p) => p.type.startsWith("tool-")) + .map((p: any) => p.toolCallId); + + // All tool calls should be present + expect(toolCallIds).toContain("call_A"); + expect(toolCallIds).toContain("call_B"); + expect(toolCallIds).toContain("call_C"); + + // Tool A should have the final state (output-available) + const toolA = result[0].parts.find( + (p: any) => p.type === "tool-toolA" && p.toolCallId === "call_A", + ) as any; + expect(toolA.state).toBe("output-available"); + expect(toolA.output).toBe("success"); + }); + + it("should merge tool calls with same toolCallId", () => { + const messages: UIMessage[] = [ + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "streaming", + role: "assistant", + parts: [ + { + type: "tool-toolA", + toolCallId: "call_A", + state: "input-available", + input: { test: "input" }, + }, + ], + text: "", + _creationTime: Date.now(), + }, + { + id: "msg1", + key: "thread-1-0", + order: 1, + stepOrder: 0, + status: "success", + role: "assistant", + parts: [ + { + type: "tool-toolA", + toolCallId: "call_A", + state: "output-available", + input: { test: "input" }, + output: "completed", + }, + ], + text: "", + _creationTime: Date.now(), + }, + ]; + + const result = combineUIMessages(messages); + + expect(result).toHaveLength(1); + expect(result[0].parts).toHaveLength(1); + + const toolCall = result[0].parts[0] as any; + expect(toolCall.toolCallId).toBe("call_A"); + expect(toolCall.state).toBe("output-available"); + expect(toolCall.output).toBe("completed"); + }); +}); diff --git a/src/UIMessages.ts b/src/UIMessages.ts index 4726f31e..d65de779 100644 --- a/src/UIMessages.ts +++ b/src/UIMessages.ts @@ -43,6 +43,7 @@ export type UIMessage< stepOrder: number; status: UIStatus; agentName?: string; + userId?: string; text: string; _creationTime: number; }; @@ -77,7 +78,8 @@ export async function fromUIMessages( "providerOptions", "metadata", ]), - ...omit(uiMessage, ["parts", "role", "key", "text"]), + ...omit(uiMessage, ["parts", "role", "key", "text", "userId"]), + userId: uiMessage.userId ?? meta.userId, status: uiMessage.status === "streaming" ? "pending" : "success", streaming: uiMessage.status === "streaming", // to override @@ -104,12 +106,21 @@ export async function fromUIMessages( finishReason: tool ? "tool-calls" : "stop", sources: fromSourceParts(uiMessage.parts), }; - if (Array.isArray(modelMessage.content)) { - const providerOptions = (modelMessage.content.find( - (c) => (c as any).providerOptions, - ) as any)?.providerOptions; if (providerOptions) { // convertToModelMessages changes providerMetadata to providerOptions - doc.providerMetadata = providerOptions; - doc.providerOptions ??= providerOptions; + if (Array.isArray(modelMessage.content)) { + // Find a content part with providerOptions (type assertion needed for SDK compatibility) + const partWithProviderOptions = modelMessage.content.find( + (c): c is typeof c & { providerOptions: unknown } => + "providerOptions" in c && c.providerOptions !== undefined, + ); + if (partWithProviderOptions?.providerOptions) { + // convertToModelMessages changes providerMetadata to providerOptions + const providerOptions = partWithProviderOptions.providerOptions as + | Record> + | undefined; + if (providerOptions) { + doc.providerMetadata = providerOptions; + doc.providerOptions ??= providerOptions; + } } } return doc; @@ -291,6 +302,7 @@ function createSystemUIMessage< text, role: "system", agentName: message.agentName, + userId: message.userId, parts: [{ type: "text", text, ...partCommon } satisfies TextUIPart], metadata: message.metadata, }; @@ -350,6 +362,7 @@ function createUserUIMessage< key: `${message.threadId}-${message.order}-${message.stepOrder}`, text, role: "user", + userId: message.userId, parts, metadata: message.metadata, }; @@ -373,6 +386,7 @@ function createAssistantUIMessage< stepOrder: firstMessage.stepOrder, key: `${firstMessage.threadId}-${firstMessage.order}-${firstMessage.stepOrder}`, agentName: firstMessage.agentName, + userId: firstMessage.userId, }; // Get status from last message @@ -469,6 +483,12 @@ function createAssistantUIMessage< typeof typedPart.output?.type === "string" ? typedPart.output.value : typedPart.output; + // Check for error at both the content part level (isError) and message level + // isError may exist on stored tool results but isn't in ToolResultPart type + const hasError = + (contentPart as { isError?: boolean }).isError || message.error; + const errorText = + message.error || (hasError ? String(output) : undefined); const call = allParts.find( (part) => part.type === `tool-${contentPart.toolName}` && @@ -476,9 +496,9 @@ function createAssistantUIMessage< part.toolCallId === contentPart.toolCallId, ) as ToolUIPart | undefined; if (call) { - if (message.error) { + if (hasError) { call.state = "output-error"; - call.errorText = message.error; + call.errorText = errorText ?? "Unknown error"; call.output = output; } else { call.state = "output-available"; @@ -489,13 +509,13 @@ function createAssistantUIMessage< "Tool result without preceding tool call.. adding anyways", contentPart, ); - if (message.error) { + if (hasError) { allParts.push({ type: `tool-${contentPart.toolName}`, toolCallId: contentPart.toolCallId, state: "output-error", input: undefined, - errorText: message.error, + errorText: errorText ?? "Unknown error", callProviderMetadata: message.providerMetadata, } satisfies ToolUIPart); } else { @@ -589,11 +609,12 @@ export function combineUIMessages(messages: UIMessage[]): UIMessage[] { const previousPartIndex = newParts.findIndex( (p) => getToolCallId(p) === toolCallId, ); - const previousPart = newParts.splice(previousPartIndex, 1)[0]; - if (!previousPart) { + if (previousPartIndex === -1) { + // Tool call not found in previous parts, add it as new newParts.push(part); continue; } + const previousPart = newParts.splice(previousPartIndex, 1)[0]; newParts.push(mergeParts(previousPart, part)); } acc.push({ diff --git a/src/mapping.ts b/src/mapping.ts index 7807c0db..2d69c0ac 100644 --- a/src/mapping.ts +++ b/src/mapping.ts @@ -572,6 +572,8 @@ function normalizeToolResult( : normalizeToolOutput("result" in part ? part.result : undefined), toolCallId: part.toolCallId, toolName: part.toolName, + // Preserve isError flag for error reporting + ...("isError" in part && part.isError ? { isError: true } : {}), ...metadata, } satisfies ToolResultPart; } diff --git a/src/toUIMessages.test.ts b/src/toUIMessages.test.ts index c5aea2a8..f871f8d3 100644 --- a/src/toUIMessages.test.ts +++ b/src/toUIMessages.test.ts @@ -728,4 +728,233 @@ describe("toUIMessages", () => { expect(textParts).toHaveLength(1); expect(textParts[0].text).toBe("The result is 42."); }); + + it("shows output-error state when tool result has isError: true (issue #162)", () => { + const messages = [ + // Tool call + baseMessageDoc({ + _id: "msg1", + order: 1, + stepOrder: 1, + tool: true, + message: { + role: "assistant", + content: [ + { + type: "tool-call", + toolName: "generateImage", + toolCallId: "call1", + args: { id: "invalid-id" }, + }, + ], + }, + }), + // Tool result with error + baseMessageDoc({ + _id: "msg2", + order: 1, + stepOrder: 2, + tool: true, + message: { + role: "tool", + content: [ + { + type: "tool-result", + toolCallId: "call1", + toolName: "generateImage", + output: { + type: "text", + value: + 'ArgumentValidationError: Value does not match validator.\nPath: .id\nValue: "invalid-id"\nValidator: v.id("images")', + }, + isError: true, + }, + ], + }, + }), + ]; + + const uiMessages = toUIMessages(messages); + + expect(uiMessages).toHaveLength(1); + expect(uiMessages[0].role).toBe("assistant"); + + const toolParts = uiMessages[0].parts.filter( + (p) => p.type === "tool-generateImage", + ); + expect(toolParts).toHaveLength(1); + + const toolPart = toolParts[0] as any; + expect(toolPart.toolCallId).toBe("call1"); + // Should show output-error, not output-available + expect(toolPart.state).toBe("output-error"); + expect(toolPart.output).toContain("ArgumentValidationError"); + }); + + it("shows output-error when tool result has isError: true without tool call present (issue #162)", () => { + // This simulates the case where the tool-call message wasn't saved + const messages = [ + baseMessageDoc({ + _id: "msg1", + order: 1, + stepOrder: 1, + tool: true, + message: { + role: "tool", + content: [ + { + type: "tool-result", + toolCallId: "call1", + toolName: "generateImage", + output: { + type: "text", + value: "Error: Something went wrong", + }, + isError: true, + }, + ], + }, + }), + ]; + + const uiMessages = toUIMessages(messages); + + expect(uiMessages).toHaveLength(1); + expect(uiMessages[0].role).toBe("assistant"); + + const toolParts = uiMessages[0].parts.filter( + (p) => p.type === "tool-generateImage", + ); + expect(toolParts).toHaveLength(1); + + const toolPart = toolParts[0] as any; + expect(toolPart.state).toBe("output-error"); + }); + + describe("userId preservation", () => { + it("preserves userId in user messages", () => { + const messages = [ + baseMessageDoc({ + userId: "user123", + message: { + role: "user", + content: "Hello!", + }, + text: "Hello!", + }), + ]; + const uiMessages = toUIMessages(messages); + expect(uiMessages).toHaveLength(1); + expect(uiMessages[0].role).toBe("user"); + expect(uiMessages[0].userId).toBe("user123"); + }); + + it("preserves userId in system messages", () => { + const messages = [ + baseMessageDoc({ + userId: "user456", + message: { + role: "system", + content: "System prompt", + }, + text: "System prompt", + }), + ]; + const uiMessages = toUIMessages(messages); + expect(uiMessages).toHaveLength(1); + expect(uiMessages[0].role).toBe("system"); + expect(uiMessages[0].userId).toBe("user456"); + }); + + it("preserves userId in assistant messages", () => { + const messages = [ + baseMessageDoc({ + userId: "user789", + message: { + role: "assistant", + content: "Hi there!", + }, + text: "Hi there!", + }), + ]; + const uiMessages = toUIMessages(messages); + expect(uiMessages).toHaveLength(1); + expect(uiMessages[0].role).toBe("assistant"); + expect(uiMessages[0].userId).toBe("user789"); + }); + + it("preserves userId from first message in grouped assistant messages", () => { + const messages = [ + baseMessageDoc({ + _id: "msg1", + userId: "userA", + order: 1, + stepOrder: 1, + tool: true, + message: { + role: "assistant", + content: [ + { + type: "tool-call", + toolName: "myTool", + toolCallId: "call1", + args: {}, + }, + ], + }, + text: "", + }), + baseMessageDoc({ + _id: "msg2", + userId: "userA", + order: 1, + stepOrder: 2, + tool: true, + message: { + role: "tool", + content: [ + { + type: "tool-result", + toolCallId: "call1", + toolName: "myTool", + output: { type: "text", value: "result" }, + }, + ], + }, + text: "", + }), + baseMessageDoc({ + _id: "msg3", + userId: "userA", + order: 1, + stepOrder: 3, + message: { + role: "assistant", + content: "Done!", + }, + text: "Done!", + }), + ]; + const uiMessages = toUIMessages(messages); + expect(uiMessages).toHaveLength(1); + expect(uiMessages[0].role).toBe("assistant"); + expect(uiMessages[0].userId).toBe("userA"); + }); + + it("handles undefined userId gracefully", () => { + const messages = [ + baseMessageDoc({ + // No userId provided + message: { + role: "user", + content: "Hello!", + }, + text: "Hello!", + }), + ]; + const uiMessages = toUIMessages(messages); + expect(uiMessages).toHaveLength(1); + expect(uiMessages[0].userId).toBeUndefined(); + }); + }); });