Skip to content

Commit 08027ad

Browse files
authored
fix: send tool_result blocks for skipped tools in native protocol (#9457)
1 parent 5c5a893 commit 08027ad

File tree

2 files changed

+189
-10
lines changed

2 files changed

+189
-10
lines changed

src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,159 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calls", () =>
202202
// Should have fallback text
203203
expect(toolResult.content).toBeTruthy()
204204
})
205+
206+
describe("Multiple tool calls handling", () => {
207+
it("should send tool_result with is_error for skipped tools in native protocol when didRejectTool is true", async () => {
208+
// Simulate multiple tool calls with native protocol (all have IDs)
209+
const toolCallId1 = "tool_call_001"
210+
const toolCallId2 = "tool_call_002"
211+
212+
mockTask.assistantMessageContent = [
213+
{
214+
type: "tool_use",
215+
id: toolCallId1,
216+
name: "read_file",
217+
params: { path: "test.txt" },
218+
},
219+
{
220+
type: "tool_use",
221+
id: toolCallId2,
222+
name: "write_to_file",
223+
params: { path: "output.txt", content: "test" },
224+
},
225+
]
226+
227+
// First tool is rejected
228+
mockTask.didRejectTool = true
229+
230+
// Process the second tool (should be skipped)
231+
mockTask.currentStreamingContentIndex = 1
232+
await presentAssistantMessage(mockTask)
233+
234+
// Find the tool_result for the second tool
235+
const toolResult = mockTask.userMessageContent.find(
236+
(item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId2,
237+
)
238+
239+
// Verify that a tool_result block was created (not a text block)
240+
expect(toolResult).toBeDefined()
241+
expect(toolResult.tool_use_id).toBe(toolCallId2)
242+
expect(toolResult.is_error).toBe(true)
243+
expect(toolResult.content).toContain("due to user rejecting a previous tool")
244+
245+
// Ensure no text blocks were added for this rejection
246+
const textBlocks = mockTask.userMessageContent.filter(
247+
(item: any) => item.type === "text" && item.text.includes("due to user rejecting"),
248+
)
249+
expect(textBlocks.length).toBe(0)
250+
})
251+
252+
it("should send tool_result with is_error for skipped tools in native protocol when didAlreadyUseTool is true", async () => {
253+
// Simulate multiple tool calls with native protocol
254+
const toolCallId1 = "tool_call_003"
255+
const toolCallId2 = "tool_call_004"
256+
257+
mockTask.assistantMessageContent = [
258+
{
259+
type: "tool_use",
260+
id: toolCallId1,
261+
name: "read_file",
262+
params: { path: "test.txt" },
263+
},
264+
{
265+
type: "tool_use",
266+
id: toolCallId2,
267+
name: "write_to_file",
268+
params: { path: "output.txt", content: "test" },
269+
},
270+
]
271+
272+
// First tool was already used
273+
mockTask.didAlreadyUseTool = true
274+
275+
// Process the second tool (should be skipped)
276+
mockTask.currentStreamingContentIndex = 1
277+
await presentAssistantMessage(mockTask)
278+
279+
// Find the tool_result for the second tool
280+
const toolResult = mockTask.userMessageContent.find(
281+
(item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId2,
282+
)
283+
284+
// Verify that a tool_result block was created (not a text block)
285+
expect(toolResult).toBeDefined()
286+
expect(toolResult.tool_use_id).toBe(toolCallId2)
287+
expect(toolResult.is_error).toBe(true)
288+
expect(toolResult.content).toContain("was not executed because a tool has already been used")
289+
290+
// Ensure no text blocks were added for this rejection
291+
const textBlocks = mockTask.userMessageContent.filter(
292+
(item: any) => item.type === "text" && item.text.includes("was not executed because"),
293+
)
294+
expect(textBlocks.length).toBe(0)
295+
})
296+
297+
it("should send text blocks for skipped tools in XML protocol (no tool IDs)", async () => {
298+
// Simulate multiple tool calls with XML protocol (no IDs)
299+
mockTask.assistantMessageContent = [
300+
{
301+
type: "tool_use",
302+
// No ID = XML protocol
303+
name: "read_file",
304+
params: { path: "test.txt" },
305+
},
306+
{
307+
type: "tool_use",
308+
// No ID = XML protocol
309+
name: "write_to_file",
310+
params: { path: "output.txt", content: "test" },
311+
},
312+
]
313+
314+
// First tool is rejected
315+
mockTask.didRejectTool = true
316+
317+
// Process the second tool (should be skipped)
318+
mockTask.currentStreamingContentIndex = 1
319+
await presentAssistantMessage(mockTask)
320+
321+
// For XML protocol, should add text block (not tool_result)
322+
const textBlocks = mockTask.userMessageContent.filter(
323+
(item: any) => item.type === "text" && item.text.includes("due to user rejecting"),
324+
)
325+
expect(textBlocks.length).toBeGreaterThan(0)
326+
327+
// Ensure no tool_result blocks were added
328+
const toolResults = mockTask.userMessageContent.filter((item: any) => item.type === "tool_result")
329+
expect(toolResults.length).toBe(0)
330+
})
331+
332+
it("should handle partial tool blocks when didRejectTool is true in native protocol", async () => {
333+
const toolCallId = "tool_call_005"
334+
335+
mockTask.assistantMessageContent = [
336+
{
337+
type: "tool_use",
338+
id: toolCallId,
339+
name: "write_to_file",
340+
params: { path: "output.txt", content: "test" },
341+
partial: true, // Partial tool block
342+
},
343+
]
344+
345+
mockTask.didRejectTool = true
346+
347+
await presentAssistantMessage(mockTask)
348+
349+
// Find the tool_result
350+
const toolResult = mockTask.userMessageContent.find(
351+
(item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId,
352+
)
353+
354+
// Verify tool_result was created for partial block
355+
expect(toolResult).toBeDefined()
356+
expect(toolResult.is_error).toBe(true)
357+
expect(toolResult.content).toContain("was interrupted and not executed")
358+
})
359+
})
205360
})

src/core/assistant-message/presentAssistantMessage.ts

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -252,16 +252,25 @@ export async function presentAssistantMessage(cline: Task) {
252252

253253
if (cline.didRejectTool) {
254254
// Ignore any tool content after user has rejected tool once.
255-
if (!block.partial) {
255+
// For native protocol, we must send a tool_result for every tool_use to avoid API errors
256+
const toolCallId = block.id
257+
const errorMessage = !block.partial
258+
? `Skipping tool ${toolDescription()} due to user rejecting a previous tool.`
259+
: `Tool ${toolDescription()} was interrupted and not executed due to user rejecting a previous tool.`
260+
261+
if (toolCallId) {
262+
// Native protocol: MUST send tool_result for every tool_use
256263
cline.userMessageContent.push({
257-
type: "text",
258-
text: `Skipping tool ${toolDescription()} due to user rejecting a previous tool.`,
259-
})
264+
type: "tool_result",
265+
tool_use_id: toolCallId,
266+
content: errorMessage,
267+
is_error: true,
268+
} as Anthropic.ToolResultBlockParam)
260269
} else {
261-
// Partial tool after user rejected a previous tool.
270+
// XML protocol: send as text
262271
cline.userMessageContent.push({
263272
type: "text",
264-
text: `Tool ${toolDescription()} was interrupted and not executed due to user rejecting a previous tool.`,
273+
text: errorMessage,
265274
})
266275
}
267276

@@ -270,10 +279,25 @@ export async function presentAssistantMessage(cline: Task) {
270279

271280
if (cline.didAlreadyUseTool) {
272281
// Ignore any content after a tool has already been used.
273-
cline.userMessageContent.push({
274-
type: "text",
275-
text: `Tool [${block.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message. You must assess the first tool's result before proceeding to use the next tool.`,
276-
})
282+
// For native protocol, we must send a tool_result for every tool_use to avoid API errors
283+
const toolCallId = block.id
284+
const errorMessage = `Tool [${block.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message. You must assess the first tool's result before proceeding to use the next tool.`
285+
286+
if (toolCallId) {
287+
// Native protocol: MUST send tool_result for every tool_use
288+
cline.userMessageContent.push({
289+
type: "tool_result",
290+
tool_use_id: toolCallId,
291+
content: errorMessage,
292+
is_error: true,
293+
} as Anthropic.ToolResultBlockParam)
294+
} else {
295+
// XML protocol: send as text
296+
cline.userMessageContent.push({
297+
type: "text",
298+
text: errorMessage,
299+
})
300+
}
277301

278302
break
279303
}

0 commit comments

Comments
 (0)