feat: detect unbalanced quotes in terminal commands to prevent hanging

src/core/tools/ExecuteCommandTool.ts

@@ -12,6 +12,7 @@ import { Task } from "../task/Task"
 import { ToolUse, ToolResponse } from "../../shared/tools"
 import { formatResponse } from "../prompts/responses"
 import { unescapeHtmlEntities } from "../../utils/text-normalization"
+import { validateCommandQuotes } from "../../shared/parse-command"
 import { ExitCodeDetails, RooTerminalCallbacks, RooTerminalProcess } from "../../integrations/terminal/types"
 import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
 import { Terminal } from "../../integrations/terminal/Terminal"
@@ -54,6 +55,21 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
 			task.consecutiveMistakeCount = 0
 
 			const unescapedCommand = unescapeHtmlEntities(command)
+
+			// Validate quotes before executing the command to prevent terminal from getting stuck
+			const quoteValidation = validateCommandQuotes(unescapedCommand)
+			if (!quoteValidation.valid) {
+				task.consecutiveMistakeCount++
+				task.recordToolError("execute_command")
+				const errorMessage =
+					`Command has unbalanced ${quoteValidation.quoteType} quotes at position ${quoteValidation.position}. ` +
+					`Context: "${quoteValidation.context}". ` +
+					`This would cause the terminal to hang waiting for the closing quote. ` +
+					`Please fix the command and try again.`
+				pushToolResult(errorMessage)
+				return
+			}
+
 			const didApprove = await askApproval("command", unescapedCommand)
 
 			if (!didApprove) {

src/core/tools/__tests__/executeCommandTool.spec.ts

@@ -251,6 +251,78 @@ describe("executeCommandTool", () => {
 		})
 	})
 
+	describe("Unbalanced quote validation", () => {
+		it("should reject commands with unbalanced double quotes", async () => {
+			// Setup - command with missing closing double quote
+			mockToolUse.params.command = 'echo "hello'
+			mockToolUse.nativeArgs = { command: 'echo "hello' }
+
+			// Execute
+			await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, {
+				askApproval: mockAskApproval as unknown as AskApproval,
+				handleError: mockHandleError as unknown as HandleError,
+				pushToolResult: mockPushToolResult as unknown as PushToolResult,
+			})
+
+			// Verify - should record error and push error result
+			expect(mockCline.consecutiveMistakeCount).toBe(1)
+			expect(mockCline.recordToolError).toHaveBeenCalledWith("execute_command")
+			expect(mockPushToolResult).toHaveBeenCalled()
+			const result = mockPushToolResult.mock.calls[0][0]
+			expect(result).toContain("unbalanced")
+			expect(result).toContain("double")
+			expect(result).toContain("quotes")
+			// Should not reach approval step
+			expect(mockAskApproval).not.toHaveBeenCalled()
+			// executeCommandInTerminal should not be called
+			expect(executeCommandModule.executeCommandInTerminal).not.toHaveBeenCalled()
+		})
+
+		it("should reject commands with unbalanced single quotes", async () => {
+			// Setup - command with missing closing single quote
+			mockToolUse.params.command = "echo 'hello"
+			mockToolUse.nativeArgs = { command: "echo 'hello" }
+
+			// Execute
+			await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, {
+				askApproval: mockAskApproval as unknown as AskApproval,
+				handleError: mockHandleError as unknown as HandleError,
+				pushToolResult: mockPushToolResult as unknown as PushToolResult,
+			})
+
+			// Verify - should record error and push error result
+			expect(mockCline.consecutiveMistakeCount).toBe(1)
+			expect(mockCline.recordToolError).toHaveBeenCalledWith("execute_command")
+			expect(mockPushToolResult).toHaveBeenCalled()
+			const result = mockPushToolResult.mock.calls[0][0]
+			expect(result).toContain("unbalanced")
+			expect(result).toContain("single")
+			expect(result).toContain("quotes")
+			// Should not reach approval step
+			expect(mockAskApproval).not.toHaveBeenCalled()
+			// executeCommandInTerminal should not be called
+			expect(executeCommandModule.executeCommandInTerminal).not.toHaveBeenCalled()
+		})
+
+		it("should accept commands with balanced quotes", async () => {
+			// Setup - command with properly balanced quotes
+			mockToolUse.params.command = 'echo "hello world"'
+			mockToolUse.nativeArgs = { command: 'echo "hello world"' }
+
+			// Execute
+			await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, {
+				askApproval: mockAskApproval as unknown as AskApproval,
+				handleError: mockHandleError as unknown as HandleError,
+				pushToolResult: mockPushToolResult as unknown as PushToolResult,
+			})
+
+			// Verify - should proceed to approval
+			expect(mockAskApproval).toHaveBeenCalled()
+			// Should not record an error for quote validation
+			expect(mockCline.recordToolError).not.toHaveBeenCalledWith("execute_command")
+		})
+	})
+
 	describe("Command execution timeout configuration", () => {
 		it("should include timeout parameter in ExecuteCommandOptions", () => {
 			// This test verifies that the timeout configuration is properly typed

src/shared/__tests__/parse-command.spec.ts

@@ -0,0 +1,197 @@
+// npx vitest run src/shared/__tests__/parse-command.spec.ts
+
+import { describe, it, expect } from "vitest"
+import { validateCommandQuotes } from "../parse-command"
+
+describe("validateCommandQuotes", () => {
+	describe("valid commands", () => {
+		it("should accept empty or whitespace-only commands", () => {
+			expect(validateCommandQuotes("")).toEqual({ valid: true })
+			expect(validateCommandQuotes("   ")).toEqual({ valid: true })
+			expect(validateCommandQuotes("\t\n")).toEqual({ valid: true })
+		})
+
+		it("should accept commands without quotes", () => {
+			expect(validateCommandQuotes("echo hello")).toEqual({ valid: true })
+			expect(validateCommandQuotes("ls -la")).toEqual({ valid: true })
+			expect(validateCommandQuotes("npm install")).toEqual({ valid: true })
+		})
+
+		it("should accept commands with balanced double quotes", () => {
+			expect(validateCommandQuotes('echo "hello world"')).toEqual({ valid: true })
+			expect(validateCommandQuotes('echo "hello" && echo "world"')).toEqual({ valid: true })
+			expect(validateCommandQuotes('cat "file with spaces.txt"')).toEqual({ valid: true })
+		})
+
+		it("should accept commands with balanced single quotes", () => {
+			expect(validateCommandQuotes("echo 'hello world'")).toEqual({ valid: true })
+			expect(validateCommandQuotes("echo 'hello' && echo 'world'")).toEqual({ valid: true })
+			expect(validateCommandQuotes("cat 'file with spaces.txt'")).toEqual({ valid: true })
+		})
+
+		it("should accept commands with mixed balanced quotes", () => {
+			expect(validateCommandQuotes(`echo "it's working"`)).toEqual({ valid: true })
+			expect(validateCommandQuotes(`echo 'say "hello"'`)).toEqual({ valid: true })
+			expect(validateCommandQuotes(`git commit -m "it's a 'test'"`)).toEqual({ valid: true })
+		})
+
+		it("should accept commands with escaped double quotes", () => {
+			expect(validateCommandQuotes('echo "hello \\"world\\""')).toEqual({ valid: true })
+			expect(validateCommandQuotes('echo "test \\"nested\\" value"')).toEqual({ valid: true })
+		})
+
+		it("should accept heredoc syntax", () => {
+			expect(validateCommandQuotes("cat <<EOF")).toEqual({ valid: true })
+			expect(validateCommandQuotes("cat <<-EOF")).toEqual({ valid: true })
+			expect(validateCommandQuotes("cat <<'EOF'")).toEqual({ valid: true })
+			expect(validateCommandQuotes('cat <<"EOF"')).toEqual({ valid: true })
+			expect(validateCommandQuotes("cat << EOF")).toEqual({ valid: true })
+			expect(validateCommandQuotes("cat <<MARKER")).toEqual({ valid: true })
+		})
+
+		it("should accept ANSI-C quoting", () => {
+			expect(validateCommandQuotes("echo $'hello\\nworld'")).toEqual({ valid: true })
+			expect(validateCommandQuotes("echo $'tab\\there'")).toEqual({ valid: true })
+		})
+
+		it("should accept complex real-world commands", () => {
+			expect(validateCommandQuotes('git log --oneline --format="%h %s"')).toEqual({ valid: true })
+			expect(validateCommandQuotes("find . -name '*.ts' -exec cat {} \\;")).toEqual({ valid: true })
+			expect(validateCommandQuotes('docker run -e "VAR=value" image')).toEqual({ valid: true })
+			expect(validateCommandQuotes("ssh user@host 'ls -la'")).toEqual({ valid: true })
+		})
+	})
+
+	describe("invalid commands with unbalanced quotes", () => {
+		it("should detect unbalanced double quotes at the start", () => {
+			const result = validateCommandQuotes('echo "hello')
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("double")
+				expect(result.position).toBe(5)
+			}
+		})
+
+		it("should detect unbalanced double quotes in the middle", () => {
+			const result = validateCommandQuotes('echo hello && cat "file.txt')
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("double")
+			}
+		})
+
+		it("should detect unbalanced single quotes at the start", () => {
+			const result = validateCommandQuotes("echo 'hello")
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("single")
+				expect(result.position).toBe(5)
+			}
+		})
+
+		it("should detect unbalanced single quotes in the middle", () => {
+			const result = validateCommandQuotes("echo hello && cat 'file.txt")
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("single")
+			}
+		})
+
+		it("should detect odd number of double quotes", () => {
+			const result = validateCommandQuotes('echo "hello" "world')
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("double")
+			}
+		})
+
+		it("should detect odd number of single quotes", () => {
+			const result = validateCommandQuotes("echo 'hello' 'world")
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("single")
+			}
+		})
+
+		it("should provide context in error result", () => {
+			const result = validateCommandQuotes('echo "hello world')
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.context).toContain('"')
+				expect(result.context.length).toBeLessThanOrEqual(30) // Context is bounded
+			}
+		})
+	})
+
+	describe("edge cases", () => {
+		it("should handle quotes at the very end of command", () => {
+			const result = validateCommandQuotes('echo test"')
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("double")
+			}
+		})
+
+		it("should handle consecutive quotes", () => {
+			expect(validateCommandQuotes('echo ""')).toEqual({ valid: true })
+			expect(validateCommandQuotes("echo ''")).toEqual({ valid: true })
+			expect(validateCommandQuotes(`echo """"""`)).toEqual({ valid: true }) // Three pairs
+		})
+
+		it("should handle backslash escaping the closing quote (unbalanced)", () => {
+			// In shell, \" escapes the quote, so echo "test\" is unbalanced
+			// The string `echo "test\"` has an escaped quote, leaving it unclosed
+			const result = validateCommandQuotes('echo "test\\"')
+			expect(result.valid).toBe(false)
+			if (!result.valid) {
+				expect(result.quoteType).toBe("double")
+			}
+		})
+
+		it("should handle literal backslash followed by closing quote (balanced)", () => {
+			// To have a backslash at the end of a quoted string, you need \\"
+			// In shell: echo "test\\" means content is "test\" (backslash at end)
+			// In JS: 'echo "test\\\\"' represents the shell command: echo "test\\"
+			expect(validateCommandQuotes('echo "test\\\\"')).toEqual({ valid: true })
+		})
+
+		it("should handle single quote in double quoted string (valid)", () => {
+			expect(validateCommandQuotes(`echo "it's fine"`)).toEqual({ valid: true })
+		})
+
+		it("should handle double quote in single quoted string (valid)", () => {
+			expect(validateCommandQuotes(`echo 'say "hi"'`)).toEqual({ valid: true })
+		})
+
+		it("should handle escaped single quote outside of quotes", () => {
+			expect(validateCommandQuotes("echo it\\'s")).toEqual({ valid: true })
+		})
+
+		it("should handle multiple unbalanced - reports first one", () => {
+			// If both quote types are unbalanced, we find whichever starts first
+			const result = validateCommandQuotes(`echo "hello 'world`)
+			expect(result.valid).toBe(false)
+			// Double quote at position 5 starts before single quote at position 12
+			if (!result.valid) {
+				expect(result.quoteType).toBe("double")
+			}
+		})
+	})
+
+	describe("shell-specific patterns", () => {
+		it("should accept bash variable substitution in quotes", () => {
+			expect(validateCommandQuotes('echo "$HOME/path"')).toEqual({ valid: true })
+			expect(validateCommandQuotes('echo "${VAR:-default}"')).toEqual({ valid: true })
+		})
+
+		it("should accept command substitution in quotes", () => {
+			expect(validateCommandQuotes('echo "Today is $(date)"')).toEqual({ valid: true })
+			expect(validateCommandQuotes("echo 'Date: $(date)'")).toEqual({ valid: true })
+		})
+
+		it("should handle PowerShell style quotes", () => {
+			expect(validateCommandQuotes('Write-Output "Hello World"')).toEqual({ valid: true })
+			expect(validateCommandQuotes("Write-Output 'Hello World'")).toEqual({ valid: true })
+		})
+	})
+})