Skip to content

Commit f82e5d5

Browse files
daniel-lxsdaniel-lxs
committed
fix: preserve tool_use blocks for all tool_results in kept messages during condensation
When conversation condensation occurs, tool_result blocks in the kept message range could become orphaned if their matching tool_use blocks were condensed into summary text. This caused the Anthropic API to reject requests with 400 errors due to mismatched tool_use/tool_result IDs. The fix modifies getKeepMessagesWithToolBlocks to: - Check ALL kept messages for tool_result blocks (not just the first) - Search backwards (bounded by N_MESSAGES_TO_KEEP) for matching tool_uses by ID - Use Set-based deduplication to avoid preserving duplicate tool_use blocks Added helper functions: - getToolResultBlocks: extracts tool_result blocks from a message - findToolUseBlockById: finds a specific tool_use block by ID Added 5 new test cases covering boundary edge cases: - tool_result in 2nd/3rd kept message - multiple tool_results across kept messages - tool_use beyond search boundary (graceful handling) - duplicate tool_use_id handling Closes ROO-380
1 parent 424bce6 commit f82e5d5

File tree

2 files changed

+317
-21
lines changed

2 files changed

+317
-21
lines changed

‎src/core/condense/__tests__/index.spec.ts‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,254 @@ describe("getKeepMessagesWithToolBlocks", () => {
334334
expect(result.toolUseBlocksToPreserve).toHaveLength(1)
335335
expect(result.reasoningBlocksToPreserve).toHaveLength(0)
336336
})
337+
338+
it("should preserve tool_use when tool_result is in 2nd kept message and tool_use is 2 messages before boundary", () => {
339+
const toolUseBlock = {
340+
type: "tool_use" as const,
341+
id: "toolu_second_kept",
342+
name: "read_file",
343+
input: { path: "test.txt" },
344+
}
345+
const toolResultBlock = {
346+
type: "tool_result" as const,
347+
tool_use_id: "toolu_second_kept",
348+
content: "file contents",
349+
}
350+
351+
const messages: ApiMessage[] = [
352+
{ role: "user", content: "Hello", ts: 1 },
353+
{ role: "assistant", content: "Let me help", ts: 2 },
354+
{
355+
role: "assistant",
356+
content: [{ type: "text" as const, text: "Reading file..." }, toolUseBlock],
357+
ts: 3,
358+
},
359+
{ role: "user", content: "Some other message", ts: 4 },
360+
{ role: "assistant", content: "First kept message", ts: 5 },
361+
{
362+
role: "user",
363+
content: [toolResultBlock, { type: "text" as const, text: "Continue" }],
364+
ts: 6,
365+
},
366+
{ role: "assistant", content: "Third kept message", ts: 7 },
367+
]
368+
369+
const result = getKeepMessagesWithToolBlocks(messages, 3)
370+
371+
// keepMessages should be the last 3 messages (ts: 5, 6, 7)
372+
expect(result.keepMessages).toHaveLength(3)
373+
expect(result.keepMessages[0].ts).toBe(5)
374+
expect(result.keepMessages[1].ts).toBe(6)
375+
expect(result.keepMessages[2].ts).toBe(7)
376+
377+
// Should preserve the tool_use block from message at ts:3 (2 messages before boundary)
378+
expect(result.toolUseBlocksToPreserve).toHaveLength(1)
379+
expect(result.toolUseBlocksToPreserve[0]).toEqual(toolUseBlock)
380+
})
381+
382+
it("should preserve tool_use when tool_result is in 3rd kept message and tool_use is at boundary edge", () => {
383+
const toolUseBlock = {
384+
type: "tool_use" as const,
385+
id: "toolu_third_kept",
386+
name: "search",
387+
input: { query: "test" },
388+
}
389+
const toolResultBlock = {
390+
type: "tool_result" as const,
391+
tool_use_id: "toolu_third_kept",
392+
content: "search results",
393+
}
394+
395+
const messages: ApiMessage[] = [
396+
{ role: "user", content: "Start", ts: 1 },
397+
{
398+
role: "assistant",
399+
content: [{ type: "text" as const, text: "Searching..." }, toolUseBlock],
400+
ts: 2,
401+
},
402+
{ role: "user", content: "First kept message", ts: 3 },
403+
{ role: "assistant", content: "Second kept message", ts: 4 },
404+
{
405+
role: "user",
406+
content: [toolResultBlock, { type: "text" as const, text: "Done" }],
407+
ts: 5,
408+
},
409+
]
410+
411+
const result = getKeepMessagesWithToolBlocks(messages, 3)
412+
413+
// keepMessages should be the last 3 messages (ts: 3, 4, 5)
414+
expect(result.keepMessages).toHaveLength(3)
415+
expect(result.keepMessages[0].ts).toBe(3)
416+
expect(result.keepMessages[1].ts).toBe(4)
417+
expect(result.keepMessages[2].ts).toBe(5)
418+
419+
// Should preserve the tool_use block from message at ts:2 (at the search boundary edge)
420+
expect(result.toolUseBlocksToPreserve).toHaveLength(1)
421+
expect(result.toolUseBlocksToPreserve[0]).toEqual(toolUseBlock)
422+
})
423+
424+
it("should preserve multiple tool_uses when tool_results are in different kept messages", () => {
425+
const toolUseBlock1 = {
426+
type: "tool_use" as const,
427+
id: "toolu_multi_1",
428+
name: "read_file",
429+
input: { path: "file1.txt" },
430+
}
431+
const toolUseBlock2 = {
432+
type: "tool_use" as const,
433+
id: "toolu_multi_2",
434+
name: "read_file",
435+
input: { path: "file2.txt" },
436+
}
437+
const toolResultBlock1 = {
438+
type: "tool_result" as const,
439+
tool_use_id: "toolu_multi_1",
440+
content: "contents 1",
441+
}
442+
const toolResultBlock2 = {
443+
type: "tool_result" as const,
444+
tool_use_id: "toolu_multi_2",
445+
content: "contents 2",
446+
}
447+
448+
const messages: ApiMessage[] = [
449+
{ role: "user", content: "Start", ts: 1 },
450+
{
451+
role: "assistant",
452+
content: [{ type: "text" as const, text: "Reading file 1..." }, toolUseBlock1],
453+
ts: 2,
454+
},
455+
{ role: "user", content: "Some message", ts: 3 },
456+
{
457+
role: "assistant",
458+
content: [{ type: "text" as const, text: "Reading file 2..." }, toolUseBlock2],
459+
ts: 4,
460+
},
461+
{
462+
role: "user",
463+
content: [toolResultBlock1, { type: "text" as const, text: "First result" }],
464+
ts: 5,
465+
},
466+
{
467+
role: "user",
468+
content: [toolResultBlock2, { type: "text" as const, text: "Second result" }],
469+
ts: 6,
470+
},
471+
{ role: "assistant", content: "Got both files", ts: 7 },
472+
]
473+
474+
const result = getKeepMessagesWithToolBlocks(messages, 3)
475+
476+
// keepMessages should be the last 3 messages (ts: 5, 6, 7)
477+
expect(result.keepMessages).toHaveLength(3)
478+
479+
// Should preserve both tool_use blocks
480+
expect(result.toolUseBlocksToPreserve).toHaveLength(2)
481+
expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock1)
482+
expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock2)
483+
})
484+
485+
it("should not crash when tool_result references tool_use beyond search boundary", () => {
486+
const toolResultBlock = {
487+
type: "tool_result" as const,
488+
tool_use_id: "toolu_beyond_boundary",
489+
content: "result",
490+
}
491+
492+
// Tool_use is at ts:1, but with N_MESSAGES_TO_KEEP=3, we only search back 3 messages
493+
// from startIndex-1. StartIndex is 7 (messages.length=10, keepCount=3, startIndex=7).
494+
// So we search from index 6 down to index 4 (7-1 down to 7-3).
495+
// The tool_use at index 0 (ts:1) is beyond the search boundary.
496+
const messages: ApiMessage[] = [
497+
{
498+
role: "assistant",
499+
content: [
500+
{ type: "text" as const, text: "Way back..." },
501+
{
502+
type: "tool_use" as const,
503+
id: "toolu_beyond_boundary",
504+
name: "old_tool",
505+
input: {},
506+
},
507+
],
508+
ts: 1,
509+
},
510+
{ role: "user", content: "Message 2", ts: 2 },
511+
{ role: "assistant", content: "Message 3", ts: 3 },
512+
{ role: "user", content: "Message 4", ts: 4 },
513+
{ role: "assistant", content: "Message 5", ts: 5 },
514+
{ role: "user", content: "Message 6", ts: 6 },
515+
{ role: "assistant", content: "Message 7", ts: 7 },
516+
{
517+
role: "user",
518+
content: [toolResultBlock],
519+
ts: 8,
520+
},
521+
{ role: "assistant", content: "Message 9", ts: 9 },
522+
{ role: "user", content: "Message 10", ts: 10 },
523+
]
524+
525+
// Should not crash
526+
const result = getKeepMessagesWithToolBlocks(messages, 3)
527+
528+
// keepMessages should be the last 3 messages
529+
expect(result.keepMessages).toHaveLength(3)
530+
expect(result.keepMessages[0].ts).toBe(8)
531+
expect(result.keepMessages[1].ts).toBe(9)
532+
expect(result.keepMessages[2].ts).toBe(10)
533+
534+
// Should not preserve the tool_use since it's beyond the search boundary
535+
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
536+
})
537+
538+
it("should not duplicate tool_use blocks when same tool_result ID appears multiple times", () => {
539+
const toolUseBlock = {
540+
type: "tool_use" as const,
541+
id: "toolu_duplicate",
542+
name: "read_file",
543+
input: { path: "test.txt" },
544+
}
545+
const toolResultBlock1 = {
546+
type: "tool_result" as const,
547+
tool_use_id: "toolu_duplicate",
548+
content: "result 1",
549+
}
550+
const toolResultBlock2 = {
551+
type: "tool_result" as const,
552+
tool_use_id: "toolu_duplicate",
553+
content: "result 2",
554+
}
555+
556+
const messages: ApiMessage[] = [
557+
{ role: "user", content: "Start", ts: 1 },
558+
{
559+
role: "assistant",
560+
content: [{ type: "text" as const, text: "Using tool..." }, toolUseBlock],
561+
ts: 2,
562+
},
563+
{
564+
role: "user",
565+
content: [toolResultBlock1],
566+
ts: 3,
567+
},
568+
{ role: "assistant", content: "Processing", ts: 4 },
569+
{
570+
role: "user",
571+
content: [toolResultBlock2], // Same tool_use_id as first result
572+
ts: 5,
573+
},
574+
]
575+
576+
const result = getKeepMessagesWithToolBlocks(messages, 3)
577+
578+
// keepMessages should be the last 3 messages (ts: 3, 4, 5)
579+
expect(result.keepMessages).toHaveLength(3)
580+
581+
// Should only preserve the tool_use block once, not twice
582+
expect(result.toolUseBlocksToPreserve).toHaveLength(1)
583+
expect(result.toolUseBlocksToPreserve[0]).toEqual(toolUseBlock)
584+
})
337585
})
338586

339587
describe("getMessagesSinceLastSummary", () => {

‎src/core/condense/index.ts‎

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { t } from "../../i18n"
77
import { ApiHandler } from "../../api"
88
import { ApiMessage } from "../task-persistence/apiMessages"
99
import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning"
10+
import { findLast } from "../../shared/array"
1011

1112
/**
1213
* Checks if a message contains tool_result blocks.
@@ -30,6 +31,28 @@ function getToolUseBlocks(message: ApiMessage): Anthropic.Messages.ToolUseBlock[
3031
return message.content.filter((block) => block.type === "tool_use") as Anthropic.Messages.ToolUseBlock[]
3132
}
3233

34+
/**
35+
* Gets the tool_result blocks from a message.
36+
*/
37+
function getToolResultBlocks(message: ApiMessage): Anthropic.ToolResultBlockParam[] {
38+
if (message.role !== "user" || typeof message.content === "string") {
39+
return []
40+
}
41+
return message.content.filter((block): block is Anthropic.ToolResultBlockParam => block.type === "tool_result")
42+
}
43+
44+
/**
45+
* Finds a tool_use block by ID in a message.
46+
*/
47+
function findToolUseBlockById(message: ApiMessage, toolUseId: string): Anthropic.Messages.ToolUseBlock | undefined {
48+
if (message.role !== "assistant" || typeof message.content === "string") {
49+
return undefined
50+
}
51+
return message.content.find(
52+
(block): block is Anthropic.Messages.ToolUseBlock => block.type === "tool_use" && block.id === toolUseId,
53+
)
54+
}
55+
3356
/**
3457
* Gets reasoning blocks from a message's content array.
3558
* Task stores reasoning as {type: "reasoning", text: "..."} blocks,
@@ -57,11 +80,11 @@ export type KeepMessagesResult = {
5780

5881
/**
5982
* Extracts tool_use blocks that need to be preserved to match tool_result blocks in keepMessages.
60-
* When the first kept message is a user message with tool_result blocks,
61-
* we need to find the corresponding tool_use blocks from the preceding assistant message.
83+
* Checks ALL kept messages for tool_result blocks and searches backwards through the condensed
84+
* region (bounded by N_MESSAGES_TO_KEEP) to find the matching tool_use blocks by ID.
6285
* These tool_use blocks will be appended to the summary message to maintain proper pairing.
6386
*
64-
* Also extracts reasoning blocks from the preceding assistant message, which are required
87+
* Also extracts reasoning blocks from messages containing preserved tool_uses, which are required
6588
* by DeepSeek and Z.ai for interleaved thinking mode. Without these, the API returns a 400 error
6689
* "Missing reasoning_content field in the assistant message".
6790
* See: https://api-docs.deepseek.com/guides/thinking_mode#tool-calls
@@ -78,28 +101,53 @@ export function getKeepMessagesWithToolBlocks(messages: ApiMessage[], keepCount:
78101
const startIndex = messages.length - keepCount
79102
const keepMessages = messages.slice(startIndex)
80103

81-
// Check if the first kept message is a user message with tool_result blocks
82-
if (keepMessages.length > 0 && hasToolResultBlocks(keepMessages[0])) {
83-
// Look for the preceding assistant message with tool_use blocks
84-
const precedingIndex = startIndex - 1
85-
if (precedingIndex >= 0) {
86-
const precedingMessage = messages[precedingIndex]
87-
const toolUseBlocks = getToolUseBlocks(precedingMessage)
88-
if (toolUseBlocks.length > 0) {
89-
// Also extract reasoning blocks for DeepSeek/Z.ai interleaved thinking
90-
// Task stores reasoning as {type: "reasoning", text: "..."} content blocks
91-
const reasoningBlocks = getReasoningBlocks(precedingMessage)
92-
// Return the tool_use blocks and reasoning blocks to be merged into the summary message
93-
return {
94-
keepMessages,
95-
toolUseBlocksToPreserve: toolUseBlocks,
96-
reasoningBlocksToPreserve: reasoningBlocks,
97-
}
104+
const toolUseBlocksToPreserve: Anthropic.Messages.ToolUseBlock[] = []
105+
const reasoningBlocksToPreserve: Anthropic.Messages.ContentBlockParam[] = []
106+
const preservedToolUseIds = new Set<string>()
107+
108+
// Check ALL kept messages for tool_result blocks
109+
for (const keepMsg of keepMessages) {
110+
if (!hasToolResultBlocks(keepMsg)) {
111+
continue
112+
}
113+
114+
const toolResults = getToolResultBlocks(keepMsg)
115+
116+
for (const toolResult of toolResults) {
117+
const toolUseId = toolResult.tool_use_id
118+
119+
// Skip if we've already found this tool_use
120+
if (preservedToolUseIds.has(toolUseId)) {
121+
continue
122+
}
123+
124+
// Search backwards through the condensed region (bounded)
125+
const searchStart = startIndex - 1
126+
const searchEnd = Math.max(0, startIndex - N_MESSAGES_TO_KEEP)
127+
const messagesToSearch = messages.slice(searchEnd, searchStart + 1)
128+
129+
// Find the message containing this tool_use
130+
const messageWithToolUse = findLast(messagesToSearch, (msg) => {
131+
return findToolUseBlockById(msg, toolUseId) !== undefined
132+
})
133+
134+
if (messageWithToolUse) {
135+
const toolUse = findToolUseBlockById(messageWithToolUse, toolUseId)!
136+
toolUseBlocksToPreserve.push(toolUse)
137+
preservedToolUseIds.add(toolUseId)
138+
139+
// Also preserve reasoning blocks from that message
140+
const reasoning = getReasoningBlocks(messageWithToolUse)
141+
reasoningBlocksToPreserve.push(...reasoning)
98142
}
99143
}
100144
}
101145

102-
return { keepMessages, toolUseBlocksToPreserve: [], reasoningBlocksToPreserve: [] }
146+
return {
147+
keepMessages,
148+
toolUseBlocksToPreserve,
149+
reasoningBlocksToPreserve,
150+
}
103151
}
104152

105153
export const N_MESSAGES_TO_KEEP = 3

0 commit comments

Comments
 (0)