From 04fc2a02efefba4f98f1c80b5b0a6cf2f1cabfd7 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 15 Jan 2026 11:26:58 -0500 Subject: [PATCH] fix: search entire condensed region for tool_use blocks (ROO-520) The bounded search window in getKeepMessagesWithToolBlocks was too restrictive after multiple condensations. When tool_use blocks are preserved in earlier summary messages, they can end up outside the N_MESSAGES_TO_KEEP search window, causing orphaned tool_result blocks that trigger 400 errors from the API. Changed the search to cover the entire condensed region (from index 0 to startIndex-1) instead of just the last N_MESSAGES_TO_KEEP messages. Fixes: https://linear.app/roocode/issue/ROO-520 --- src/core/condense/__tests__/index.spec.ts | 97 ++++++++++++++++++++--- src/core/condense/index.ts | 17 ++-- 2 files changed, 95 insertions(+), 19 deletions(-) diff --git a/src/core/condense/__tests__/index.spec.ts b/src/core/condense/__tests__/index.spec.ts index ef5af012435..605dcd7fb56 100644 --- a/src/core/condense/__tests__/index.spec.ts +++ b/src/core/condense/__tests__/index.spec.ts @@ -482,27 +482,29 @@ describe("getKeepMessagesWithToolBlocks", () => { expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock2) }) - it("should not crash when tool_result references tool_use beyond search boundary", () => { + it("should find tool_use even when it is far back in the message history (ROO-520 fix)", () => { + // This test verifies the fix for https://linear.app/roocode/issue/ROO-520 + // The bug occurred when tool_use was preserved in an earlier summary message + // after multiple condensations. The bounded search window was too small to find it, + // resulting in orphaned tool_result blocks that caused 400 errors from the API. const toolResultBlock = { type: "tool_result" as const, - tool_use_id: "toolu_beyond_boundary", + tool_use_id: "toolu_early_in_history", content: "result", } - // Tool_use is at ts:1, but with N_MESSAGES_TO_KEEP=3, we only search back 3 messages - // from startIndex-1. StartIndex is 7 (messages.length=10, keepCount=3, startIndex=7). - // So we search from index 6 down to index 4 (7-1 down to 7-3). - // The tool_use at index 0 (ts:1) is beyond the search boundary. + // Tool_use is at index 0, tool_result is in the last 3 messages. + // The search must cover the ENTIRE condensed region to find the tool_use. const messages: ApiMessage[] = [ { role: "assistant", content: [ - { type: "text" as const, text: "Way back..." }, + { type: "text" as const, text: "Early tool call..." }, { type: "tool_use" as const, - id: "toolu_beyond_boundary", - name: "old_tool", - input: {}, + id: "toolu_early_in_history", + name: "read_file", + input: { path: "test.txt" }, }, ], ts: 1, @@ -522,7 +524,6 @@ describe("getKeepMessagesWithToolBlocks", () => { { role: "user", content: "Message 10", ts: 10 }, ] - // Should not crash const result = getKeepMessagesWithToolBlocks(messages, 3) // keepMessages should be the last 3 messages @@ -531,8 +532,78 @@ describe("getKeepMessagesWithToolBlocks", () => { expect(result.keepMessages[1].ts).toBe(9) expect(result.keepMessages[2].ts).toBe(10) - // Should not preserve the tool_use since it's beyond the search boundary - expect(result.toolUseBlocksToPreserve).toHaveLength(0) + // With the fix, tool_use should be found even though it's far back in history + expect(result.toolUseBlocksToPreserve).toHaveLength(1) + expect(result.toolUseBlocksToPreserve[0].id).toBe("toolu_early_in_history") + }) + + it("should find tool_use in previous summary message after multiple condensations (ROO-520)", () => { + // This test simulates the exact scenario from ROO-520: + // After first condensation, tool_use was preserved in summary1. + // After second condensation, the bounded search couldn't find tool_use in summary1 + // because it was outside the N_MESSAGES_TO_KEEP search window. + const toolUseBlock = { + type: "tool_use" as const, + id: "toolu_in_summary", + name: "read_file", + input: { path: "test.txt" }, + } + const toolResultBlock = { + type: "tool_result" as const, + tool_use_id: "toolu_in_summary", + content: "file contents", + } + + // Simulate state after first condensation: + // summary1 contains a preserved tool_use block from the first condense + const summaryMessage: ApiMessage = { + role: "assistant", + content: [{ type: "text" as const, text: "Summary of first conversation chunk" }, toolUseBlock], + ts: 5, + isSummary: true, + condenseId: "summary-1", + } + + const messages: ApiMessage[] = [ + { role: "user", content: "First message", ts: 1 }, + // Messages 2-4 are tagged with condenseParent from first condensation + { role: "assistant", content: "Message 2", ts: 2, condenseParent: "summary-1" }, + { role: "user", content: "Message 3", ts: 3, condenseParent: "summary-1" }, + { role: "assistant", content: "Message 4", ts: 4, condenseParent: "summary-1" }, + // summary1 with preserved tool_use + summaryMessage, + // Messages after first condensation + { role: "user", content: "Message 6", ts: 6 }, + { role: "assistant", content: "Message 7", ts: 7 }, + { role: "user", content: "Message 8", ts: 8 }, + { role: "assistant", content: "Message 9", ts: 9 }, + { role: "user", content: "Message 10", ts: 10 }, + { role: "assistant", content: "Message 11", ts: 11 }, + // This user message has tool_result referencing the tool_use in summary1 + { + role: "user", + content: [toolResultBlock, { type: "text" as const, text: "Continue" }], + ts: 12, + }, + { role: "assistant", content: "Message 13", ts: 13 }, + { role: "user", content: "Message 14", ts: 14 }, + ] + + // Second condensation: keepCount=3 means we keep messages 12, 13, 14 + // startIndex = 14 - 3 = 11 + // Old bounded search would only look at messages[8:11] = [msg9, msg10, msg11] + // The tool_use in summary1 at index 4 would NOT be found! + const result = getKeepMessagesWithToolBlocks(messages, 3) + + expect(result.keepMessages).toHaveLength(3) + expect(result.keepMessages[0].ts).toBe(12) // Has tool_result + expect(result.keepMessages[1].ts).toBe(13) + expect(result.keepMessages[2].ts).toBe(14) + + // With the fix, we search the ENTIRE condensed region (messages[0:11]) + // and find the tool_use in summary1 + expect(result.toolUseBlocksToPreserve).toHaveLength(1) + expect(result.toolUseBlocksToPreserve[0].id).toBe("toolu_in_summary") }) it("should not duplicate tool_use blocks when same tool_result ID appears multiple times", () => { diff --git a/src/core/condense/index.ts b/src/core/condense/index.ts index 79bc31ef9fc..dc18c61876f 100644 --- a/src/core/condense/index.ts +++ b/src/core/condense/index.ts @@ -80,10 +80,15 @@ export type KeepMessagesResult = { /** * Extracts tool_use blocks that need to be preserved to match tool_result blocks in keepMessages. - * Checks ALL kept messages for tool_result blocks and searches backwards through the condensed - * region (bounded by N_MESSAGES_TO_KEEP) to find the matching tool_use blocks by ID. + * Checks ALL kept messages for tool_result blocks and searches backwards through the ENTIRE + * condensed region to find the matching tool_use blocks by ID. * These tool_use blocks will be appended to the summary message to maintain proper pairing. * + * IMPORTANT: The search must cover the entire condensed region (from index 0 to startIndex-1), + * not just a bounded window. After multiple condensations, tool_use blocks can be preserved in + * earlier summary messages that are outside a bounded search window. + * See: https://linear.app/roocode/issue/ROO-520 + * * Also extracts reasoning blocks from messages containing preserved tool_uses, which are required * by DeepSeek and Z.ai for interleaved thinking mode. Without these, the API returns a 400 error * "Missing reasoning_content field in the assistant message". @@ -121,10 +126,10 @@ export function getKeepMessagesWithToolBlocks(messages: ApiMessage[], keepCount: continue } - // Search backwards through the condensed region (bounded) - const searchStart = startIndex - 1 - const searchEnd = Math.max(0, startIndex - N_MESSAGES_TO_KEEP) - const messagesToSearch = messages.slice(searchEnd, searchStart + 1) + // Search backwards through the ENTIRE condensed region (from index 0 to startIndex-1) + // This is critical because after multiple condensations, tool_use blocks may be + // preserved in earlier summary messages that are outside a bounded search window. + const messagesToSearch = messages.slice(0, startIndex) // Find the message containing this tool_use const messageWithToolUse = findLast(messagesToSearch, (msg) => {