Skip to content

Commit eb0add2

Browse files
committed
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
1 parent 0e1b23d commit eb0add2

File tree

4 files changed

+369
-95
lines changed

4 files changed

+369
-95
lines changed

src/core/assistant-message/AssistantMessageParser.ts

Lines changed: 77 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ export class AssistantMessageParser {
1717
private readonly MAX_ACCUMULATOR_SIZE = 1024 * 1024 // 1MB limit
1818
private readonly MAX_PARAM_LENGTH = 1024 * 100 // 100KB per parameter limit
1919
private accumulator = ""
20+
private inCodeBlock = false
21+
private inInlineCode = false
22+
private codeBlockDelimiterCount = 0
2023

2124
/**
2225
* Initialize a new AssistantMessageParser instance.
@@ -37,6 +40,9 @@ export class AssistantMessageParser {
3740
this.currentParamName = undefined
3841
this.currentParamValueStartIndex = 0
3942
this.accumulator = ""
43+
this.inCodeBlock = false
44+
this.inInlineCode = false
45+
this.codeBlockDelimiterCount = 0
4046
}
4147

4248
/**
@@ -63,6 +69,41 @@ export class AssistantMessageParser {
6369
this.accumulator += char
6470
const currentPosition = accumulatorStartLength + i
6571

72+
// Track code blocks and inline code
73+
if (char === "`") {
74+
this.codeBlockDelimiterCount++
75+
if (this.codeBlockDelimiterCount === 3) {
76+
this.inCodeBlock = !this.inCodeBlock
77+
this.codeBlockDelimiterCount = 0
78+
this.inInlineCode = false // Code blocks take precedence
79+
}
80+
} else {
81+
// If we had one backtick and now a different char, toggle inline code
82+
if (this.codeBlockDelimiterCount === 1 && !this.inCodeBlock) {
83+
this.inInlineCode = !this.inInlineCode
84+
}
85+
this.codeBlockDelimiterCount = 0
86+
}
87+
88+
// Skip tool parsing if we're inside code blocks or inline code
89+
if (this.inCodeBlock || this.inInlineCode) {
90+
// Continue accumulating text content
91+
if (this.currentTextContent === undefined && !this.currentToolUse) {
92+
this.currentTextContentStartIndex = currentPosition
93+
this.currentTextContent = {
94+
type: "text",
95+
content: this.accumulator.slice(this.currentTextContentStartIndex).trim(),
96+
partial: true,
97+
}
98+
// Add the new text content to contentBlocks immediately
99+
this.contentBlocks.push(this.currentTextContent)
100+
} else if (this.currentTextContent) {
101+
// Update the existing text content
102+
this.currentTextContent.content = this.accumulator.slice(this.currentTextContentStartIndex).trim()
103+
}
104+
continue
105+
}
106+
66107
// There should not be a param without a tool use.
67108
if (this.currentToolUse && this.currentParamName) {
68109
const currentParamValue = this.accumulator.slice(this.currentParamValueStartIndex)
@@ -159,47 +200,50 @@ export class AssistantMessageParser {
159200

160201
for (const toolUseOpeningTag of possibleToolUseOpeningTags) {
161202
if (this.accumulator.endsWith(toolUseOpeningTag)) {
162-
// Extract and validate the tool name
163-
const extractedToolName = toolUseOpeningTag.slice(1, -1)
203+
// Only process tool tags if we're not in code blocks
204+
if (!this.inCodeBlock && !this.inInlineCode) {
205+
// Extract and validate the tool name
206+
const extractedToolName = toolUseOpeningTag.slice(1, -1)
164207

165-
// Check if the extracted tool name is valid
166-
if (!toolNames.includes(extractedToolName as ToolName)) {
167-
// Invalid tool name, treat as plain text and continue
168-
continue
169-
}
208+
// Check if the extracted tool name is valid
209+
if (!toolNames.includes(extractedToolName as ToolName)) {
210+
// Invalid tool name, treat as plain text and continue
211+
continue
212+
}
170213

171-
// Start of a new tool use.
172-
this.currentToolUse = {
173-
type: "tool_use",
174-
name: extractedToolName as ToolName,
175-
params: {},
176-
partial: true,
177-
}
214+
// Start of a new tool use.
215+
this.currentToolUse = {
216+
type: "tool_use",
217+
name: extractedToolName as ToolName,
218+
params: {},
219+
partial: true,
220+
}
178221

179-
this.currentToolUseStartIndex = this.accumulator.length
222+
this.currentToolUseStartIndex = this.accumulator.length
180223

181-
// This also indicates the end of the current text content.
182-
if (this.currentTextContent) {
183-
this.currentTextContent.partial = false
224+
// This also indicates the end of the current text content.
225+
if (this.currentTextContent) {
226+
this.currentTextContent.partial = false
184227

185-
// Remove the partially accumulated tool use tag from the
186-
// end of text (<tool).
187-
this.currentTextContent.content = this.currentTextContent.content
188-
.slice(0, -toolUseOpeningTag.slice(0, -1).length)
189-
.trim()
228+
// Remove the partially accumulated tool use tag from the
229+
// end of text (<tool).
230+
this.currentTextContent.content = this.currentTextContent.content
231+
.slice(0, -toolUseOpeningTag.slice(0, -1).length)
232+
.trim()
190233

191-
// No need to push, currentTextContent is already in contentBlocks
192-
this.currentTextContent = undefined
193-
}
234+
// No need to push, currentTextContent is already in contentBlocks
235+
this.currentTextContent = undefined
236+
}
194237

195-
// Immediately push new tool_use block as partial
196-
let idx = this.contentBlocks.findIndex((block) => block === this.currentToolUse)
197-
if (idx === -1) {
198-
this.contentBlocks.push(this.currentToolUse)
199-
}
238+
// Immediately push new tool_use block as partial
239+
let idx = this.contentBlocks.findIndex((block) => block === this.currentToolUse)
240+
if (idx === -1) {
241+
this.contentBlocks.push(this.currentToolUse)
242+
}
200243

201-
didStartToolUse = true
202-
break
244+
didStartToolUse = true
245+
break
246+
}
203247
}
204248
}
205249

src/core/assistant-message/__tests__/parseAssistantMessage.spec.ts

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,5 +336,154 @@ const isEmptyTextContent = (block: AssistantMessageContent) =>
336336
expect((result[5] as ToolUse).name).toBe("execute_command")
337337
})
338338
})
339+
340+
describe("code block handling", () => {
341+
it("should not parse tool tags within code blocks", () => {
342+
const message = `Here's an example of the ask_followup_question tool:
343+
344+
\`\`\`xml
345+
<ask_followup_question>
346+
<question>What is the path to the frontend-config.json file?</question>
347+
<follow_up>
348+
<suggest>./src/frontend-config.json</suggest>
349+
<suggest>./config/frontend-config.json</suggest>
350+
<suggest>./frontend-config.json</suggest>
351+
</follow_up>
352+
</ask_followup_question>
353+
\`\`\`
354+
355+
This is how you use it.`
356+
357+
const result = parser(message)
358+
359+
// Should only have text content, no tool use
360+
expect(result).toHaveLength(1)
361+
expect(result[0].type).toBe("text")
362+
const textContent = result[0] as TextContent
363+
expect(textContent.content).toContain("Here's an example")
364+
expect(textContent.content).toContain("<ask_followup_question>")
365+
expect(textContent.content).toContain("This is how you use it")
366+
expect(textContent.partial).toBe(true)
367+
})
368+
369+
it("should not parse tool tags within inline code", () => {
370+
const message = "Use the \`<read_file><path>file.ts</path></read_file>\` tool to read files."
371+
const result = parser(message)
372+
373+
// Should only have text content, no tool use
374+
expect(result).toHaveLength(1)
375+
expect(result[0].type).toBe("text")
376+
const textContent = result[0] as TextContent
377+
expect(textContent.content).toBe(message)
378+
expect(textContent.partial).toBe(true)
379+
})
380+
381+
it("should parse tool tags outside of code blocks", () => {
382+
const message = `Here's an example:
383+
384+
\`\`\`
385+
<example>code</example>
386+
\`\`\`
387+
388+
Now let me read a file:
389+
390+
<read_file><path>test.ts</path></read_file>`
391+
392+
const result = parser(message)
393+
394+
// Should have text content and a tool use
395+
expect(result).toHaveLength(2)
396+
397+
// First should be text containing the code block
398+
expect(result[0].type).toBe("text")
399+
const textContent = result[0] as TextContent
400+
expect(textContent.content).toContain("Here's an example")
401+
expect(textContent.content).toContain("<example>code</example>")
402+
expect(textContent.content).toContain("Now let me read a file:")
403+
expect(textContent.partial).toBe(false)
404+
405+
// Second should be the actual tool use
406+
expect(result[1].type).toBe("tool_use")
407+
const toolUse = result[1] as ToolUse
408+
expect(toolUse.name).toBe("read_file")
409+
expect(toolUse.params.path).toBe("test.ts")
410+
expect(toolUse.partial).toBe(false)
411+
})
412+
413+
it("should handle mixed inline code and actual tool uses", () => {
414+
const message =
415+
"The tool \`<read_file>\` is used like this: <read_file><path>actual.ts</path></read_file>"
416+
const result = parser(message)
417+
418+
// Should have text and tool use
419+
expect(result).toHaveLength(2)
420+
421+
expect(result[0].type).toBe("text")
422+
const textContent = result[0] as TextContent
423+
expect(textContent.content).toContain("The tool \`<read_file>\` is used like this:")
424+
425+
expect(result[1].type).toBe("tool_use")
426+
const toolUse = result[1] as ToolUse
427+
expect(toolUse.name).toBe("read_file")
428+
expect(toolUse.params.path).toBe("actual.ts")
429+
})
430+
431+
it("should handle code blocks with triple backticks inside", () => {
432+
const message = `Here's a markdown example:
433+
434+
\`\`\`markdown
435+
# Example
436+
\`\`\`python
437+
print("hello")
438+
\`\`\`
439+
<read_file><path>not_a_tool.ts</path></read_file>
440+
\`\`\`
441+
442+
That was the example.`
443+
444+
const result = parser(message)
445+
446+
// Should only have text content
447+
expect(result).toHaveLength(1)
448+
expect(result[0].type).toBe("text")
449+
const textContent = result[0] as TextContent
450+
expect(textContent.content).toContain("Here's a markdown example")
451+
expect(textContent.content).toContain("<read_file><path>not_a_tool.ts</path></read_file>")
452+
expect(textContent.content).toContain("That was the example")
453+
})
454+
455+
it("should correctly handle the exact issue scenario", () => {
456+
const message = `## ask_followup_question
457+
Description: Ask the user a question to gather additional information needed to complete the task.
458+
459+
Usage:
460+
\`\`\`
461+
<ask_followup_question>
462+
<question>Your question here</question>
463+
<follow_up>
464+
<suggest>First suggestion</suggest>
465+
<suggest mode="code">Action with mode switch</suggest>
466+
</follow_up>
467+
</ask_followup_question>
468+
\`\`\`
469+
470+
This tool helps gather information.`
471+
472+
const result = parser(message)
473+
474+
// Should only have text content, no tool invocation
475+
expect(result).toHaveLength(1)
476+
expect(result[0].type).toBe("text")
477+
const textContent = result[0] as TextContent
478+
expect(textContent.content).toContain("ask_followup_question")
479+
expect(textContent.content).toContain("Description:")
480+
expect(textContent.content).toContain("<ask_followup_question>")
481+
expect(textContent.content).toContain("This tool helps gather information")
482+
483+
// Ensure no tool_use blocks were created
484+
const toolUses = result.filter((block) => block.type === "tool_use")
485+
expect(toolUses).toHaveLength(0)
486+
})
487+
})
339488
})
340489
})

0 commit comments

Comments
 (0)