Fix DirectiveStreamingParser to prevent processing nested XML tags as directives inside tool parameters

- Modified DirectiveStreamingParser to check if currently inside a tool parameter context - Added logic to treat nested XML tags as plain text when inside tool directive parameters - Made currentContext and currentParamName properties public in ToolDirectiveHandler for access - Updated both onopentag and onclosetag handlers to respect tool parameter boundaries - Fixes issue where <log_message> tags inside <attempt_completion><result> were being processed as separate directives - All existing tests continue to pass, new test case now passes
2026-09-08 22:21:23 +00:00 · 2025-06-16 22:21:38 +07:00 · 2025-06-16 22:21:38 +07:00 · b879dd21b6
commit b879dd21b6
parent 5ef186300e
6 changed files with 581 additions and 7 deletions
--- a/debug-log-issue.js
+++ b/debug-log-issue.js
@ -0,0 +1,58 @@
+// Debug script to test the exact scenario from the image
+const { DirectiveStreamingParser } = require("./src/core/message-parsing/DirectiveStreamingParser.ts")
+const { FallbackParser } = require("./src/core/message-parsing/FallbackParser.ts")
+
+// Test the exact content that might be causing the issue
+const testContent = `I'm happy to provide an example of the XML format for the log_message directive. As specified in the guidelines, log messages are used to output debugging information to the VSCode output channel. Here's an example of how it is formatted:
+
+In this example:
+
+The message tag contains the text of the log message.
+The level tag specifies the log level, which can be "debug", "info", "warn", or "error". It's optional and defaults to "info" if not provided.
+
+This format allows for immediate logging without requiring user approval, and it can be used multiple times in a single message if needed.
+
+<attempt_completion>
+<result>I've provided an example of the XML format for the log_message directive. It includes the message content and an optional level specification, formatted as required for logging to the VSCode output channel.</result>
+</attempt_completion>`
+
+console.log("Testing DirectiveStreamingParser...")
+const result1 = DirectiveStreamingParser.parse(testContent)
+console.log("DirectiveStreamingParser result:")
+result1.forEach((directive, idx) => {
+	console.log(`  ${idx}: ${directive.type}`)
+	if (directive.type === "log_message") {
+		console.log(`    ⚠️  LOG MESSAGE FOUND: "${directive.message}"`)
+	}
+	if (directive.type === "tool_use") {
+		console.log(`    Tool: ${directive.name}`)
+		console.log(`    Params:`, Object.keys(directive.params))
+	}
+})
+
+console.log("\nTesting FallbackParser...")
+const result2 = FallbackParser.parse(testContent)
+console.log("FallbackParser result:")
+result2.forEach((directive, idx) => {
+	console.log(`  ${idx}: ${directive.type}`)
+	if (directive.type === "log_message") {
+		console.log(`    ⚠️  LOG MESSAGE FOUND: "${directive.message}"`)
+	}
+	if (directive.type === "tool_use") {
+		console.log(`    Tool: ${directive.name}`)
+		console.log(`    Params:`, Object.keys(directive.params))
+	}
+})
+
+// Check for any log messages
+const allLogMessages1 = result1.filter((r) => r.type === "log_message")
+const allLogMessages2 = result2.filter((r) => r.type === "log_message")
+
+console.log(`\nDirectiveStreamingParser found ${allLogMessages1.length} log messages`)
+console.log(`FallbackParser found ${allLogMessages2.length} log messages`)
+
+if (allLogMessages1.length > 0 || allLogMessages2.length > 0) {
+	console.log("\n🚨 ISSUE FOUND: Log messages are still being parsed as directives!")
+} else {
+	console.log("\n✅ No log message directives found - this is correct!")
+}
--- a/docs/directive-streaming-parser-code-block-fix.md
+++ b/docs/directive-streaming-parser-code-block-fix.md
@ -0,0 +1,396 @@
+# DirectiveStreamingParser Code Block Fix - Implementation Plan
+
+## Problem Statement
+
+The test `"should not parse directives inside triple backticks as directives"` in [`src/core/message-parsing/__tests__/directive-streaming-parser.spec.ts`](../src/core/message-parsing/__tests__/directive-streaming-parser.spec.ts) is failing because the [`DirectiveStreamingParser`](../src/core/message-parsing/DirectiveStreamingParser.ts) currently parses XML directives inside code blocks (`...`) instead of treating them as plain text.
+
+### Current Behavior
+
+````
+Input: "Some text\n```\n<log_message>content</log_message>\n```\nMore text"
+Output: [TextDirective, LogDirective, TextDirective] ❌
+````
+
+### Expected Behavior
+
+````
+Input: "Some text\n```\n<log_message>content</log_message>\n```\nMore text"
+Output: [TextDirective] ✅ (entire content as single text block)
+````
+
+## Streaming Complexity
+
+The parser must handle **true streaming scenarios** where code block boundaries can be split across message chunks:
+
+### Example Streaming Scenario
+
+````
+Chunk 1: "Some text with\n`"
+Chunk 2: "``\n<log_message>\n<message>"
+Chunk 3: "This is an example</message>\n<level>warn</level>"
+Chunk 4: "\n</log_message>\n```\nMore text"
+````
+
+**Challenge**: The ``` boundary is split across chunks 1-2, requiring stateful parsing.
+
+## Solution: Streaming-Aware State Machine
+
+### Architecture Overview
+
+````mermaid
+flowchart TD
+    A[Streaming Text Input] --> B[Code Block State Machine]
+    B --> C{Current State}
+    C -->|OUTSIDE| D[Check for ``` Start]
+    C -->|INSIDE| E[Check for ``` End]
+    C -->|PARTIAL_START| F[Complete ``` Detection]
+    C -->|PARTIAL_END| G[Complete ``` End Detection]
+
+    D -->|Found ```| H[Enter INSIDE State]
+    D -->|Partial `| I[Enter PARTIAL_START State]
+    D -->|Normal Text| J[Process as Text/XML]
+
+    E -->|Found ```| K[Exit to OUTSIDE State]
+    E -->|Partial `| L[Enter PARTIAL_END State]
+    E -->|Normal Text| M[Accumulate as Plain Text]
+
+    F -->|Complete ```| H
+    F -->|More `| F
+    F -->|Not ```| N[Revert to OUTSIDE + Process]
+
+    G -->|Complete ```| K
+    G -->|More `| G
+    G -->|Not ```| M
+
+    H --> O[Suppress XML Parsing]
+    K --> P[Resume XML Parsing]
+    M --> Q[Add to Code Block Content]
+    J --> R[Allow Directive Processing]
+````
+
+### State Machine Definition
+
+````typescript
+enum CodeBlockState {
+	OUTSIDE = "outside", // Normal parsing mode
+	INSIDE = "inside", // Inside code block - suppress XML
+	PARTIAL_START = "partial_start", // Detected partial ``` at start
+	PARTIAL_END = "partial_end", // Detected partial ``` at end
+}
+````
+
+## Implementation Plan
+
+### Phase 1: Extend ParseContext
+
+**File**: [`src/core/message-parsing/ParseContext.ts`](../src/core/message-parsing/ParseContext.ts)
+
+````typescript
+export interface ParseContext {
+	currentText: string
+	contentBlocks: Directive[]
+	hasXmlTags: boolean
+	hasIncompleteXml: boolean
+
+	// New code block state tracking
+	codeBlockState: CodeBlockState
+	pendingBackticks: string // For partial ``` detection
+	codeBlockContent: string // Accumulated content inside code blocks
+	codeBlockStartIndex: number // Track where code block started
+}
+````
+
+### Phase 2: Create Code Block State Machine
+
+**New File**: `src/core/message-parsing/CodeBlockStateMachine.ts`
+
+```typescript
+export interface ProcessedTextResult {
+	processedText: string
+	suppressXmlParsing: boolean
+	stateChanged: boolean
+}
+
+export class CodeBlockStateMachine {
+	processText(text: string, context: ParseContext): ProcessedTextResult {
+		// Core state machine logic
+		// Handle all edge cases for partial boundaries
+		// Return processed text and parsing instructions
+	}
+
+	private detectCodeBlockBoundary(
+		text: string,
+		startIndex: number,
+	): {
+		found: boolean
+		endIndex: number
+		isComplete: boolean
+	}
+
+	private handlePartialBoundary(text: string, context: ParseContext): void
+
+	private transitionState(newState: CodeBlockState, context: ParseContext): void
+}
+```
+
+### Phase 3: Enhanced TextDirectiveHandler
+
+**File**: [`src/core/message-parsing/handlers/TextDirectiveHandler.ts`](../src/core/message-parsing/handlers/TextDirectiveHandler.ts)
+
+```typescript
+export class TextDirectiveHandler extends BaseDirectiveHandler {
+	private stateMachine = new CodeBlockStateMachine()
+
+	override onText(text: string, context: ParseContext): void {
+		const result = this.stateMachine.processText(text, context)
+
+		if (result.suppressXmlParsing) {
+			// Inside code block - accumulate as plain text
+			context.codeBlockContent += result.processedText
+		} else {
+			// Normal text processing
+			if (this.currentState === "text") {
+				context.currentText += result.processedText
+			}
+		}
+	}
+
+	override onEnd(context: ParseContext): void {
+		// Handle any remaining code block content
+		if (context.codeBlockContent) {
+			context.currentText += context.codeBlockContent
+		}
+
+		if (context.currentText.trim()) {
+			context.contentBlocks.push({
+				type: "text",
+				content: context.currentText.trim(),
+				partial: true,
+			} as TextDirective)
+		}
+	}
+}
+```
+
+### Phase 4: Parser-Level Integration
+
+**File**: [`src/core/message-parsing/DirectiveStreamingParser.ts`](../src/core/message-parsing/DirectiveStreamingParser.ts)
+
+```typescript
+export class DirectiveStreamingParser {
+	static parse(assistantMessage: string): Directive[] {
+		const context: ParseContext = {
+			// ... existing fields
+			codeBlockState: CodeBlockState.OUTSIDE,
+			pendingBackticks: "",
+			codeBlockContent: "",
+			codeBlockStartIndex: -1,
+		}
+
+		// ... existing parser setup
+
+		parser.onopentag = (node: sax.Tag) => {
+			// Only process XML tags if NOT inside code block
+			if (context.codeBlockState !== CodeBlockState.INSIDE) {
+				// Existing XML processing logic
+				context.hasXmlTags = true
+				tagStack.push(node.name)
+				const handler = this.registry.getHandler(node.name)
+				// ... rest of existing logic
+			} else {
+				// Inside code block - treat as plain text
+				const tagText = `<${node.name}${this.attributesToString(node.attributes)}>`
+				this.registry.getTextHandler().onText(tagText, context)
+			}
+		}
+
+		parser.onclosetag = (tagName: string) => {
+			if (context.codeBlockState !== CodeBlockState.INSIDE) {
+				// Existing close tag logic
+			} else {
+				// Inside code block - treat as plain text
+				this.registry.getTextHandler().onText(`</${tagName}>`, context)
+			}
+		}
+
+		// ... rest of existing logic
+	}
+
+	private attributesToString(attributes: { [key: string]: string }): string {
+		return Object.entries(attributes)
+			.map(([key, value]) => ` ${key}="${value}"`)
+			.join("")
+	}
+}
+```
+
+## Edge Cases to Handle
+
+### 1. Partial Boundaries Across Chunks
+
+````typescript
+// Chunk 1: "text `"
+// Chunk 2: "``\ncontent"
+// Expected: Detect complete ``` boundary
+````
+
+### 2. Multiple Code Blocks
+
+````typescript
+// "text ```code1``` more ```code2``` end"
+// Expected: Two separate code blocks, both suppressed
+````
+
+### 3. Nested Backticks
+
+````typescript
+// "```\nSome `code` here\n```"
+// Expected: Inner backticks treated as literal text
+````
+
+### 4. Malformed Boundaries
+
+```typescript
+// "text `` incomplete"
+// Expected: Treat as normal text, not code block
+```
+
+### 5. Mixed Content
+
+````typescript
+// "text ```code``` <directive>content</directive>"
+// Expected: Code block as text, directive processed normally
+````
+
+## Test Strategy
+
+### New Test Cases Required
+
+**File**: [`src/core/message-parsing/__tests__/directive-streaming-parser.spec.ts`](../src/core/message-parsing/__tests__/directive-streaming-parser.spec.ts)
+
+````typescript
+describe("Code Block Handling", () => {
+	test("should handle partial code block boundaries across chunks", () => {
+		// Test streaming scenario with split boundaries
+	})
+
+	test("should handle multiple code blocks in single message", () => {
+		// Test multiple ```...``` blocks
+	})
+
+	test("should handle mixed content with code blocks and directives", () => {
+		// Test your example scenario
+	})
+
+	test("should handle nested backticks inside code blocks", () => {
+		// Test backticks within code blocks
+	})
+
+	test("should handle malformed code block boundaries", () => {
+		// Test incomplete or invalid ``` patterns
+	})
+
+	test("should maintain performance with large messages", () => {
+		// Performance regression test
+	})
+})
+````
+
+### Existing Test Verification
+
+- ✅ All existing tests must continue to pass
+- ✅ Specifically: `"should not parse directives inside triple backticks as directives"`
+- ✅ No regression in normal directive parsing
+
+## Performance Considerations
+
+### Optimization Strategies
+
+1. **Lazy Activation**: Only activate state machine when backticks detected
+2. **Efficient String Processing**: Minimize string concatenation overhead
+3. **State Caching**: Cache frequently accessed state information
+4. **Early Exit**: Skip processing when clearly outside code blocks
+
+### Performance Benchmarks
+
+- Measure parsing time for messages with/without code blocks
+- Test memory usage with large messages containing multiple code blocks
+- Verify no significant regression in normal parsing scenarios
+
+## Implementation Checklist
+
+### Phase 1: Core Infrastructure
+
+- [ ] Extend [`ParseContext`](../src/core/message-parsing/ParseContext.ts) with code block state
+- [ ] Create `CodeBlockStateMachine` class
+- [ ] Implement state transition logic
+- [ ] Add comprehensive unit tests for state machine
+
+### Phase 2: Parser Integration
+
+- [ ] Modify [`DirectiveStreamingParser`](../src/core/message-parsing/DirectiveStreamingParser.ts) to check code block state
+- [ ] Update XML tag processing to respect code block state
+- [ ] Handle attribute serialization for suppressed tags
+- [ ] Test parser-level integration
+
+### Phase 3: Handler Updates
+
+- [ ] Enhance [`TextDirectiveHandler`](../src/core/message-parsing/handlers/TextDirectiveHandler.ts) with state machine
+- [ ] Update text processing logic
+- [ ] Handle code block content accumulation
+- [ ] Test handler-level functionality
+
+### Phase 4: Comprehensive Testing
+
+- [ ] Add all edge case tests
+- [ ] Verify existing test compatibility
+- [ ] Performance benchmarking
+- [ ] Integration testing with real streaming scenarios
+
+### Phase 5: Documentation & Cleanup
+
+- [ ] Update code documentation
+- [ ] Add inline comments for complex logic
+- [ ] Performance optimization if needed
+- [ ] Final integration testing
+
+## Risk Mitigation
+
+### Potential Issues
+
+1. **Complex State Management**: Multiple edge cases to handle
+2. **Performance Impact**: Additional processing overhead
+3. **Backward Compatibility**: Existing functionality must remain intact
+4. **Memory Usage**: State persistence across chunks
+
+### Mitigation Strategies
+
+1. **Comprehensive Testing**: Cover all identified edge cases
+2. **Performance Benchmarking**: Measure and optimize impact
+3. **Gradual Rollout**: Feature flag for new behavior if needed
+4. **Fallback Mechanism**: Graceful degradation on state machine errors
+
+## Success Criteria
+
+- ✅ Failing test `"should not parse directives inside triple backticks as directives"` passes
+- ✅ All existing tests continue to pass
+- ✅ Handles streaming scenarios with partial code block boundaries
+- ✅ Performance impact < 10% for normal parsing scenarios
+- ✅ Memory usage remains stable for large messages
+- ✅ Comprehensive test coverage for all edge cases
+
+## Files to Modify/Create
+
+### Modified Files
+
+1. [`src/core/message-parsing/ParseContext.ts`](../src/core/message-parsing/ParseContext.ts)
+2. [`src/core/message-parsing/DirectiveStreamingParser.ts`](../src/core/message-parsing/DirectiveStreamingParser.ts)
+3. [`src/core/message-parsing/handlers/TextDirectiveHandler.ts`](../src/core/message-parsing/handlers/TextDirectiveHandler.ts)
+4. [`src/core/message-parsing/__tests__/directive-streaming-parser.spec.ts`](../src/core/message-parsing/__tests__/directive-streaming-parser.spec.ts)
+
+### New Files
+
+1. `src/core/message-parsing/CodeBlockStateMachine.ts`
+2. `src/core/message-parsing/__tests__/code-block-state-machine.spec.ts`
+
+This comprehensive plan addresses the streaming nature of the parser while ensuring robust handling of all edge cases related to code block detection and XML directive suppression.
--- a/junk.txt
+++ b/junk.txt
@ -0,0 +1,19 @@
+Some text with
+```
+<log_message>
+<message>This is an example message that should display in text because it is in a code block</message>
+<level>warn</level>
+</log_message>
+```
+More text
+<log_message>
+<message>This is a warning message that should be handled by the directive handler</message>
+<level>warn</level>
+</log_message>
+"
+
+
+Partials could have splits like
+1. Some text with\n`
+2. ``\n<log_
+3. message>\n<message>
--- a/src/core/message-parsing/DirectiveStreamingParser.ts
+++ b/src/core/message-parsing/DirectiveStreamingParser.ts
@ -33,8 +33,14 @@ export class DirectiveStreamingParser {
 					"isInsideParameterCodeBlock" in activeHandler &&
 					(activeHandler as any).isInsideParameterCodeBlock())

-			// Only process XML tags if NOT inside code block
-			if (!insideCodeBlock) {
+			// Check if we're inside a tool parameter (but not at the parameter level itself)
+			const insideToolParameter =
+				activeHandler &&
+				activeHandler.constructor.name === "ToolDirectiveHandler" &&
+				(activeHandler as any).currentContext === "param"
+
+			// Only process XML tags if NOT inside code block AND NOT inside tool parameter
+			if (!insideCodeBlock && !insideToolParameter) {
 				context.hasXmlTags = true
 				tagStack.push(node.name)
 				const handler = this.registry.getHandler(node.name)
@ -47,7 +53,7 @@ export class DirectiveStreamingParser {
 					activeHandler.onOpenTag(node, context)
 				}
 			} else {
-				// Inside code block - treat as plain text
+				// Inside code block or tool parameter - treat as plain text
 				const tagText = `<${node.name}${this.attributesToString(node.attributes)}>`
 				if (activeHandler) {
 					activeHandler.onText(tagText, context)
@ -65,7 +71,14 @@ export class DirectiveStreamingParser {
 					"isInsideParameterCodeBlock" in activeHandler &&
 					(activeHandler as any).isInsideParameterCodeBlock())

-			if (!insideCodeBlock) {
+			// Check if we're inside a tool parameter (but not at the parameter level itself)
+			const insideToolParameter =
+				activeHandler &&
+				activeHandler.constructor.name === "ToolDirectiveHandler" &&
+				(activeHandler as any).currentContext === "param" &&
+				tagName !== (activeHandler as any).currentParamName
+
+			if (!insideCodeBlock && !insideToolParameter) {
 				// Normal XML processing
 				if (activeHandler) {
 					activeHandler.onCloseTag(tagName, context)
@ -76,7 +89,7 @@ export class DirectiveStreamingParser {
 				}
 				tagStack.pop()
 			} else {
-				// Inside code block - treat as plain text
+				// Inside code block or tool parameter - treat as plain text
 				if (activeHandler) {
 					activeHandler.onText(`</${tagName}>`, context)
 				} else {
--- a/src/core/message-parsing/tests/directive-streaming-parser.spec.ts
+++ b/src/core/message-parsing/tests/directive-streaming-parser.spec.ts
@ -209,4 +209,92 @@ suite("DirectiveStreamingParser", () => {
 			expect((attemptCompletion as any).params.result).toContain("<log_message>")
 		}
 	})
+
+	test("should handle real-world scenario with log message example", () => {
+		// Test a scenario similar to what's shown in the user's image
+		const input = `I'm happy to provide an example of the XML format for the log_message directive.
+
+<log_message>
+<message>This is an example log message for demonstration purposes</message>
+<level>info</level>
+</log_message>
+
+<attempt_completion>
+<result>I've provided an example of the XML format for the log_message directive.</result>
+</attempt_completion>`
+
+		const result = DirectiveStreamingParser.parse(input)
+
+		// Should have text, log_message, and attempt_completion
+		expect(result).toHaveLength(3)
+		expect(result[0].type).toBe("text")
+		expect(result[1].type).toBe("log_message")
+		expect(result[2].type).toBe("tool_use")
+
+		// The log message should be processed as a real directive (this is correct behavior)
+		expect((result[1] as any).message).toBe("This is an example log message for demonstration purposes")
+		expect((result[1] as any).level).toBe("info")
+	})
+
+	test("should NOT process log messages inside code blocks in attempt_completion", () => {
+		// Test the problematic scenario
+		const input = `<attempt_completion>
+<result>Here's an example:
+
+\`\`\`xml
+<log_message>
+<message>This should NOT be processed as a log directive</message>
+<level>debug</level>
+</log_message>
+\`\`\`
+
+That's the format.</result>
+</attempt_completion>`
+
+		const result = DirectiveStreamingParser.parse(input)
+
+		// Should only have the attempt_completion directive
+		expect(result).toHaveLength(1)
+		expect(result[0].type).toBe("tool_use")
+		expect((result[0] as any).name).toBe("attempt_completion")
+
+		// The result should contain the log_message as plain text
+		expect((result[0] as any).params.result).toContain("<log_message>")
+		expect((result[0] as any).params.result).toContain("This should NOT be processed as a log directive")
+
+		// Most importantly: NO separate log_message directive should exist
+		const logMessages = result.filter((r) => r.type === "log_message")
+		expect(logMessages).toHaveLength(0)
+	})
+
+	test("should handle log message directly inside attempt_completion result", () => {
+		// Test the actual scenario from the user's image - log message directly inside attempt_completion result
+		const input = `<attempt_completion>
+<result>I'm happy to provide an example of the XML format for the log_message directive.
+
+<log_message>
+<message>This is an example log message for demonstration purposes</message>
+<level>info</level>
+</log_message>
+
+I've provided an example of the XML format for the log_message directive.</result>
+</attempt_completion>`
+
+		const result = DirectiveStreamingParser.parse(input)
+
+		console.log("Result:", JSON.stringify(result, null, 2))
+
+		// Should only have the attempt_completion directive
+		expect(result).toHaveLength(1)
+		expect(result[0].type).toBe("tool_use")
+		expect((result[0] as any).name).toBe("attempt_completion")
+
+		// The result should contain the log_message as plain text (NOT as a separate directive)
+		expect((result[0] as any).params.result).toContain("<log_message>")
+		expect((result[0] as any).params.result).toContain("This is an example log message for demonstration purposes")
+
+		// Most importantly: NO separate log_message directive should exist
+		const logMessages = result.filter((r) => r.type === "log_message")
+		expect(logMessages).toHaveLength(0)
+	})
 })
--- a/src/core/message-parsing/handlers/ToolDirectiveHandler.ts
+++ b/src/core/message-parsing/handlers/ToolDirectiveHandler.ts
@ -7,9 +7,9 @@ import { CodeBlockStateMachine } from "../CodeBlockStateMachine"
 export class ToolDirectiveHandler extends BaseDirectiveHandler {
 	readonly tagName: string
 	private currentToolDirective?: ToolDirective
-	private currentParamName?: ToolParamName
+	public currentParamName?: ToolParamName
 	private currentParamValue = ""
-	private currentContext: "param" | "none" = "none"
+	public currentContext: "param" | "none" = "none"
 	private stateMachine = new CodeBlockStateMachine()
 	private paramCodeBlockState: CodeBlockState = CodeBlockState.OUTSIDE