From 96264a063bea5efee7caebb6460e44942d0eecc7 Mon Sep 17 00:00:00 2001 From: PRATHAMESH HUKKERI Date: Mon, 20 Jul 2026 19:57:26 +0530 Subject: [PATCH] fix: Extend orphan message cleanup to handle trailing orphans from memory trimming (#34385) Co-authored-by: Prathamesh Hukkeri --- .../utils/agent-execution/memoryManagement.ts | 53 ++++-- .../test/memoryManagement.test.ts | 175 ++++++++++++++++++ 2 files changed, 217 insertions(+), 11 deletions(-) diff --git a/packages/@n8n/nodes-langchain/utils/agent-execution/memoryManagement.ts b/packages/@n8n/nodes-langchain/utils/agent-execution/memoryManagement.ts index c0c6ed37673..4197c6d8bc6 100644 --- a/packages/@n8n/nodes-langchain/utils/agent-execution/memoryManagement.ts +++ b/packages/@n8n/nodes-langchain/utils/agent-execution/memoryManagement.ts @@ -155,39 +155,70 @@ export function buildToolContext(steps: ToolCallData[]): string { } /** - * Removes orphaned ToolMessages and AIMessages with tool_calls from the start of chat history. + * Removes orphaned ToolMessages and AIMessages with tool_calls from the start and end of chat history. * This happens when memory trimming cuts messages mid-turn, leaving incomplete tool call sequences. * * @param chatHistory - Array of messages to clean up - * @returns Cleaned array with orphaned messages removed from the start + * @returns Cleaned array with orphaned messages removed from both ends */ -function cleanupOrphanedMessages(chatHistory: BaseMessage[]): BaseMessage[] { +export function cleanupOrphanedMessages(chatHistory: BaseMessage[]): BaseMessage[] { + const result = [...chatHistory]; + + // Clean up orphaned messages from the start let changed = true; - while (changed && chatHistory.length > 0) { + while (changed && result.length > 0) { changed = false; // Remove orphaned ToolMessages at the start - while (chatHistory.length > 0 && chatHistory[0] instanceof ToolMessage) { - chatHistory.shift(); + while (result.length > 0 && result[0] instanceof ToolMessage) { + result.shift(); changed = true; } // Remove AIMessages with tool_calls if they don't have following ToolMessages - if (chatHistory.length > 0) { - const firstMessage = chatHistory[0]; + if (result.length > 0) { + const firstMessage = result[0]; const hasOrphanedAIMessage = firstMessage instanceof AIMessage && (firstMessage.tool_calls?.length ?? 0) > 0 && - !(chatHistory[1] instanceof ToolMessage); + !(result[1] instanceof ToolMessage); if (hasOrphanedAIMessage) { - chatHistory.shift(); + result.shift(); changed = true; } } } - return chatHistory; + // Clean up orphaned messages from the end + changed = true; + while (changed && result.length > 0) { + changed = false; + const lastIdx = result.length - 1; + const lastMessage = result[lastIdx]; + + if (lastMessage instanceof ToolMessage) { + // Scan backwards past all consecutive trailing ToolMessages + let precedingIdx = lastIdx - 1; + while (precedingIdx >= 0 && result[precedingIdx] instanceof ToolMessage) { + precedingIdx--; + } + // The ToolMessage group is orphaned if not preceded by an AIMessage with tool_calls + const precedingMessage = precedingIdx >= 0 ? result[precedingIdx] : undefined; + const isPaired = + precedingMessage instanceof AIMessage && (precedingMessage.tool_calls?.length ?? 0) > 0; + if (!isPaired) { + result.pop(); + changed = true; + } + } else if (lastMessage instanceof AIMessage && (lastMessage.tool_calls?.length ?? 0) > 0) { + // An AIMessage with tool_calls at the end is orphaned (no ToolMessage follows) + result.pop(); + changed = true; + } + } + + return result; } /** diff --git a/packages/@n8n/nodes-langchain/utils/agent-execution/test/memoryManagement.test.ts b/packages/@n8n/nodes-langchain/utils/agent-execution/test/memoryManagement.test.ts index c7ad7d38c42..946da2d213a 100644 --- a/packages/@n8n/nodes-langchain/utils/agent-execution/test/memoryManagement.test.ts +++ b/packages/@n8n/nodes-langchain/utils/agent-execution/test/memoryManagement.test.ts @@ -16,6 +16,7 @@ import { buildToolContext, extractToolCallId, buildMessagesFromSteps, + cleanupOrphanedMessages, } from '../memoryManagement'; import type { ToolCallData } from '../types'; @@ -217,6 +218,180 @@ describe('memoryManagement', () => { }); }); + describe('cleanupOrphanedMessages', () => { + it('should return empty array when all messages are orphans', () => { + const chatHistory = [ + new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }), + new AIMessage({ + content: 'Call', + tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }], + }), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(0); + }); + + it('should remove orphaned AIMessage with tool_calls at end (missing trailing ToolMessage)', () => { + const chatHistory = [ + new HumanMessage('Question'), + new AIMessage('Answer'), + new AIMessage({ + content: 'Calling tool', + tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }], + }), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(2); + expect(result[0]).toBeInstanceOf(HumanMessage); + expect(result[1]).toBeInstanceOf(AIMessage); + }); + + it('should remove orphaned ToolMessages at end (no preceding AIMessage with tool_calls)', () => { + const chatHistory = [ + new HumanMessage('Question'), + new AIMessage('Answer'), + new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(2); + expect(result[0]).toBeInstanceOf(HumanMessage); + expect(result[1]).toBeInstanceOf(AIMessage); + }); + + it('should preserve valid tool call pair at end', () => { + const chatHistory = [ + new HumanMessage('Question'), + new AIMessage('Answer'), + new AIMessage({ + content: 'Calling tool', + tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }], + }), + new ToolMessage({ content: 'Result', tool_call_id: 'call-1', name: 'tool' }), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(4); + }); + + it('should handle trimming that splits mid-turn at end', () => { + // Simulates slice that kept AIMessage(tool_calls) but lost its ToolMessage + const chatHistory = [ + new HumanMessage('Hello'), + new AIMessage('Response'), + new HumanMessage('Use tool'), + new AIMessage({ + content: 'Calling tool', + tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }], + }), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(3); + expect(result[0]).toBeInstanceOf(HumanMessage); + expect(result[0].content).toBe('Hello'); + expect(result[1]).toBeInstanceOf(AIMessage); + expect(result[2]).toBeInstanceOf(HumanMessage); + expect(result[2].content).toBe('Use tool'); + }); + + it('should handle trimming that splits mid-turn at start', () => { + // Simulates slice that lost AIMessage but kept ToolMessage + const chatHistory = [ + new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }), + new HumanMessage('Next question'), + new AIMessage('Answer'), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(2); + expect(result[0]).toBeInstanceOf(HumanMessage); + expect(result[1]).toBeInstanceOf(AIMessage); + }); + + it('should preserve complete tool call sequences', () => { + const chatHistory = [ + new HumanMessage('Hello'), + new AIMessage({ + content: 'Calling tool', + tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }], + }), + new ToolMessage({ content: 'Result', tool_call_id: 'call-1', name: 'tool' }), + new AIMessage('Done'), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(4); + expect(result[0]).toBeInstanceOf(HumanMessage); + expect(result[1]).toBeInstanceOf(AIMessage); + expect(result[2]).toBeInstanceOf(ToolMessage); + expect(result[3]).toBeInstanceOf(AIMessage); + }); + + it('should not mutate the original array', () => { + const chatHistory = [ + new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }), + new HumanMessage('Question'), + ]; + + const originalLength = chatHistory.length; + cleanupOrphanedMessages(chatHistory); + + expect(chatHistory).toHaveLength(originalLength); + }); + + it('should preserve parallel tool call results at end', () => { + // Parallel tool calls: one AIMessage with multiple tool_calls, followed by multiple ToolMessages + const aiMessage = new AIMessage({ + content: 'Calling tools', + tool_calls: [ + { id: 'call-1', name: 'tool1', args: {}, type: 'tool_call' as const }, + { id: 'call-2', name: 'tool2', args: {}, type: 'tool_call' as const }, + ], + }); + + const chatHistory = [ + new HumanMessage('Question'), + aiMessage, + new ToolMessage({ content: 'Result 1', tool_call_id: 'call-1', name: 'tool1' }), + new ToolMessage({ content: 'Result 2', tool_call_id: 'call-2', name: 'tool2' }), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(4); + expect(result[0]).toBeInstanceOf(HumanMessage); + expect(result[1]).toBe(aiMessage); + expect(result[2]).toBeInstanceOf(ToolMessage); + expect(result[3]).toBeInstanceOf(ToolMessage); + }); + + it('should remove orphaned parallel tool results when AIMessage is missing', () => { + // Multiple ToolMessages at end but no preceding AIMessage with tool_calls + const chatHistory = [ + new HumanMessage('Question'), + new AIMessage('Answer'), + new ToolMessage({ content: 'Result 1', tool_call_id: 'call-1', name: 'tool1' }), + new ToolMessage({ content: 'Result 2', tool_call_id: 'call-2', name: 'tool2' }), + ]; + + const result = cleanupOrphanedMessages(chatHistory); + + expect(result).toHaveLength(2); + expect(result[0]).toBeInstanceOf(HumanMessage); + expect(result[1]).toBeInstanceOf(AIMessage); + }); + }); + describe('saveToMemory', () => { it('should save conversation to memory', async () => { const input = 'What is 2+2?';