Accidental execution of tool syntax fix (#3456)

Co-authored-by: cte <[email protected]>

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

@@ -335,6 +335,158 @@ const isEmptyTextContent = (block: AssistantMessageContent) =>
 				expect(result[5].type).toBe("tool_use")
 				expect((result[5] as ToolUse).name).toBe("execute_command")
 			})
+
+			it("treats inline <read_file> mention as plain text", () => {
+				const message = "Use the `<read_file>` tool when you need to read a file."
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+
+			it("treats fenced code block mention as plain text", () => {
+				const message = [
+					"Here is an example:",
+					"```",
+					"<read_file>",
+					"<path>demo.txt</path>",
+					"</read_file>",
+					"```",
+				].join("\n")
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+
+			it("parses real tool invocation with newline correctly", () => {
+				const invocation = ["<read_file>", "<path>demo.txt</path>", "</read_file>"].join("\n")
+				const result = parser(invocation)
+				expect(result).toHaveLength(1)
+				const tool = result[0] as ToolUse
+				expect(tool.type).toBe("tool_use")
+				expect(tool.name).toBe("read_file")
+				expect(tool.params.path).toBe("demo.txt")
+			})
+		})
+
+		describe("code block edge cases", () => {
+			it("handles nested code blocks with tool tags", () => {
+				const message = [
+					"Here's a nested code block:",
+					"> ```",
+					"> <read_file><path>test.txt</path></read_file>",
+					"> ```",
+				].join("\n")
+
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+
+			it("handles escaped backticks", () => {
+				const message = "This has an escaped \\`<read_file>\\` tag"
+
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+
+			it("handles incomplete code blocks", () => {
+				const message = [
+					"```javascript",
+					"<read_file><path>test.txt</path></read_file>",
+					// Missing closing backticks
+				].join("\n")
+
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+		})
+
+		describe("tool tag edge cases", () => {
+			it("handles HTML comments containing tool tags", () => {
+				const message = "<!-- <read_file><path>test.txt</path></read_file> -->"
+
+				// Note: Currently both parsers treat tool tags in HTML comments as actual tool invocations
+				// This test documents the current behavior rather than the ideal behavior
+				const result = parser(message)
+				expect(result).toHaveLength(3)
+				expect(result[0].type).toBe("text")
+				expect((result[0] as TextContent).content).toBe("<!--")
+				expect(result[1].type).toBe("tool_use")
+				expect((result[1] as ToolUse).name).toBe("read_file")
+				expect(result[2].type).toBe("text")
+				expect((result[2] as TextContent).content).toBe("-->")
+			})
+
+			it("handles tool tags with HTML-like attributes", () => {
+				const message = '<read_file class="example"><path>test.txt</path></read_file>'
+
+				// This should be treated as plain text since it's not a valid tool format
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+
+			it("handles malformed tool tags with extra spaces", () => {
+				const message = "< read_file >< path >test.txt< /path >< /read_file >"
+
+				// This should be treated as plain text since it's not a valid tool format
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+		})
+
+		describe("mixed formatting edge cases", () => {
+			it("handles tool tags with markdown formatting", () => {
+				const message = "**<read_file>**<path>test.txt</path></read_file>"
+
+				// The opening tag is malformed due to the bold formatting
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+			})
+
+			it("handles tool tags in markdown links", () => {
+				const message = "[Example](<read_file><path>test.txt</path></read_file>)"
+
+				// Note: Currently both parsers treat tool tags in markdown links as actual tool invocations
+				// This test documents the current behavior rather than the ideal behavior
+				const result = parser(message)
+				expect(result).toHaveLength(3)
+				expect(result[0].type).toBe("text")
+				expect((result[0] as TextContent).content).toBe("[Example](")
+				expect(result[1].type).toBe("tool_use")
+				expect((result[1] as ToolUse).name).toBe("read_file")
+				expect(result[2].type).toBe("text")
+				expect((result[2] as TextContent).content).toBe(")")
+			})
+		})
+
+		describe("performance edge cases", () => {
+			it("handles very long text with no tool uses", () => {
+				const message = "A".repeat(10000)
+
+				const result = parser(message)
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("text")
+				expect((result[0] as TextContent).content.length).toBe(10000)
+			})
+
+			it("handles deeply nested tool parameters", () => {
+				// Create a message with a tool that has many nested parameters
+				let message = "<execute_command><command>"
+				for (let i = 0; i < 10; i++) {
+					message += `echo "Level ${i}" && `
+				}
+				message += 'echo "Done"</command></execute_command>'
+
+				const result = parser(message).filter((block) => !isEmptyTextContent(block))
+				expect(result).toHaveLength(1)
+				expect(result[0].type).toBe("tool_use")
+				expect((result[0] as ToolUse).name).toBe("execute_command")
+			})
 		})
 	})
 })

src/core/assistant-message/__tests__/parseAssistantMessageBenchmark.ts

@@ -81,16 +81,13 @@ const runBenchmark = () => {
 	const namePadding = maxNameLength + 2
 
 	console.log(
-		`| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1/V2 Ratio | V1 Heap (bytes) | V2 Heap (bytes) |`,
-	)
-	console.log(
-		`| ${"-".repeat(namePadding)} | ------------ | ------------ | ----------- | ---------------- | ---------------- |`,
+		`| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1 Heap (bytes)  | V2 Heap (bytes) |`,
 	)
+	console.log(`| ${"-".repeat(namePadding)} | ------------ | ------------ | ---------------- | ---------------- |`)
 
 	for (const testCase of testCases) {
 		const v1Time = measureExecutionTime(parseAssistantMessageV1, testCase.input)
 		const v2Time = measureExecutionTime(parseAssistantMessageV2, testCase.input)
-		const timeRatio = v1Time / v2Time
 
 		const v1Memory = measureMemoryUsage(parseAssistantMessageV1, testCase.input)
 		const v2Memory = measureMemoryUsage(parseAssistantMessageV2, testCase.input)
@@ -99,7 +96,6 @@ const runBenchmark = () => {
 			`| ${testCase.name.padEnd(namePadding)} | ` +
 				`${v1Time.toFixed(4).padStart(12)} | ` +
 				`${v2Time.toFixed(4).padStart(12)} | ` +
-				`${timeRatio.toFixed(2).padStart(11)} | ` +
 				`${formatNumber(Math.round(v1Memory.heapUsed)).padStart(16)} | ` +
 				`${formatNumber(Math.round(v2Memory.heapUsed)).padStart(16)} |`,
 		)

src/core/assistant-message/index.ts

@@ -1,2 +1,3 @@
 export { type AssistantMessageContent, parseAssistantMessage } from "./parseAssistantMessage"
 export { presentAssistantMessage } from "./presentAssistantMessage"
+export { parseAssistantMessageV2 } from "./parseAssistantMessageV2"

src/core/assistant-message/parseAssistantMessage.ts

@@ -13,10 +13,52 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 	let currentParamValueStartIndex = 0
 	let accumulator = ""
 
+	// Track whether we are inside markdown code blocks or inline code to avoid treating textual mentions
+	// of tool tags (e.g. <read_file>) as actual tool invocations.
+	let insideCodeBlock = false // ``` fenced code block
+	let insideInlineCode = false // `inline code`
+
+	// Helper to decide if we should parse for tool-related tags at the current position
+	const shouldParseToolTags = () => !insideCodeBlock && !insideInlineCode
+
 	for (let i = 0; i < assistantMessage.length; i++) {
 		const char = assistantMessage[i]
+
+		// Detect fenced code block (```).
+		if (!insideInlineCode && assistantMessage.slice(i, i + 3) === "```") {
+			insideCodeBlock = !insideCodeBlock
+			// Append the full trio of backticks to accumulator.
+			accumulator += "```"
+			i += 2 // Skip the two extra backticks we just added.
+
+			// When toggling code block state, continue to next iteration as
+			// these chars are text.
+			continue
+		}
+
+		// Detect inline code (`) when not inside a fenced code block and not part of triple backticks
+		if (!insideCodeBlock && char === "`") {
+			insideInlineCode = !insideInlineCode
+		}
+
 		accumulator += char
 
+		// If we are in any kind of code context, treat everything as plain text.
+		if (!shouldParseToolTags()) {
+			// Handle accumulating text content block.
+			if (currentTextContent === undefined) {
+				currentTextContentStartIndex = i
+			}
+
+			currentTextContent = {
+				type: "text",
+				content: accumulator.slice(currentTextContentStartIndex).trim(),
+				partial: true,
+			}
+
+			continue
+		}
+
 		// There should not be a param without a tool use.
 		if (currentToolUse && currentParamName) {
 			const currentParamValue = accumulator.slice(currentParamValueStartIndex)
@@ -45,6 +87,7 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 				continue
 			} else {
 				const possibleParamOpeningTags = toolParamNames.map((name) => `<${name}>`)
+
 				for (const paramOpeningTag of possibleParamOpeningTags) {
 					if (accumulator.endsWith(paramOpeningTag)) {
 						// Start of a new parameter.
@@ -89,6 +132,25 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 
 		for (const toolUseOpeningTag of possibleToolUseOpeningTags) {
 			if (accumulator.endsWith(toolUseOpeningTag)) {
+				// Check that this is likely an actual tool invocation and not
+				// an inline textual reference.
+				// We consider it an invocation only if the next non-whitespace
+				// character is a newline (\n or \r) or an opening angle bracket
+				// '<' (which would start the first parameter tag).
+
+				let j = i + 1 // Position after the closing '>' of the opening tag.
+
+				while (j < assistantMessage.length && assistantMessage[j] === " ") {
+					j++
+				}
+
+				const nextChar = assistantMessage[j] ?? ""
+
+				if (nextChar && nextChar !== "<" && nextChar !== "\n" && nextChar !== "\r") {
+					// Treat as plain text, not a tool invocation.
+					continue
+				}
+
 				// Start of a new tool use.
 				currentToolUse = {
 					type: "tool_use",
@@ -151,5 +213,6 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 		contentBlocks.push(currentTextContent)
 	}
 
-	return contentBlocks
+	// Remove any empty text blocks that may have been created by whitespace or newlines before/after tool calls
+	return contentBlocks.filter((block) => !(block.type === "text" && block.content.trim().length === 0))
 }

src/core/assistant-message/parseAssistantMessageV2.ts

@@ -46,6 +46,14 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 	let currentParamValueStart = 0 // Index *after* the opening tag of the current param.
 	let currentParamName: ToolParamName | undefined = undefined
 
+	// Track whether we are inside markdown code blocks or inline code to avoid treating textual mentions
+	// of tool tags (e.g. <read_file>) as actual tool invocations.
+	let insideCodeBlock = false // ``` fenced code block
+	let insideInlineCode = false // `inline code`
+
+	// Helper to decide if we should parse for tool-related tags at the current position
+	const shouldParseToolTags = () => !insideCodeBlock && !insideInlineCode
+
 	// Precompute tags for faster lookups.
 	const toolUseOpenTags = new Map<string, ToolName>()
 	const toolParamOpenTags = new Map<string, ToolParamName>()
@@ -63,16 +71,39 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 	for (let i = 0; i < len; i++) {
 		const currentCharIndex = i
 
+		// Detect fenced code block (```).
+		if (!insideInlineCode && i + 2 < len && assistantMessage.slice(i, i + 3) === "```") {
+			insideCodeBlock = !insideCodeBlock
+			i += 2 // Skip the two extra backticks.
+			continue
+		}
+
+		// Detect inline code (`) when not inside a fenced code block and not
+		// part of triple backticks.
+		if (!insideCodeBlock && assistantMessage[i] === "`") {
+			insideInlineCode = !insideInlineCode
+		}
+
+		// If we are in any kind of code context, treat everything as plain text.
+		if (!shouldParseToolTags()) {
+			// If we're not already tracking text content, start now.
+			if (!currentTextContent) {
+				currentTextContentStart = currentCharIndex
+				currentTextContent = { type: "text", content: "", partial: true }
+			}
+
+			continue
+		}
+
 		// Parsing a tool parameter
 		if (currentToolUse && currentParamName) {
 			const closeTag = `</${currentParamName}>`
-			// Check if the string *ending* at index `i` matches the closing tag
+
+			// Check if the string *ending* at index `i` matches the closing tag.
 			if (
 				currentCharIndex >= closeTag.length - 1 &&
-				assistantMessage.startsWith(
-					closeTag,
-					currentCharIndex - closeTag.length + 1, // Start checking from potential start of tag.
-				)
+				// Start checking from potential start of tag.
+				assistantMessage.startsWith(closeTag, currentCharIndex - closeTag.length + 1)
 			) {
 				// Found the closing tag for the parameter.
 				const value = assistantMessage
@@ -175,6 +206,23 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 					currentCharIndex >= tag.length - 1 &&
 					assistantMessage.startsWith(tag, currentCharIndex - tag.length + 1)
 				) {
+					// Check that this is likely an actual tool invocation and not
+					// an inline textual reference.
+					// We consider it an invocation only if the next non-whitespace
+					// character is a newline (\n or \r) or an opening angle bracket
+					// '<' (which would start the first parameter tag).
+					let j = currentCharIndex + 1 // Position after the closing '>' of the opening tag.
+
+					while (j < len && assistantMessage[j] === " ") {
+						j++
+					}
+
+					const nextChar = assistantMessage[j] ?? ""
+
+					if (nextChar && nextChar !== "<" && nextChar !== "\n" && nextChar !== "\r") {
+						// Treat as plain text, not a tool invocation.
+						continue
+					}
 					// End current text block if one was active.
 					if (currentTextContent) {
 						currentTextContent.content = assistantMessage
@@ -201,22 +249,13 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 							.trim()
 
 						if (potentialText.length > 0) {
-							contentBlocks.push({
-								type: "text",
-								content: potentialText,
-								partial: false,
-							})
+							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.
-					}
-
+					// Assume partial until closing tag is found.
+					currentToolUse = { type: "tool_use", name: toolName, params: {}, partial: true }
 					currentToolUseStart = currentCharIndex + 1 // Tool content starts after the opening tag.
 					startedNewTool = true
 

src/core/task/Task.ts

@@ -60,7 +60,11 @@ import { SYSTEM_PROMPT } from "../prompts/system"
 import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector"
 import { FileContextTracker } from "../context-tracking/FileContextTracker"
 import { RooIgnoreController } from "../ignore/RooIgnoreController"
-import { type AssistantMessageContent, parseAssistantMessage, presentAssistantMessage } from "../assistant-message"
+import {
+	type AssistantMessageContent,
+	parseAssistantMessageV2 as parseAssistantMessage,
+	presentAssistantMessage,
+} from "../assistant-message"
 import { truncateConversationIfNeeded } from "../sliding-window"
 import { ClineProvider } from "../webview/ClineProvider"
 import { MultiSearchReplaceDiffStrategy } from "../diff/strategies/multi-search-replace"

webview-ui/src/__tests__/utils/command-validation.test.ts

@@ -1,158 +0,0 @@
-import { parseCommand, validateCommand } from "../../utils/command-validation"
-
-/**
- * Tests for the command-validation module
- *
- * These tests include a reproduction of a bug where the shell-quote library
- * used in parseCommand throws an error when parsing commands that contain
- * the Bash $RANDOM variable in array indexing expressions.
- *
- * Error: "Bad substitution: levels[$RANDOM"
- *
- * The issue occurs specifically with complex expressions like:
- * ${levels[$RANDOM % ${#levels[@]}]}
- */
-describe("command-validation", () => {
-	describe("parseCommand", () => {
-		it("should correctly parse simple commands", () => {
-			const result = parseCommand("echo hello")
-			expect(result).toEqual(["echo hello"])
-		})
-
-		it("should correctly parse commands with chaining operators", () => {
-			const result = parseCommand("echo hello && echo world")
-			expect(result).toEqual(["echo hello", "echo world"])
-		})
-
-		it("should correctly parse commands with quotes", () => {
-			const result = parseCommand('echo "hello world"')
-			expect(result).toEqual(['echo "hello world"'])
-		})
-
-		it("should correctly parse commands with redirections", () => {
-			const result = parseCommand("echo hello 2>&1")
-			expect(result).toEqual(["echo hello 2>&1"])
-		})
-
-		it("should correctly parse commands with subshells", () => {
-			const result = parseCommand("echo $(date)")
-			expect(result).toEqual(["echo", "date"])
-		})
-
-		it("should not throw an error when parsing commands with simple array indexing", () => {
-			// Simple array indexing works fine
-			const commandWithArrayIndex = "value=${array[$index]}"
-
-			expect(() => {
-				parseCommand(commandWithArrayIndex)
-			}).not.toThrow()
-		})
-
-		it("should not throw an error when parsing commands with $RANDOM in array index", () => {
-			// This test reproduces the specific bug reported in the error
-			const commandWithRandom = "level=${levels[$RANDOM % ${#levels[@]}]}"
-
-			expect(() => {
-				parseCommand(commandWithRandom)
-			}).not.toThrow()
-		})
-
-		it("should not throw an error with simple $RANDOM variable", () => {
-			// Simple $RANDOM usage works fine
-			const commandWithRandom = "echo $RANDOM"
-
-			expect(() => {
-				parseCommand(commandWithRandom)
-			}).not.toThrow()
-		})
-
-		it("should not throw an error with simple array indexing using $RANDOM", () => {
-			// Simple array indexing with $RANDOM works fine
-			const commandWithRandomIndex = "echo ${array[$RANDOM]}"
-
-			expect(() => {
-				parseCommand(commandWithRandomIndex)
-			}).not.toThrow()
-		})
-
-		it("should not throw an error with complex array indexing using $RANDOM and arithmetic", () => {
-			// This test reproduces the exact expression from the original error
-			const commandWithComplexRandom = "echo ${levels[$RANDOM % ${#levels[@]}]}"
-
-			expect(() => {
-				parseCommand(commandWithComplexRandom)
-			}).not.toThrow("Bad substitution")
-		})
-
-		it("should not throw an error when parsing the full log generator command", () => {
-			// This is the exact command from the original error message
-			const logGeneratorCommand = `while true; do \\
-  levels=(INFO WARN ERROR DEBUG); \\
-  msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\
-  level=\${levels[$RANDOM % \${#levels[@]}]}; \\
-  msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\
-  echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\
-  sleep 1; \\
-done`
-
-			// This reproduces the original error
-			expect(() => {
-				parseCommand(logGeneratorCommand)
-			}).not.toThrow("Bad substitution: levels[$RANDOM")
-		})
-
-		it("should not throw an error when parsing just the problematic part", () => {
-			// This isolates just the part mentioned in the error message
-			const problematicPart = "level=${levels[$RANDOM"
-
-			expect(() => {
-				parseCommand(problematicPart)
-			}).not.toThrow("Bad substitution")
-		})
-	})
-
-	describe("validateCommand", () => {
-		it("should validate allowed commands", () => {
-			const result = validateCommand("echo hello", ["echo"])
-			expect(result).toBe(true)
-		})
-
-		it("should reject disallowed commands", () => {
-			const result = validateCommand("rm -rf /", ["echo", "ls"])
-			expect(result).toBe(false)
-		})
-
-		it("should not fail validation for commands with simple $RANDOM variable", () => {
-			const commandWithRandom = "echo $RANDOM"
-
-			expect(() => {
-				validateCommand(commandWithRandom, ["echo"])
-			}).not.toThrow()
-		})
-
-		it("should not fail validation for commands with simple array indexing using $RANDOM", () => {
-			const commandWithRandomIndex = "echo ${array[$RANDOM]}"
-
-			expect(() => {
-				validateCommand(commandWithRandomIndex, ["echo"])
-			}).not.toThrow()
-		})
-
-		it("should return false for the full log generator command due to subshell detection", () => {
-			// This is the exact command from the original error message
-			const logGeneratorCommand = `while true; do \\
-  levels=(INFO WARN ERROR DEBUG); \\
-  msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\
-  level=\${levels[$RANDOM % \${#levels[@]}]}; \\
-  msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\
-  echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\
-  sleep 1; \\
-done`
-
-			// validateCommand should return false due to subshell detection
-			// without throwing an error
-			const result = validateCommand(logGeneratorCommand, ["while"])
-			expect(result).toBe(false)
-		})
-	})
-})

webview-ui/src/utils/__tests__/command-validation.test.ts

@@ -1,3 +1,8 @@
+/* eslint-disable no-useless-escape */
+/* eslint-disable no-template-curly-in-string */
+
+// npx jest src/utils/__tests__/command-validation.test.ts
+
 import { parseCommand, isAllowedSingleCommand, validateCommand } from "../command-validation"
 
 describe("Command Validation", () => {
@@ -119,3 +124,160 @@ describe("Command Validation", () => {
 		})
 	})
 })
+
+/**
+ * Tests for the command-validation module
+ *
+ * These tests include a reproduction of a bug where the shell-quote library
+ * used in parseCommand throws an error when parsing commands that contain
+ * the Bash $RANDOM variable in array indexing expressions.
+ *
+ * Error: "Bad substitution: levels[$RANDOM"
+ *
+ * The issue occurs specifically with complex expressions like:
+ * ${levels[$RANDOM % ${#levels[@]}]}
+ */
+describe("command-validation", () => {
+	describe("parseCommand", () => {
+		it("should correctly parse simple commands", () => {
+			const result = parseCommand("echo hello")
+			expect(result).toEqual(["echo hello"])
+		})
+
+		it("should correctly parse commands with chaining operators", () => {
+			const result = parseCommand("echo hello && echo world")
+			expect(result).toEqual(["echo hello", "echo world"])
+		})
+
+		it("should correctly parse commands with quotes", () => {
+			const result = parseCommand('echo "hello world"')
+			expect(result).toEqual(['echo "hello world"'])
+		})
+
+		it("should correctly parse commands with redirections", () => {
+			const result = parseCommand("echo hello 2>&1")
+			expect(result).toEqual(["echo hello 2>&1"])
+		})
+
+		it("should correctly parse commands with subshells", () => {
+			const result = parseCommand("echo $(date)")
+			expect(result).toEqual(["echo", "date"])
+		})
+
+		it("should not throw an error when parsing commands with simple array indexing", () => {
+			// Simple array indexing works fine
+			const commandWithArrayIndex = "value=${array[$index]}"
+
+			expect(() => {
+				parseCommand(commandWithArrayIndex)
+			}).not.toThrow()
+		})
+
+		it("should not throw an error when parsing commands with $RANDOM in array index", () => {
+			// This test reproduces the specific bug reported in the error
+			const commandWithRandom = "level=${levels[$RANDOM % ${#levels[@]}]}"
+
+			expect(() => {
+				parseCommand(commandWithRandom)
+			}).not.toThrow()
+		})
+
+		it("should not throw an error with simple $RANDOM variable", () => {
+			// Simple $RANDOM usage works fine
+			const commandWithRandom = "echo $RANDOM"
+
+			expect(() => {
+				parseCommand(commandWithRandom)
+			}).not.toThrow()
+		})
+
+		it("should not throw an error with simple array indexing using $RANDOM", () => {
+			// Simple array indexing with $RANDOM works fine
+			const commandWithRandomIndex = "echo ${array[$RANDOM]}"
+
+			expect(() => {
+				parseCommand(commandWithRandomIndex)
+			}).not.toThrow()
+		})
+
+		it("should not throw an error with complex array indexing using $RANDOM and arithmetic", () => {
+			// This test reproduces the exact expression from the original error
+			const commandWithComplexRandom = "echo ${levels[$RANDOM % ${#levels[@]}]}"
+
+			expect(() => {
+				parseCommand(commandWithComplexRandom)
+			}).not.toThrow("Bad substitution")
+		})
+
+		it("should not throw an error when parsing the full log generator command", () => {
+			// This is the exact command from the original error message
+			const logGeneratorCommand = `while true; do \\
+  levels=(INFO WARN ERROR DEBUG); \\
+  msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\
+  level=\${levels[$RANDOM % \${#levels[@]}]}; \\
+  msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\
+  echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\
+  sleep 1; \\
+done`
+
+			// This reproduces the original error
+			expect(() => {
+				parseCommand(logGeneratorCommand)
+			}).not.toThrow("Bad substitution: levels[$RANDOM")
+		})
+
+		it("should not throw an error when parsing just the problematic part", () => {
+			// This isolates just the part mentioned in the error message
+			const problematicPart = "level=${levels[$RANDOM"
+
+			expect(() => {
+				parseCommand(problematicPart)
+			}).not.toThrow("Bad substitution")
+		})
+	})
+
+	describe("validateCommand", () => {
+		it("should validate allowed commands", () => {
+			const result = validateCommand("echo hello", ["echo"])
+			expect(result).toBe(true)
+		})
+
+		it("should reject disallowed commands", () => {
+			const result = validateCommand("rm -rf /", ["echo", "ls"])
+			expect(result).toBe(false)
+		})
+
+		it("should not fail validation for commands with simple $RANDOM variable", () => {
+			const commandWithRandom = "echo $RANDOM"
+
+			expect(() => {
+				validateCommand(commandWithRandom, ["echo"])
+			}).not.toThrow()
+		})
+
+		it("should not fail validation for commands with simple array indexing using $RANDOM", () => {
+			const commandWithRandomIndex = "echo ${array[$RANDOM]}"
+
+			expect(() => {
+				validateCommand(commandWithRandomIndex, ["echo"])
+			}).not.toThrow()
+		})
+
+		it("should return false for the full log generator command due to subshell detection", () => {
+			// This is the exact command from the original error message
+			const logGeneratorCommand = `while true; do \\
+  levels=(INFO WARN ERROR DEBUG); \\
+  msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\
+  level=\${levels[$RANDOM % \${#levels[@]}]}; \\
+  msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\
+  echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\
+  sleep 1; \\
+done`
+
+			// validateCommand should return false due to subshell detection
+			// without throwing an error
+			const result = validateCommand(logGeneratorCommand, ["while"])
+			expect(result).toBe(false)
+		})
+	})
+})