mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: prevent tool parsing within code blocks
- Added code block detection to parseAssistantMessage.ts - Added code block detection to parseAssistantMessageV2.ts - Added code block detection to AssistantMessageParser.ts - Added comprehensive tests for code block scenarios - Fixes issue where tool XML tags in code examples were incorrectly parsed as actual tool invocations Fixes #8242
This commit is contained in:
parent
0e1b23d09c
commit
eb0add23d2
4 changed files with 378 additions and 104 deletions
|
|
@ -17,6 +17,9 @@ export class AssistantMessageParser {
|
|||
private readonly MAX_ACCUMULATOR_SIZE = 1024 * 1024 // 1MB limit
|
||||
private readonly MAX_PARAM_LENGTH = 1024 * 100 // 100KB per parameter limit
|
||||
private accumulator = ""
|
||||
private inCodeBlock = false
|
||||
private inInlineCode = false
|
||||
private codeBlockDelimiterCount = 0
|
||||
|
||||
/**
|
||||
* Initialize a new AssistantMessageParser instance.
|
||||
|
|
@ -37,6 +40,9 @@ export class AssistantMessageParser {
|
|||
this.currentParamName = undefined
|
||||
this.currentParamValueStartIndex = 0
|
||||
this.accumulator = ""
|
||||
this.inCodeBlock = false
|
||||
this.inInlineCode = false
|
||||
this.codeBlockDelimiterCount = 0
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -63,6 +69,41 @@ export class AssistantMessageParser {
|
|||
this.accumulator += char
|
||||
const currentPosition = accumulatorStartLength + i
|
||||
|
||||
// Track code blocks and inline code
|
||||
if (char === "`") {
|
||||
this.codeBlockDelimiterCount++
|
||||
if (this.codeBlockDelimiterCount === 3) {
|
||||
this.inCodeBlock = !this.inCodeBlock
|
||||
this.codeBlockDelimiterCount = 0
|
||||
this.inInlineCode = false // Code blocks take precedence
|
||||
}
|
||||
} else {
|
||||
// If we had one backtick and now a different char, toggle inline code
|
||||
if (this.codeBlockDelimiterCount === 1 && !this.inCodeBlock) {
|
||||
this.inInlineCode = !this.inInlineCode
|
||||
}
|
||||
this.codeBlockDelimiterCount = 0
|
||||
}
|
||||
|
||||
// Skip tool parsing if we're inside code blocks or inline code
|
||||
if (this.inCodeBlock || this.inInlineCode) {
|
||||
// Continue accumulating text content
|
||||
if (this.currentTextContent === undefined && !this.currentToolUse) {
|
||||
this.currentTextContentStartIndex = currentPosition
|
||||
this.currentTextContent = {
|
||||
type: "text",
|
||||
content: this.accumulator.slice(this.currentTextContentStartIndex).trim(),
|
||||
partial: true,
|
||||
}
|
||||
// Add the new text content to contentBlocks immediately
|
||||
this.contentBlocks.push(this.currentTextContent)
|
||||
} else if (this.currentTextContent) {
|
||||
// Update the existing text content
|
||||
this.currentTextContent.content = this.accumulator.slice(this.currentTextContentStartIndex).trim()
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// There should not be a param without a tool use.
|
||||
if (this.currentToolUse && this.currentParamName) {
|
||||
const currentParamValue = this.accumulator.slice(this.currentParamValueStartIndex)
|
||||
|
|
@ -159,47 +200,50 @@ export class AssistantMessageParser {
|
|||
|
||||
for (const toolUseOpeningTag of possibleToolUseOpeningTags) {
|
||||
if (this.accumulator.endsWith(toolUseOpeningTag)) {
|
||||
// Extract and validate the tool name
|
||||
const extractedToolName = toolUseOpeningTag.slice(1, -1)
|
||||
// Only process tool tags if we're not in code blocks
|
||||
if (!this.inCodeBlock && !this.inInlineCode) {
|
||||
// Extract and validate the tool name
|
||||
const extractedToolName = toolUseOpeningTag.slice(1, -1)
|
||||
|
||||
// Check if the extracted tool name is valid
|
||||
if (!toolNames.includes(extractedToolName as ToolName)) {
|
||||
// Invalid tool name, treat as plain text and continue
|
||||
continue
|
||||
// Check if the extracted tool name is valid
|
||||
if (!toolNames.includes(extractedToolName as ToolName)) {
|
||||
// Invalid tool name, treat as plain text and continue
|
||||
continue
|
||||
}
|
||||
|
||||
// Start of a new tool use.
|
||||
this.currentToolUse = {
|
||||
type: "tool_use",
|
||||
name: extractedToolName as ToolName,
|
||||
params: {},
|
||||
partial: true,
|
||||
}
|
||||
|
||||
this.currentToolUseStartIndex = this.accumulator.length
|
||||
|
||||
// This also indicates the end of the current text content.
|
||||
if (this.currentTextContent) {
|
||||
this.currentTextContent.partial = false
|
||||
|
||||
// Remove the partially accumulated tool use tag from the
|
||||
// end of text (<tool).
|
||||
this.currentTextContent.content = this.currentTextContent.content
|
||||
.slice(0, -toolUseOpeningTag.slice(0, -1).length)
|
||||
.trim()
|
||||
|
||||
// No need to push, currentTextContent is already in contentBlocks
|
||||
this.currentTextContent = undefined
|
||||
}
|
||||
|
||||
// Immediately push new tool_use block as partial
|
||||
let idx = this.contentBlocks.findIndex((block) => block === this.currentToolUse)
|
||||
if (idx === -1) {
|
||||
this.contentBlocks.push(this.currentToolUse)
|
||||
}
|
||||
|
||||
didStartToolUse = true
|
||||
break
|
||||
}
|
||||
|
||||
// Start of a new tool use.
|
||||
this.currentToolUse = {
|
||||
type: "tool_use",
|
||||
name: extractedToolName as ToolName,
|
||||
params: {},
|
||||
partial: true,
|
||||
}
|
||||
|
||||
this.currentToolUseStartIndex = this.accumulator.length
|
||||
|
||||
// This also indicates the end of the current text content.
|
||||
if (this.currentTextContent) {
|
||||
this.currentTextContent.partial = false
|
||||
|
||||
// Remove the partially accumulated tool use tag from the
|
||||
// end of text (<tool).
|
||||
this.currentTextContent.content = this.currentTextContent.content
|
||||
.slice(0, -toolUseOpeningTag.slice(0, -1).length)
|
||||
.trim()
|
||||
|
||||
// No need to push, currentTextContent is already in contentBlocks
|
||||
this.currentTextContent = undefined
|
||||
}
|
||||
|
||||
// Immediately push new tool_use block as partial
|
||||
let idx = this.contentBlocks.findIndex((block) => block === this.currentToolUse)
|
||||
if (idx === -1) {
|
||||
this.contentBlocks.push(this.currentToolUse)
|
||||
}
|
||||
|
||||
didStartToolUse = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -336,5 +336,154 @@ const isEmptyTextContent = (block: AssistantMessageContent) =>
|
|||
expect((result[5] as ToolUse).name).toBe("execute_command")
|
||||
})
|
||||
})
|
||||
|
||||
describe("code block handling", () => {
|
||||
it("should not parse tool tags within code blocks", () => {
|
||||
const message = `Here's an example of the ask_followup_question tool:
|
||||
|
||||
\`\`\`xml
|
||||
<ask_followup_question>
|
||||
<question>What is the path to the frontend-config.json file?</question>
|
||||
<follow_up>
|
||||
<suggest>./src/frontend-config.json</suggest>
|
||||
<suggest>./config/frontend-config.json</suggest>
|
||||
<suggest>./frontend-config.json</suggest>
|
||||
</follow_up>
|
||||
</ask_followup_question>
|
||||
\`\`\`
|
||||
|
||||
This is how you use it.`
|
||||
|
||||
const result = parser(message)
|
||||
|
||||
// Should only have text content, no tool use
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("text")
|
||||
const textContent = result[0] as TextContent
|
||||
expect(textContent.content).toContain("Here's an example")
|
||||
expect(textContent.content).toContain("<ask_followup_question>")
|
||||
expect(textContent.content).toContain("This is how you use it")
|
||||
expect(textContent.partial).toBe(true)
|
||||
})
|
||||
|
||||
it("should not parse tool tags within inline code", () => {
|
||||
const message = "Use the \`<read_file><path>file.ts</path></read_file>\` tool to read files."
|
||||
const result = parser(message)
|
||||
|
||||
// Should only have text content, no tool use
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("text")
|
||||
const textContent = result[0] as TextContent
|
||||
expect(textContent.content).toBe(message)
|
||||
expect(textContent.partial).toBe(true)
|
||||
})
|
||||
|
||||
it("should parse tool tags outside of code blocks", () => {
|
||||
const message = `Here's an example:
|
||||
|
||||
\`\`\`
|
||||
<example>code</example>
|
||||
\`\`\`
|
||||
|
||||
Now let me read a file:
|
||||
|
||||
<read_file><path>test.ts</path></read_file>`
|
||||
|
||||
const result = parser(message)
|
||||
|
||||
// Should have text content and a tool use
|
||||
expect(result).toHaveLength(2)
|
||||
|
||||
// First should be text containing the code block
|
||||
expect(result[0].type).toBe("text")
|
||||
const textContent = result[0] as TextContent
|
||||
expect(textContent.content).toContain("Here's an example")
|
||||
expect(textContent.content).toContain("<example>code</example>")
|
||||
expect(textContent.content).toContain("Now let me read a file:")
|
||||
expect(textContent.partial).toBe(false)
|
||||
|
||||
// Second should be the actual tool use
|
||||
expect(result[1].type).toBe("tool_use")
|
||||
const toolUse = result[1] as ToolUse
|
||||
expect(toolUse.name).toBe("read_file")
|
||||
expect(toolUse.params.path).toBe("test.ts")
|
||||
expect(toolUse.partial).toBe(false)
|
||||
})
|
||||
|
||||
it("should handle mixed inline code and actual tool uses", () => {
|
||||
const message =
|
||||
"The tool \`<read_file>\` is used like this: <read_file><path>actual.ts</path></read_file>"
|
||||
const result = parser(message)
|
||||
|
||||
// Should have text and tool use
|
||||
expect(result).toHaveLength(2)
|
||||
|
||||
expect(result[0].type).toBe("text")
|
||||
const textContent = result[0] as TextContent
|
||||
expect(textContent.content).toContain("The tool \`<read_file>\` is used like this:")
|
||||
|
||||
expect(result[1].type).toBe("tool_use")
|
||||
const toolUse = result[1] as ToolUse
|
||||
expect(toolUse.name).toBe("read_file")
|
||||
expect(toolUse.params.path).toBe("actual.ts")
|
||||
})
|
||||
|
||||
it("should handle code blocks with triple backticks inside", () => {
|
||||
const message = `Here's a markdown example:
|
||||
|
||||
\`\`\`markdown
|
||||
# Example
|
||||
\`\`\`python
|
||||
print("hello")
|
||||
\`\`\`
|
||||
<read_file><path>not_a_tool.ts</path></read_file>
|
||||
\`\`\`
|
||||
|
||||
That was the example.`
|
||||
|
||||
const result = parser(message)
|
||||
|
||||
// Should only have text content
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("text")
|
||||
const textContent = result[0] as TextContent
|
||||
expect(textContent.content).toContain("Here's a markdown example")
|
||||
expect(textContent.content).toContain("<read_file><path>not_a_tool.ts</path></read_file>")
|
||||
expect(textContent.content).toContain("That was the example")
|
||||
})
|
||||
|
||||
it("should correctly handle the exact issue scenario", () => {
|
||||
const message = `## ask_followup_question
|
||||
Description: Ask the user a question to gather additional information needed to complete the task.
|
||||
|
||||
Usage:
|
||||
\`\`\`
|
||||
<ask_followup_question>
|
||||
<question>Your question here</question>
|
||||
<follow_up>
|
||||
<suggest>First suggestion</suggest>
|
||||
<suggest mode="code">Action with mode switch</suggest>
|
||||
</follow_up>
|
||||
</ask_followup_question>
|
||||
\`\`\`
|
||||
|
||||
This tool helps gather information.`
|
||||
|
||||
const result = parser(message)
|
||||
|
||||
// Should only have text content, no tool invocation
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("text")
|
||||
const textContent = result[0] as TextContent
|
||||
expect(textContent.content).toContain("ask_followup_question")
|
||||
expect(textContent.content).toContain("Description:")
|
||||
expect(textContent.content).toContain("<ask_followup_question>")
|
||||
expect(textContent.content).toContain("This tool helps gather information")
|
||||
|
||||
// Ensure no tool_use blocks were created
|
||||
const toolUses = result.filter((block) => block.type === "tool_use")
|
||||
expect(toolUses).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -13,11 +13,49 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
|
|||
let currentParamName: ToolParamName | undefined = undefined
|
||||
let currentParamValueStartIndex = 0
|
||||
let accumulator = ""
|
||||
let inCodeBlock = false
|
||||
let inInlineCode = false
|
||||
let codeBlockDelimiterCount = 0
|
||||
let lastTwoChars = ""
|
||||
|
||||
for (let i = 0; i < assistantMessage.length; i++) {
|
||||
const char = assistantMessage[i]
|
||||
accumulator += char
|
||||
|
||||
// Track last two characters for inline code detection
|
||||
lastTwoChars = (lastTwoChars + char).slice(-2)
|
||||
|
||||
// Check for code block delimiters (```)
|
||||
if (char === "`") {
|
||||
codeBlockDelimiterCount++
|
||||
if (codeBlockDelimiterCount === 3) {
|
||||
inCodeBlock = !inCodeBlock
|
||||
codeBlockDelimiterCount = 0
|
||||
}
|
||||
} else {
|
||||
// Check for inline code (single backtick)
|
||||
if (codeBlockDelimiterCount === 1 && !inCodeBlock) {
|
||||
inInlineCode = !inInlineCode
|
||||
}
|
||||
codeBlockDelimiterCount = 0
|
||||
}
|
||||
|
||||
// Skip tool parsing if we're inside a code block or inline code
|
||||
if (inCodeBlock || inInlineCode) {
|
||||
// If we're in text content, keep accumulating
|
||||
if (currentTextContent === undefined && !currentToolUse) {
|
||||
currentTextContentStartIndex = i
|
||||
currentTextContent = {
|
||||
type: "text",
|
||||
content: accumulator.slice(currentTextContentStartIndex).trim(),
|
||||
partial: true,
|
||||
}
|
||||
} else if (currentTextContent) {
|
||||
currentTextContent.content = accumulator.slice(currentTextContentStartIndex).trim()
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// There should not be a param without a tool use.
|
||||
if (currentToolUse && currentParamName) {
|
||||
const currentParamValue = accumulator.slice(currentParamValueStartIndex)
|
||||
|
|
@ -97,32 +135,35 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
|
|||
|
||||
for (const toolUseOpeningTag of possibleToolUseOpeningTags) {
|
||||
if (accumulator.endsWith(toolUseOpeningTag)) {
|
||||
// Start of a new tool use.
|
||||
currentToolUse = {
|
||||
type: "tool_use",
|
||||
name: toolUseOpeningTag.slice(1, -1) as ToolName,
|
||||
params: {},
|
||||
partial: true,
|
||||
// Only start a new tool use if we're not in a code block
|
||||
if (!inCodeBlock && !inInlineCode) {
|
||||
// Start of a new tool use.
|
||||
currentToolUse = {
|
||||
type: "tool_use",
|
||||
name: toolUseOpeningTag.slice(1, -1) as ToolName,
|
||||
params: {},
|
||||
partial: true,
|
||||
}
|
||||
|
||||
currentToolUseStartIndex = accumulator.length
|
||||
|
||||
// This also indicates the end of the current text content.
|
||||
if (currentTextContent) {
|
||||
currentTextContent.partial = false
|
||||
|
||||
// Remove the partially accumulated tool use tag from the
|
||||
// end of text (<tool).
|
||||
currentTextContent.content = currentTextContent.content
|
||||
.slice(0, -toolUseOpeningTag.slice(0, -1).length)
|
||||
.trim()
|
||||
|
||||
contentBlocks.push(currentTextContent)
|
||||
currentTextContent = undefined
|
||||
}
|
||||
|
||||
didStartToolUse = true
|
||||
break
|
||||
}
|
||||
|
||||
currentToolUseStartIndex = accumulator.length
|
||||
|
||||
// This also indicates the end of the current text content.
|
||||
if (currentTextContent) {
|
||||
currentTextContent.partial = false
|
||||
|
||||
// Remove the partially accumulated tool use tag from the
|
||||
// end of text (<tool).
|
||||
currentTextContent.content = currentTextContent.content
|
||||
.slice(0, -toolUseOpeningTag.slice(0, -1).length)
|
||||
.trim()
|
||||
|
||||
contentBlocks.push(currentTextContent)
|
||||
currentTextContent = undefined
|
||||
}
|
||||
|
||||
didStartToolUse = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -47,6 +47,11 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
|
|||
let currentParamValueStart = 0 // Index *after* the opening tag of the current param.
|
||||
let currentParamName: ToolParamName | undefined = undefined
|
||||
|
||||
// Track code block state
|
||||
let inCodeBlock = false
|
||||
let inInlineCode = false
|
||||
let backtickCount = 0
|
||||
|
||||
// Precompute tags for faster lookups.
|
||||
const toolUseOpenTags = new Map<string, ToolName>()
|
||||
const toolParamOpenTags = new Map<string, ToolParamName>()
|
||||
|
|
@ -63,6 +68,38 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
|
|||
|
||||
for (let i = 0; i < len; i++) {
|
||||
const currentCharIndex = i
|
||||
const char = assistantMessage[i]
|
||||
|
||||
// Track code blocks and inline code
|
||||
if (char === "`") {
|
||||
backtickCount++
|
||||
// Check if we have three backticks for code block
|
||||
if (backtickCount === 3) {
|
||||
inCodeBlock = !inCodeBlock
|
||||
backtickCount = 0
|
||||
inInlineCode = false // Code blocks take precedence
|
||||
}
|
||||
} else {
|
||||
// If we had one backtick and now a different char, toggle inline code
|
||||
if (backtickCount === 1 && !inCodeBlock) {
|
||||
inInlineCode = !inInlineCode
|
||||
}
|
||||
backtickCount = 0
|
||||
}
|
||||
|
||||
// Skip tool parsing if we're inside code blocks or inline code
|
||||
if (inCodeBlock || inInlineCode) {
|
||||
// Continue accumulating text content
|
||||
if (!currentTextContent && !currentToolUse) {
|
||||
currentTextContentStart = currentCharIndex
|
||||
currentTextContent = {
|
||||
type: "text",
|
||||
content: "",
|
||||
partial: true,
|
||||
}
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Parsing a tool parameter
|
||||
if (currentToolUse && currentParamName) {
|
||||
|
|
@ -177,52 +214,55 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
|
|||
currentCharIndex >= tag.length - 1 &&
|
||||
assistantMessage.startsWith(tag, currentCharIndex - tag.length + 1)
|
||||
) {
|
||||
// End current text block if one was active.
|
||||
if (currentTextContent) {
|
||||
currentTextContent.content = assistantMessage
|
||||
.slice(
|
||||
currentTextContentStart, // From where text started.
|
||||
currentCharIndex - tag.length + 1, // To before the tool tag starts.
|
||||
)
|
||||
.trim()
|
||||
// Only process tool tags if we're not in code blocks
|
||||
if (!inCodeBlock && !inInlineCode) {
|
||||
// End current text block if one was active.
|
||||
if (currentTextContent) {
|
||||
currentTextContent.content = assistantMessage
|
||||
.slice(
|
||||
currentTextContentStart, // From where text started.
|
||||
currentCharIndex - tag.length + 1, // To before the tool tag starts.
|
||||
)
|
||||
.trim()
|
||||
|
||||
currentTextContent.partial = false // Ended because tool started.
|
||||
currentTextContent.partial = false // Ended because tool started.
|
||||
|
||||
if (currentTextContent.content.length > 0) {
|
||||
contentBlocks.push(currentTextContent)
|
||||
if (currentTextContent.content.length > 0) {
|
||||
contentBlocks.push(currentTextContent)
|
||||
}
|
||||
|
||||
currentTextContent = undefined
|
||||
} else {
|
||||
// Check for any text between the last block and this tag.
|
||||
const potentialText = assistantMessage
|
||||
.slice(
|
||||
currentTextContentStart, // From where text *might* have started.
|
||||
currentCharIndex - tag.length + 1, // To before the tool tag starts.
|
||||
)
|
||||
.trim()
|
||||
|
||||
if (potentialText.length > 0) {
|
||||
contentBlocks.push({
|
||||
type: "text",
|
||||
content: potentialText,
|
||||
partial: false,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
currentTextContent = undefined
|
||||
} else {
|
||||
// Check for any text between the last block and this tag.
|
||||
const potentialText = assistantMessage
|
||||
.slice(
|
||||
currentTextContentStart, // From where text *might* have started.
|
||||
currentCharIndex - tag.length + 1, // To before the tool tag starts.
|
||||
)
|
||||
.trim()
|
||||
|
||||
if (potentialText.length > 0) {
|
||||
contentBlocks.push({
|
||||
type: "text",
|
||||
content: potentialText,
|
||||
partial: false,
|
||||
})
|
||||
// Start the new tool use.
|
||||
currentToolUse = {
|
||||
type: "tool_use",
|
||||
name: toolName,
|
||||
params: {},
|
||||
partial: true, // Assume partial until closing tag is found.
|
||||
}
|
||||
|
||||
currentToolUseStart = currentCharIndex + 1 // Tool content starts after the opening tag.
|
||||
startedNewTool = true
|
||||
|
||||
break
|
||||
}
|
||||
|
||||
// Start the new tool use.
|
||||
currentToolUse = {
|
||||
type: "tool_use",
|
||||
name: toolName,
|
||||
params: {},
|
||||
partial: true, // Assume partial until closing tag is found.
|
||||
}
|
||||
|
||||
currentToolUseStart = currentCharIndex + 1 // Tool content starts after the opening tag.
|
||||
startedNewTool = true
|
||||
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue