From 704d95672b5348fa02fde474cf7c62b1ce10b406 Mon Sep 17 00:00:00 2001 From: yehorkardash Date: Tue, 7 Jul 2026 12:37:39 +0200 Subject: [PATCH] fix(AI Agent Node): Remove assistant tool call redundant message and simplify chat tool response (#33640) --- .../llms/LMChatOllama/LmChatOllama.node.ts | 4 +- .../llms/LmChatCohere/LmChatCohere.node.ts | 13 +- .../nodes/trigger/ChatTrigger/Chat.node.ts | 20 ++- .../ChatTrigger/__test__/Chat.node.test.ts | 27 ++++ .../utils/agent-execution/buildSteps.ts | 14 ++- .../agent-execution/test/buildSteps.test.ts | 63 ++-------- .../utils/chatModelMessageWrapper.test.ts | 116 ++++++++++++++++++ .../utils/chatModelMessageWrapper.ts | 78 ++++++++++++ 8 files changed, 267 insertions(+), 68 deletions(-) create mode 100644 packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.test.ts create mode 100644 packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.ts diff --git a/packages/@n8n/nodes-langchain/nodes/llms/LMChatOllama/LmChatOllama.node.ts b/packages/@n8n/nodes-langchain/nodes/llms/LMChatOllama/LmChatOllama.node.ts index 0bb1885f76c..933992ceedf 100644 --- a/packages/@n8n/nodes-langchain/nodes/llms/LMChatOllama/LmChatOllama.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/llms/LMChatOllama/LmChatOllama.node.ts @@ -14,6 +14,8 @@ import { type SupplyData, } from 'n8n-workflow'; +import { wrapChatModelMessageInput } from '@utils/chatModelMessageWrapper'; + import { ollamaModel, ollamaOptions, ollamaDescription } from '../LMOllama/description'; export class LmChatOllama implements INodeType { @@ -81,7 +83,7 @@ export class LmChatOllama implements INodeType { }); return { - response: model, + response: wrapChatModelMessageInput(model), }; } } diff --git a/packages/@n8n/nodes-langchain/nodes/llms/LmChatCohere/LmChatCohere.node.ts b/packages/@n8n/nodes-langchain/nodes/llms/LmChatCohere/LmChatCohere.node.ts index aa5e20a7a90..dcf2e158228 100644 --- a/packages/@n8n/nodes-langchain/nodes/llms/LmChatCohere/LmChatCohere.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/llms/LmChatCohere/LmChatCohere.node.ts @@ -1,5 +1,10 @@ import { ChatCohere } from '@langchain/cohere'; import type { LLMResult } from '@langchain/core/outputs'; +import { + makeN8nLlmFailedAttemptHandler, + N8nLlmTracing, + getConnectionHintNoticeField, +} from '@n8n/ai-utilities'; import type { INodeType, INodeTypeDescription, @@ -7,11 +12,7 @@ import type { SupplyData, } from 'n8n-workflow'; -import { - makeN8nLlmFailedAttemptHandler, - N8nLlmTracing, - getConnectionHintNoticeField, -} from '@n8n/ai-utilities'; +import { wrapChatModelMessageInput } from '@utils/chatModelMessageWrapper'; export function tokensUsageParser(result: LLMResult): { completionTokens: number; @@ -176,7 +177,7 @@ export class LmChatCohere implements INodeType { }); return { - response: model, + response: wrapChatModelMessageInput(model), }; } } diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/Chat.node.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/Chat.node.ts index 58ef77149d1..b1e9a380c45 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/Chat.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/Chat.node.ts @@ -18,6 +18,7 @@ import { SEND_AND_WAIT_OPERATION, getHighlightedInputKey, getHighlightedResponseKey, + isToolType, } from 'n8n-workflow'; import type { IExecuteFunctions, @@ -36,6 +37,23 @@ import { getSendAndWaitPropertiesForChatNode, } from './util'; +function getToolFlowResponse(context: IExecuteFunctions, data: INodeExecutionData) { + // context.isToolExecution() doesn't work with ExecuteFunctionContext + const isToolExecution = isToolType(context.getNode().type); + if (!isToolExecution) return data; + + // strip empty field and add sent: true to clarify the result for LLMs + const json: IDataObject = { ...data.json, sent: true }; + if (json.chatInput === '') { + delete json.chatInput; + } + + return { + ...data, + json, + }; +} + export class Chat implements INodeType { description: INodeTypeDescription = { usableAsTool: true, @@ -226,7 +244,7 @@ export class Chat implements INodeType { if (!waitForReply) { // return original message instead of input data - if (nodeVersion >= 1.3) return [[data]]; + if (nodeVersion >= 1.3) return [[getToolFlowResponse(context, data)]]; const inputData = context.getInputData(); return [inputData]; diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/Chat.node.test.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/Chat.node.test.ts index 06a514b2630..a80e1454885 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/Chat.node.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/Chat.node.test.ts @@ -1,6 +1,7 @@ import type { INode, IExecuteFunctions } from 'n8n-workflow'; import { CHAT_NODE_TYPE, + CHAT_TOOL_NODE_TYPE, CHAT_TRIGGER_NODE_TYPE, FREE_TEXT_CHAT_RESPONSE_TYPE, SEND_AND_WAIT_OPERATION, @@ -317,6 +318,32 @@ describe('Test Chat Node', () => { expect(result).toEqual([[message]]); }); + it('v1.3 should return a tool-friendly response when used as a tool without waiting for reply', async () => { + const chatToolNode = mock({ + name: 'Chat', + type: CHAT_TOOL_NODE_TYPE, + parameters: {}, + typeVersion: 1.3, + }); + const message = { json: { chatInput: '' } }; + mockExecuteFunctions.getInputData.mockReturnValue([{ json: { chatInput: 'other input' } }]); + mockExecuteFunctions.getNode.mockReturnValue(chatToolNode); + mockExecuteFunctions.getNodeParameter.mockImplementation((parameterName) => { + switch (parameterName) { + case 'operation': + return 'send'; + case 'options': + return { memoryConnection: false }; + default: + return undefined; + } + }); + + const result = await chat.onMessage(mockExecuteFunctions, message); + + expect(result).toEqual([[{ json: { sent: true } }]]); + }); + it('v1.2 should return output data directly without nesting into `data` field (except `approved`)', async () => { const chatNode = mock({ name: 'Chat', diff --git a/packages/@n8n/nodes-langchain/utils/agent-execution/buildSteps.ts b/packages/@n8n/nodes-langchain/utils/agent-execution/buildSteps.ts index 26664102157..556ebcebfe3 100644 --- a/packages/@n8n/nodes-langchain/utils/agent-execution/buildSteps.ts +++ b/packages/@n8n/nodes-langchain/utils/agent-execution/buildSteps.ts @@ -134,7 +134,7 @@ function buildMessageContent( toolInput: IDataObject, toolId: string, toolName: string, -): string | Array { +): null | Array { const { thinkingContent, thinkingType, thinkingSignature } = providerMetadata; // Anthropic thinking mode: build content blocks @@ -149,8 +149,7 @@ function buildMessageContent( ); } - // Default: simple string content - return `Calling ${toolName} with input: ${JSON.stringify(toolInput)}`; + return null; } function resolveToolName(tool: EngineResult): string { @@ -231,9 +230,9 @@ function buildIndividualAIMessage( const content = buildMessageContent(providerMetadata, toolInput, toolId, toolName); return new AIMessage({ - content, + content: content ?? [], // When content is an array (Anthropic thinking), LangChain ignores tool_calls - ...(typeof content === 'string' && { tool_calls: [toolCall] }), + ...(content === null && { tool_calls: [toolCall] }), ...(providerMetadata.thoughtSignature && { additional_kwargs: buildGeminiAdditionalKwargs( [{ id: toolId, name: toolName, args: toolInput }], @@ -387,11 +386,14 @@ export function buildSteps( : [] : [buildIndividualAIMessage(toolId, toolName, toolInput, providerMetadata)]; + const logFallback = messageLog[0]?.content?.length + ? messageLog[0]?.content + : `Calling ${nodeName}`; steps.push({ action: { tool: toolName, toolInput: toolInputForResult, - log: toolInput.log || (messageLog[0]?.content ?? `Calling ${nodeName}`), + log: toolInput.log || logFallback, messageLog, toolCallId: toolInput?.id, type: toolInput.type || 'tool_call', diff --git a/packages/@n8n/nodes-langchain/utils/agent-execution/test/buildSteps.test.ts b/packages/@n8n/nodes-langchain/utils/agent-execution/test/buildSteps.test.ts index 3aebc9068bb..86311104beb 100644 --- a/packages/@n8n/nodes-langchain/utils/agent-execution/test/buildSteps.test.ts +++ b/packages/@n8n/nodes-langchain/utils/agent-execution/test/buildSteps.test.ts @@ -410,6 +410,8 @@ describe('buildSteps', () => { expect(result[0].action.messageLog).toHaveLength(1); const message = result[0].action.messageLog![0]; + expect(message.content).toEqual([]); + expect(result[0].action.log).toBe('Calling Calculator Node'); expect(message).toHaveProperty('tool_calls'); expect(message.tool_calls).toHaveLength(1); expect(message.tool_calls?.[0]).toMatchObject({ @@ -878,7 +880,7 @@ describe('buildSteps', () => { }); }); - it('should use string content when no thinking blocks present', () => { + it('should use empty content and tool_calls when no thinking blocks are present', () => { const response: EngineResponse = { actionResponses: [ { @@ -917,9 +919,9 @@ describe('buildSteps', () => { expect(result[0].action.messageLog).toHaveLength(1); const message = result[0].action.messageLog![0]; - expect(typeof message.content).toBe('string'); - expect(message.content).toContain('Calling Calculator'); + expect(message.content).toEqual([]); expect(message).toHaveProperty('tool_calls'); + expect(message.tool_calls?.[0].name).toBe('Calculator'); }); it('should handle thinking content without thinkingType', () => { @@ -961,8 +963,9 @@ describe('buildSteps', () => { expect(result).toHaveLength(1); const message = result[0].action.messageLog![0]; - // Should fall back to string content when thinkingType is missing - expect(typeof message.content).toBe('string'); + // Should fall back to default tool_calls format when thinkingType is missing + expect(message.content).toEqual([]); + expect(message.tool_calls?.[0].name).toBe('Calculator'); }); it('should work alongside Gemini thought_signature', () => { @@ -1111,53 +1114,6 @@ describe('buildSteps', () => { expect(result[0].action.tool).toBe('Calculator_Node'); }); - it('should use HITL toolName in message content', () => { - const response: EngineResponse = { - actionResponses: [ - { - action: { - actionType: 'ExecutionNodeAction', - nodeName: 'HITL Node', - input: { - id: 'call_123', - input: { query: 'test' }, - }, - type: NodeConnectionTypes.AiTool, - id: 'call_123', - metadata: { - itemIndex: 0, - hitl: { - toolName: 'custom_tool', - gatedToolNodeName: 'Custom Tool', - originalInput: { query: 'test' }, - }, - }, - }, - data: { - data: { - ai_tool: [[{ json: { result: 'success' } }]], - }, - executionTime: 0, - startTime: 0, - executionIndex: 0, - source: [], - }, - }, - ], - metadata: {}, - }; - - const result = buildSteps(response, itemIndex); - - expect(result).toHaveLength(1); - const message = result[0].action.messageLog![0]; - // Message content should use the HITL toolName - expect(message.content).toContain('Calling custom_tool'); - expect(message.content).not.toContain('HITL Node'); - // Tool call should also use the HITL toolName - expect(message.tool_calls?.[0].name).toBe('custom_tool'); - }); - it('should use converted nodeName in message content when HITL metadata is absent', () => { const response: EngineResponse = { actionResponses: [ @@ -1193,8 +1149,7 @@ describe('buildSteps', () => { expect(result).toHaveLength(1); const message = result[0].action.messageLog![0]; - // Message content should use the converted tool name - expect(message.content).toContain('Calling My_Custom_Node'); + expect(message.content).toEqual([]); expect(message.tool_calls?.[0].name).toBe('My_Custom_Node'); }); diff --git a/packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.test.ts b/packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.test.ts new file mode 100644 index 00000000000..2c8221645d4 --- /dev/null +++ b/packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.test.ts @@ -0,0 +1,116 @@ +import type { BaseChatModel } from '@langchain/core/language_models/chat_models'; +import { + AIMessage, + AIMessageChunk, + type BaseMessage, + HumanMessage, +} from '@langchain/core/messages'; +import { ChatGenerationChunk } from '@langchain/core/outputs'; + +import { + normalizeEmptyToolCallContent, + wrapChatModelMessageInput, +} from './chatModelMessageWrapper'; + +const toolCall = { + id: 'call_123', + name: 'Chat', + args: { Message: 'hello' }, + type: 'tool_call' as const, +}; + +describe('chatModelMessageWrapper', () => { + describe('normalizeEmptyToolCallContent', () => { + it('converts empty array content on AI tool-call messages to an empty string', () => { + const message = new AIMessage({ content: [], tool_calls: [toolCall] }); + + const [normalized] = normalizeEmptyToolCallContent([message]); + + expect(AIMessage.isInstance(normalized)).toBe(true); + expect(normalized.content).toBe(''); + expect((normalized as AIMessage).tool_calls).toEqual([toolCall]); + }); + + it('leaves non-tool-call messages unchanged', () => { + const message = new HumanMessage('hello'); + + const [normalized] = normalizeEmptyToolCallContent([message]); + + expect(normalized).toBe(message); + }); + + it('leaves AI messages without tool calls unchanged', () => { + const message = new AIMessage({ content: [] }); + + const [normalized] = normalizeEmptyToolCallContent([message]); + + expect(normalized).toBe(message); + }); + }); + + it('wraps generate and stream paths with the message transformer', async () => { + const seenGenerateMessages: unknown[] = []; + const seenStreamMessages: unknown[] = []; + const model = { + _generate: vi.fn(async (messages) => { + await Promise.resolve(); + seenGenerateMessages.push(messages); + return { generations: [] }; + }), + _streamResponseChunks: vi.fn(async function* (messages) { + await Promise.resolve(); + seenStreamMessages.push(messages); + yield new ChatGenerationChunk({ + text: '', + message: new AIMessageChunk({ content: '' }), + }); + }), + } as unknown as BaseChatModel; + + const wrapped = wrapChatModelMessageInput(model); + const message = new AIMessage({ content: [], tool_calls: [toolCall] }); + + expect(wrapped).toBe(model); + + await wrapped._generate([message], {}); + const streamChunks = []; + for await (const chunk of wrapped._streamResponseChunks([message], {})) { + streamChunks.push(chunk); + } + + expect(seenGenerateMessages).toHaveLength(1); + expect(seenStreamMessages).toHaveLength(1); + expect(streamChunks).toHaveLength(1); + expect((seenGenerateMessages[0] as AIMessage[])[0].content).toBe(''); + expect((seenStreamMessages[0] as AIMessage[])[0].content).toBe(''); + }); + + it('does not wrap the same model more than once', async () => { + const seenGenerateMessages: unknown[] = []; + const model = { + _generate: vi.fn(async (messages) => { + await Promise.resolve(); + seenGenerateMessages.push(messages); + return { generations: [] }; + }), + _streamResponseChunks: vi.fn(async function* () { + await Promise.resolve(); + yield new ChatGenerationChunk({ + text: '', + message: new AIMessageChunk({ content: '' }), + }); + }), + } as unknown as BaseChatModel; + const firstWrapper = vi.fn((messages: BaseMessage[]) => messages); + const secondWrapper = vi.fn((messages: BaseMessage[]) => messages); + + wrapChatModelMessageInput(model, firstWrapper); + wrapChatModelMessageInput(model, secondWrapper); + + await model._generate([new HumanMessage('hello')], {}); + + expect(firstWrapper).toHaveBeenCalledTimes(1); + expect(secondWrapper).not.toHaveBeenCalled(); + expect(seenGenerateMessages).toHaveLength(1); + }); +}); diff --git a/packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.ts b/packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.ts new file mode 100644 index 00000000000..1fd8e151f37 --- /dev/null +++ b/packages/@n8n/nodes-langchain/utils/chatModelMessageWrapper.ts @@ -0,0 +1,78 @@ +import type { CallbackManagerForLLMRun } from '@langchain/core/callbacks/manager'; +import type { BaseChatModel } from '@langchain/core/language_models/chat_models'; +import { AIMessage } from '@langchain/core/messages'; +import type { BaseMessage } from '@langchain/core/messages'; +import type { ChatGenerationChunk, ChatResult } from '@langchain/core/outputs'; + +const wrappedChatModelMessageInput = Symbol('wrappedChatModelMessageInput'); + +type GenerateMethod = ( + messages: BaseMessage[], + options: unknown, + runManager?: CallbackManagerForLLMRun, +) => Promise; + +type StreamMethod = ( + messages: BaseMessage[], + options: unknown, + runManager?: CallbackManagerForLLMRun, +) => AsyncGenerator; + +type PatchableChatModel = { + _generate: GenerateMethod; + _streamResponseChunks: StreamMethod; + [wrappedChatModelMessageInput]?: true; +}; + +export type ChatModelMessageWrapper = (messages: BaseMessage[]) => BaseMessage[]; + +export function normalizeEmptyToolCallContent(messages: BaseMessage[]): BaseMessage[] { + return messages.map((message) => { + if ( + AIMessage.isInstance(message) && + Array.isArray(message.content) && + message.content.length === 0 && + message.tool_calls?.length + ) { + return new AIMessage({ + id: message.id, + name: message.name, + content: '', + additional_kwargs: message.additional_kwargs, + response_metadata: message.response_metadata, + tool_calls: message.tool_calls, + invalid_tool_calls: message.invalid_tool_calls, + usage_metadata: message.usage_metadata, + }); + } + + return message; + }); +} + +/** + * A method that wraps langchain chat model to convert incoming messages to a format that is compatible with the model. + * By default, it normalizes messages with tool calls and content:[] to have content:''. Some old models expect to have some content alongside the tool calls. + */ +export function wrapChatModelMessageInput( + model: TModel, + wrapMessages: ChatModelMessageWrapper = normalizeEmptyToolCallContent, +): TModel { + const patchableModel = model as TModel & PatchableChatModel; + + if (patchableModel[wrappedChatModelMessageInput]) return model; + + const originalGenerate = patchableModel._generate.bind(model); + const originalStreamResponseChunks = patchableModel._streamResponseChunks.bind(model); + + patchableModel._generate = async (messages, options, runManager) => + await originalGenerate(wrapMessages(messages), options, runManager); + + patchableModel._streamResponseChunks = async function* (messages, options, runManager) { + yield* originalStreamResponseChunks(wrapMessages(messages), options, runManager); + }; + + patchableModel[wrappedChatModelMessageInput] = true; + + return model; +}