Browse Source

Revert "Accidental execution of tool syntax fix" (#3560)

Chris Estreich 10 months ago
parent
commit
0f9691ab27

+ 0 - 152
src/core/assistant-message/__tests__/parseAssistantMessage.test.ts

@@ -335,158 +335,6 @@ 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")
-			})
 		})
 	})
 })

+ 6 - 2
src/core/assistant-message/__tests__/parseAssistantMessageBenchmark.ts

@@ -81,13 +81,16 @@ const runBenchmark = () => {
 	const namePadding = maxNameLength + 2
 
 	console.log(
-		`| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1 Heap (bytes)  | V2 Heap (bytes) |`,
+		`| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1/V2 Ratio | V1 Heap (bytes) | V2 Heap (bytes) |`,
+	)
+	console.log(
+		`| ${"-".repeat(namePadding)} | ------------ | ------------ | ----------- | ---------------- | ---------------- |`,
 	)
-	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)
@@ -96,6 +99,7 @@ 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)} |`,
 		)

+ 0 - 1
src/core/assistant-message/index.ts

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

+ 1 - 64
src/core/assistant-message/parseAssistantMessage.ts

@@ -13,52 +13,10 @@ 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)
@@ -87,7 +45,6 @@ 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.
@@ -132,25 +89,6 @@ 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",
@@ -213,6 +151,5 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 		contentBlocks.push(currentTextContent)
 	}
 
-	// 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))
+	return contentBlocks
 }

+ 17 - 56
src/core/assistant-message/parseAssistantMessageV2.ts

@@ -46,14 +46,6 @@ 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>()
@@ -71,39 +63,16 @@ 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 &&
-				// Start checking from potential start of tag.
-				assistantMessage.startsWith(closeTag, currentCharIndex - closeTag.length + 1)
+				assistantMessage.startsWith(
+					closeTag,
+					currentCharIndex - closeTag.length + 1, // Start checking from potential start of tag.
+				)
 			) {
 				// Found the closing tag for the parameter.
 				const value = assistantMessage
@@ -206,23 +175,6 @@ 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
@@ -249,13 +201,22 @@ 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.
-					// Assume partial until closing tag is found.
-					currentToolUse = { type: "tool_use", name: toolName, params: {}, partial: true }
+					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
 

+ 1 - 5
src/core/task/Task.ts

@@ -60,11 +60,7 @@ 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,
-	parseAssistantMessageV2 as parseAssistantMessage,
-	presentAssistantMessage,
-} from "../assistant-message"
+import { type AssistantMessageContent, parseAssistantMessage, presentAssistantMessage } from "../assistant-message"
 import { truncateConversationIfNeeded } from "../sliding-window"
 import { ClineProvider } from "../webview/ClineProvider"
 import { MultiSearchReplaceDiffStrategy } from "../diff/strategies/multi-search-replace"

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

@@ -0,0 +1,158 @@
+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)
+		})
+	})
+})

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

@@ -1,8 +1,3 @@
-/* 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", () => {
@@ -124,160 +119,3 @@ 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)
-		})
-	})
-})