mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
fix: preserve tool blocks for native protocol in conversation history (#9319)
This commit is contained in:
Daniel
GitHub
parent
0e51a1ab9b
commit
fc19e7620b
3 changed files with 316 additions and 32 deletions
|
|
@ -1415,8 +1415,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
// even if it goes out of sync with cline messages.
|
||||
let existingApiConversationHistory: ApiMessage[] = await this.getSavedApiConversationHistory()
|
||||
|
||||
// v2.0 xml tags refactor caveat: since we don't use tools anymore, we need to replace all tool use blocks with a text block since the API disallows conversations with tool uses and no tool schema
|
||||
// Now also protocol-aware: format according to current protocol setting
|
||||
// v2.0 xml tags refactor caveat: since we don't use tools anymore for XML protocol,
|
||||
// we need to replace all tool use blocks with a text block since the API disallows
|
||||
// conversations with tool uses and no tool schema.
|
||||
// For native protocol, we preserve tool_use and tool_result blocks as they're expected by the API.
|
||||
const state = await this.providerRef.deref()?.getState()
|
||||
const protocol = resolveToolProtocol(
|
||||
this.apiConfiguration,
|
||||
|
|
@ -1426,37 +1428,41 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
)
|
||||
const useNative = isNativeProtocol(protocol)
|
||||
|
||||
const conversationWithoutToolBlocks = existingApiConversationHistory.map((message) => {
|
||||
if (Array.isArray(message.content)) {
|
||||
const newContent = message.content.map((block) => {
|
||||
if (block.type === "tool_use") {
|
||||
// Format tool invocation based on protocol
|
||||
const params = block.input as Record<string, any>
|
||||
const formattedText = formatToolInvocation(block.name, params, protocol)
|
||||
// Only convert tool blocks to text for XML protocol
|
||||
// For native protocol, the API expects proper tool_use/tool_result structure
|
||||
if (!useNative) {
|
||||
const conversationWithoutToolBlocks = existingApiConversationHistory.map((message) => {
|
||||
if (Array.isArray(message.content)) {
|
||||
const newContent = message.content.map((block) => {
|
||||
if (block.type === "tool_use") {
|
||||
// Format tool invocation based on protocol
|
||||
const params = block.input as Record<string, any>
|
||||
const formattedText = formatToolInvocation(block.name, params, protocol)
|
||||
|
||||
return {
|
||||
type: "text",
|
||||
text: formattedText,
|
||||
} as Anthropic.Messages.TextBlockParam
|
||||
} else if (block.type === "tool_result") {
|
||||
// Convert block.content to text block array, removing images
|
||||
const contentAsTextBlocks = Array.isArray(block.content)
|
||||
? block.content.filter((item) => item.type === "text")
|
||||
: [{ type: "text", text: block.content }]
|
||||
const textContent = contentAsTextBlocks.map((item) => item.text).join("\n\n")
|
||||
const toolName = findToolName(block.tool_use_id, existingApiConversationHistory)
|
||||
return {
|
||||
type: "text",
|
||||
text: `[${toolName} Result]\n\n${textContent}`,
|
||||
} as Anthropic.Messages.TextBlockParam
|
||||
}
|
||||
return block
|
||||
})
|
||||
return { ...message, content: newContent }
|
||||
}
|
||||
return message
|
||||
})
|
||||
existingApiConversationHistory = conversationWithoutToolBlocks
|
||||
return {
|
||||
type: "text",
|
||||
text: formattedText,
|
||||
} as Anthropic.Messages.TextBlockParam
|
||||
} else if (block.type === "tool_result") {
|
||||
// Convert block.content to text block array, removing images
|
||||
const contentAsTextBlocks = Array.isArray(block.content)
|
||||
? block.content.filter((item) => item.type === "text")
|
||||
: [{ type: "text", text: block.content }]
|
||||
const textContent = contentAsTextBlocks.map((item) => item.text).join("\n\n")
|
||||
const toolName = findToolName(block.tool_use_id, existingApiConversationHistory)
|
||||
return {
|
||||
type: "text",
|
||||
text: `[${toolName} Result]\n\n${textContent}`,
|
||||
} as Anthropic.Messages.TextBlockParam
|
||||
}
|
||||
return block
|
||||
})
|
||||
return { ...message, content: newContent }
|
||||
}
|
||||
return message
|
||||
})
|
||||
existingApiConversationHistory = conversationWithoutToolBlocks
|
||||
}
|
||||
|
||||
// FIXME: remove tool use blocks altogether
|
||||
|
||||
|
|
|
|||
200
src/core/task/__tests__/task-tool-history.spec.ts
Normal file
200
src/core/task/__tests__/task-tool-history.spec.ts
Normal file
|
|
@ -0,0 +1,200 @@
|
|||
import { describe, it, expect, beforeEach, vi } from "vitest"
|
||||
import { Anthropic } from "@anthropic-ai/sdk"
|
||||
import { TOOL_PROTOCOL } from "@roo-code/types"
|
||||
import { resolveToolProtocol } from "../../../utils/resolveToolProtocol"
|
||||
|
||||
describe("Task Tool History Handling", () => {
|
||||
describe("resumeTaskFromHistory tool block preservation", () => {
|
||||
it("should preserve tool_use and tool_result blocks for native protocol", () => {
|
||||
// Mock API conversation history with tool blocks
|
||||
const apiHistory: any[] = [
|
||||
{
|
||||
role: "user",
|
||||
content: "Read the file config.json",
|
||||
ts: Date.now(),
|
||||
},
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "I'll read that file for you.",
|
||||
},
|
||||
{
|
||||
type: "tool_use",
|
||||
id: "toolu_123",
|
||||
name: "read_file",
|
||||
input: { path: "config.json" },
|
||||
},
|
||||
],
|
||||
ts: Date.now(),
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "tool_result",
|
||||
tool_use_id: "toolu_123",
|
||||
content: '{"setting": "value"}',
|
||||
},
|
||||
],
|
||||
ts: Date.now(),
|
||||
},
|
||||
]
|
||||
|
||||
// Simulate the protocol check
|
||||
const mockApiConfiguration = { apiProvider: "roo" as const }
|
||||
const mockModelInfo = { supportsNativeTools: true }
|
||||
const mockExperiments = {}
|
||||
|
||||
const protocol = TOOL_PROTOCOL.NATIVE
|
||||
|
||||
// Test the logic that should NOT convert tool blocks for native protocol
|
||||
const useNative = protocol === TOOL_PROTOCOL.NATIVE
|
||||
|
||||
if (!useNative) {
|
||||
// This block should NOT execute for native protocol
|
||||
throw new Error("Should not convert tool blocks for native protocol")
|
||||
}
|
||||
|
||||
// Verify tool blocks are preserved
|
||||
const assistantMessage = apiHistory[1]
|
||||
const userMessage = apiHistory[2]
|
||||
|
||||
expect(assistantMessage.content).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
type: "tool_use",
|
||||
id: "toolu_123",
|
||||
name: "read_file",
|
||||
}),
|
||||
]),
|
||||
)
|
||||
|
||||
expect(userMessage.content).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
type: "tool_result",
|
||||
tool_use_id: "toolu_123",
|
||||
}),
|
||||
]),
|
||||
)
|
||||
})
|
||||
|
||||
it("should convert tool blocks to text for XML protocol", () => {
|
||||
// Mock API conversation history with tool blocks
|
||||
const apiHistory: any[] = [
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{
|
||||
type: "tool_use",
|
||||
id: "toolu_123",
|
||||
name: "read_file",
|
||||
input: { path: "config.json" },
|
||||
},
|
||||
],
|
||||
ts: Date.now(),
|
||||
},
|
||||
]
|
||||
|
||||
// Simulate XML protocol - tool blocks should be converted to text
|
||||
const protocol = "xml"
|
||||
const useNative = false // XML protocol is not native
|
||||
|
||||
// For XML protocol, we should convert tool blocks
|
||||
if (!useNative) {
|
||||
const conversationWithoutToolBlocks = apiHistory.map((message) => {
|
||||
if (Array.isArray(message.content)) {
|
||||
const newContent = message.content.map((block: any) => {
|
||||
if (block.type === "tool_use") {
|
||||
return {
|
||||
type: "text",
|
||||
text: `<read_file>\n<path>\nconfig.json\n</path>\n</read_file>`,
|
||||
}
|
||||
}
|
||||
return block
|
||||
})
|
||||
return { ...message, content: newContent }
|
||||
}
|
||||
return message
|
||||
})
|
||||
|
||||
// Verify tool blocks were converted to text
|
||||
expect(conversationWithoutToolBlocks[0].content[0].type).toBe("text")
|
||||
expect(conversationWithoutToolBlocks[0].content[0].text).toContain("<read_file>")
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe("convertToOpenAiMessages format", () => {
|
||||
it("should properly convert tool_use to tool_calls format", () => {
|
||||
const anthropicMessage: Anthropic.Messages.MessageParam = {
|
||||
role: "assistant",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "I'll read that file.",
|
||||
},
|
||||
{
|
||||
type: "tool_use",
|
||||
id: "toolu_123",
|
||||
name: "read_file",
|
||||
input: { path: "config.json" },
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
// Simulate what convertToOpenAiMessages does
|
||||
const toolUseBlocks = (anthropicMessage.content as any[]).filter((block) => block.type === "tool_use")
|
||||
|
||||
const tool_calls = toolUseBlocks.map((toolMessage) => ({
|
||||
id: toolMessage.id,
|
||||
type: "function" as const,
|
||||
function: {
|
||||
name: toolMessage.name,
|
||||
arguments: JSON.stringify(toolMessage.input),
|
||||
},
|
||||
}))
|
||||
|
||||
expect(tool_calls).toHaveLength(1)
|
||||
expect(tool_calls[0]).toEqual({
|
||||
id: "toolu_123",
|
||||
type: "function",
|
||||
function: {
|
||||
name: "read_file",
|
||||
arguments: '{"path":"config.json"}',
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it("should properly convert tool_result to tool role messages", () => {
|
||||
const anthropicMessage: Anthropic.Messages.MessageParam = {
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "tool_result",
|
||||
tool_use_id: "toolu_123",
|
||||
content: '{"setting": "value"}',
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
// Simulate what convertToOpenAiMessages does
|
||||
const toolMessages = (anthropicMessage.content as any[]).filter((block) => block.type === "tool_result")
|
||||
|
||||
const openAiToolMessages = toolMessages.map((toolMessage) => ({
|
||||
role: "tool" as const,
|
||||
tool_call_id: toolMessage.tool_use_id,
|
||||
content: typeof toolMessage.content === "string" ? toolMessage.content : toolMessage.content[0].text,
|
||||
}))
|
||||
|
||||
expect(openAiToolMessages).toHaveLength(1)
|
||||
expect(openAiToolMessages[0]).toEqual({
|
||||
role: "tool",
|
||||
tool_call_id: "toolu_123",
|
||||
content: '{"setting": "value"}',
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
78
src/core/task/__tests__/task-xml-protocol-regression.spec.ts
Normal file
78
src/core/task/__tests__/task-xml-protocol-regression.spec.ts
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
import { describe, it, expect } from "vitest"
|
||||
import { formatToolInvocation } from "../../tools/helpers/toolResultFormatting"
|
||||
|
||||
/**
|
||||
* Regression tests to ensure XML protocol behavior remains unchanged
|
||||
* after adding native protocol support.
|
||||
*/
|
||||
describe("XML Protocol Regression Tests", () => {
|
||||
it("should format tool invocations as XML tags for xml protocol", () => {
|
||||
const result = formatToolInvocation(
|
||||
"read_file",
|
||||
{ path: "config.json", start_line: "1", end_line: "10" },
|
||||
"xml",
|
||||
)
|
||||
|
||||
expect(result).toContain("<read_file>")
|
||||
expect(result).toContain("<path>")
|
||||
expect(result).toContain("config.json")
|
||||
expect(result).toContain("</path>")
|
||||
expect(result).toContain("<start_line>")
|
||||
expect(result).toContain("1")
|
||||
expect(result).toContain("</start_line>")
|
||||
expect(result).toContain("</read_file>")
|
||||
})
|
||||
|
||||
it("should handle complex nested structures in XML format", () => {
|
||||
const result = formatToolInvocation(
|
||||
"execute_command",
|
||||
{
|
||||
command: "npm install",
|
||||
cwd: "/home/user/project",
|
||||
},
|
||||
"xml",
|
||||
)
|
||||
|
||||
expect(result).toContain("<execute_command>")
|
||||
expect(result).toContain("<command>")
|
||||
expect(result).toContain("npm install")
|
||||
expect(result).toContain("</command>")
|
||||
expect(result).toContain("<cwd>")
|
||||
expect(result).toContain("/home/user/project")
|
||||
expect(result).toContain("</cwd>")
|
||||
expect(result).toContain("</execute_command>")
|
||||
})
|
||||
|
||||
it("should handle empty parameters correctly in XML format", () => {
|
||||
const result = formatToolInvocation("list_files", {}, "xml")
|
||||
|
||||
expect(result).toBe("<list_files>\n\n</list_files>")
|
||||
})
|
||||
|
||||
it("should preserve XML format for tool results in conversation history", () => {
|
||||
// Simulate what happens in resumeTaskFromHistory for XML protocol
|
||||
const useNative = false // XML protocol
|
||||
|
||||
const mockToolUse = {
|
||||
type: "tool_use",
|
||||
id: "toolu_123",
|
||||
name: "read_file",
|
||||
input: { path: "test.ts" },
|
||||
}
|
||||
|
||||
if (!useNative) {
|
||||
// This is the conversion logic that should happen for XML
|
||||
const converted = {
|
||||
type: "text",
|
||||
text: formatToolInvocation(mockToolUse.name, mockToolUse.input as Record<string, any>, "xml"),
|
||||
}
|
||||
|
||||
expect(converted.type).toBe("text")
|
||||
expect(converted.text).toContain("<read_file>")
|
||||
expect(converted.text).toContain("<path>")
|
||||
expect(converted.text).toContain("test.ts")
|
||||
} else {
|
||||
throw new Error("Should not reach here for XML protocol")
|
||||
}
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Reference in a new issue