fix: improve command parsing to handle Output: separator correctly

- Fixed parseCommandAndOutput to properly handle the newline + 'Output:' separator
- Added test cases for commands with numbers at the start of output lines
- Updated existing tests to use template literals for proper newline handling
- Fixed test assertions to handle multiple code blocks when output is present

This resolves the issue where output lines starting with numbers (like 'wc -l' output)
were being incorrectly parsed as the command instead of the actual command text.

Author: hannesrudolph

webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx

@@ -21,6 +21,19 @@ vi.mock("../../common/CodeBlock", () => ({
 	default: ({ source }: { source: string }) => <div data-testid="code-block">{source}</div>,
 }))
 
+// Mock the commandPatterns module but use the actual implementation
+vi.mock("../../../utils/commandPatterns", async () => {
+	const actual = await vi.importActual<typeof import("../../../utils/commandPatterns")>(
+		"../../../utils/commandPatterns",
+	)
+	return {
+		...actual,
+		parseCommandAndOutput: actual.parseCommandAndOutput,
+		extractCommandPatterns: actual.extractCommandPatterns,
+		getPatternDescription: actual.getPatternDescription,
+	}
+})
+
 vi.mock("../CommandPatternSelector", () => ({
 	CommandPatternSelector: ({ patterns, onAllowPatternChange, onDenyPatternChange }: any) => (
 		<div data-testid="command-pattern-selector">
@@ -66,7 +79,7 @@ describe("CommandExecution", () => {
 	it("should render command with output", () => {
 		render(
 			<ExtensionStateWrapper>
-				<CommandExecution executionId="test-1" text="npm install\n\nCommand Output:\nInstalling packages..." />
+				<CommandExecution executionId="test-1" text="npm install\nOutput:\nInstalling packages..." />
 			</ExtensionStateWrapper>,
 		)
 
@@ -166,29 +179,42 @@ describe("CommandExecution", () => {
 		expect(vscode.postMessage).toHaveBeenCalledWith({ type: "deniedCommands", commands: [] })
 	})
 
-	it("should parse command with $ prefix", () => {
+	it("should parse command with Output: separator", () => {
+		const commandText = `npm install
+Output:
+Installing...`
+
 		render(
 			<ExtensionStateWrapper>
-				<CommandExecution executionId="test-1" text="$ npm install\nInstalling..." />
+				<CommandExecution executionId="test-1" text={commandText} />
 			</ExtensionStateWrapper>,
 		)
 
-		expect(screen.getByTestId("code-block")).toHaveTextContent("npm install")
+		const codeBlocks = screen.getAllByTestId("code-block")
+		expect(codeBlocks[0]).toHaveTextContent("npm install")
 	})
 
 	it("should parse command with AI suggestions", () => {
+		const commandText = `npm install
+Output:
+Suggested patterns: npm, npm install, npm run`
+
 		render(
 			<ExtensionStateWrapper>
-				<CommandExecution
-					executionId="test-1"
-					text="$ npm install\nSuggested patterns: npm, npm install, npm run"
-				/>
+				<CommandExecution executionId="test-1" text={commandText} />
 			</ExtensionStateWrapper>,
 		)
 
+		// First check that the command was parsed correctly
+		const codeBlocks = screen.getAllByTestId("code-block")
+		expect(codeBlocks[0]).toHaveTextContent("npm install")
+		expect(codeBlocks[1]).toHaveTextContent("Suggested patterns: npm, npm install, npm run")
+
 		expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
 		// Check that the patterns are present in the mock
 		expect(screen.getByText("npm")).toBeInTheDocument()
+		expect(screen.getAllByText("npm install").length).toBeGreaterThan(0)
+		expect(screen.getByText("npm run")).toBeInTheDocument()
 	})
 
 	it("should handle commands with pipes", () => {
@@ -232,14 +258,20 @@ describe("CommandExecution", () => {
 			terminalShellIntegrationDisabled: true,
 		}
 
+		const commandText = `npm install
+Output:
+Output here`
+
 		render(
 			<ExtensionStateContext.Provider value={disabledState as any}>
-				<CommandExecution executionId="test-1" text="npm install\n\nCommand Output:\nOutput here" />
+				<CommandExecution executionId="test-1" text={commandText} />
 			</ExtensionStateContext.Provider>,
 		)
 
 		// Output should be visible when shell integration is disabled
-		expect(screen.getByText(/Output here/)).toBeInTheDocument()
+		const codeBlocks = screen.getAllByTestId("code-block")
+		expect(codeBlocks).toHaveLength(2) // Command and output blocks
+		expect(codeBlocks[1]).toHaveTextContent("Output here")
 	})
 
 	it("should handle undefined allowedCommands and deniedCommands", () => {

webview-ui/src/utils/__tests__/commandPatterns.spec.ts

@@ -133,25 +133,28 @@ describe("getPatternDescription", () => {
 })
 
 describe("parseCommandAndOutput", () => {
-	it("should parse command with $ prefix", () => {
+	it("should handle command with $ prefix without Output: separator", () => {
 		const text = "$ npm install\nInstalling packages..."
 		const result = parseCommandAndOutput(text)
-		expect(result.command).toBe("npm install")
-		expect(result.output).toBe("Installing packages...")
+		// Without Output: separator, the entire text is treated as command
+		expect(result.command).toBe("$ npm install\nInstalling packages...")
+		expect(result.output).toBe("")
 	})
 
-	it("should parse command with ❯ prefix", () => {
+	it("should handle command with ❯ prefix without Output: separator", () => {
 		const text = "❯ git status\nOn branch main"
 		const result = parseCommandAndOutput(text)
-		expect(result.command).toBe("git status")
-		expect(result.output).toBe("On branch main")
+		// Without Output: separator, the entire text is treated as command
+		expect(result.command).toBe("❯ git status\nOn branch main")
+		expect(result.output).toBe("")
 	})
 
-	it("should parse command with > prefix", () => {
+	it("should handle command with > prefix without Output: separator", () => {
 		const text = "> echo hello\nhello"
 		const result = parseCommandAndOutput(text)
-		expect(result.command).toBe("echo hello")
-		expect(result.output).toBe("hello")
+		// Without Output: separator, the entire text is treated as command
+		expect(result.command).toBe("> echo hello\nhello")
+		expect(result.output).toBe("")
 	})
 
 	it("should return original text if no command prefix found", () => {
@@ -161,52 +164,60 @@ describe("parseCommandAndOutput", () => {
 		expect(result.output).toBe("")
 	})
 
-	it("should extract AI suggestions from output", () => {
-		const text = "$ npm install\nSuggested patterns: npm, npm install, npm run"
+	it("should extract AI suggestions from output with Output: separator", () => {
+		const text = "npm install\nOutput:\nSuggested patterns: npm, npm install, npm run"
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("npm install")
 		expect(result.suggestions).toEqual(["npm", "npm install", "npm run"])
 	})
 
 	it("should extract suggestions with different formats", () => {
-		const text = "$ git push\nCommand patterns: git, git push"
+		const text = "git push\nOutput:\nCommand patterns: git, git push"
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("git push")
 		expect(result.suggestions).toEqual(["git", "git push"])
 	})
 
 	it('should extract suggestions from "you can allow" format', () => {
-		const text = "$ docker run\nYou can allow: docker, docker run"
+		const text = "docker run\nOutput:\nYou can allow: docker, docker run"
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("docker run")
 		expect(result.suggestions).toEqual(["docker", "docker run"])
 	})
 
 	it("should extract suggestions from bullet points", () => {
-		const text = `$ npm test
+		const text = `npm test
+Output:
 Output here...
 - npm
 - npm test
 - npm run`
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("npm test")
 		expect(result.suggestions).toContain("npm")
 		expect(result.suggestions).toContain("npm test")
 		expect(result.suggestions).toContain("npm run")
 	})
 
 	it("should extract suggestions from various bullet formats", () => {
-		const text = `$ command
+		const text = `command
+Output:
 • npm
 * git
 - docker
 ▪ python`
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("command")
 		expect(result.suggestions).toContain("npm")
 		expect(result.suggestions).toContain("git")
 		expect(result.suggestions).toContain("docker")
 		expect(result.suggestions).toContain("python")
 	})
 
 	it("should extract suggestions with backticks", () => {
-		const text = "$ npm install\n- `npm`\n- `npm install`"
+		const text = "npm install\nOutput:\n- `npm`\n- `npm install`"
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("npm install")
 		expect(result.suggestions).toContain("npm")
 		expect(result.suggestions).toContain("npm install")
 	})
@@ -218,25 +229,28 @@ Output here...
 		expect(result.suggestions).toEqual([])
 	})
 
-	it("should handle multiline commands", () => {
+	it("should handle multiline commands without Output: separator", () => {
 		const text = `$ npm install \\
-  express \\
-  mongoose
+	 express \\
+	 mongoose
 Installing...`
 		const result = parseCommandAndOutput(text)
-		expect(result.command).toBe("npm install \\")
-		expect(result.output).toContain("express")
+		// Without Output: separator, entire text is treated as command
+		expect(result.command).toBe(text)
+		expect(result.output).toBe("")
 	})
 
-	it("should include all suggestions from comma-separated list", () => {
-		const text = "$ test\nSuggested patterns: npm, npm install, npm run"
+	it("should include all suggestions from comma-separated list with Output: separator", () => {
+		const text = "test\nOutput:\nSuggested patterns: npm, npm install, npm run"
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("test")
 		expect(result.suggestions).toEqual(["npm", "npm install", "npm run"])
 	})
 
 	it("should handle case variations in suggestion patterns", () => {
-		const text = "$ test\nSuggested Patterns: npm, git\nCommand Patterns: docker"
+		const text = "test\nOutput:\nSuggested Patterns: npm, git\nCommand Patterns: docker"
 		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("test")
 		// Now it should accumulate all suggestions
 		expect(result.suggestions).toContain("npm")
 		expect(result.suggestions).toContain("git")
@@ -277,6 +291,55 @@ Installing...`
 		expect(result.command).toBe('echo "test"')
 		expect(result.output).toBe("First output\nOutput: Second output")
 	})
+
+	it("should handle output with numbers at the start of lines", () => {
+		const text = `wc -l *.go *.java
+Output:
+25 hello_world.go
+316 HelloWorld.java
+341 total`
+		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("wc -l *.go *.java")
+		expect(result.output).toBe("25 hello_world.go\n316 HelloWorld.java\n341 total")
+		expect(result.suggestions).toEqual([])
+	})
+
+	it("should handle edge case where text starts with Output:", () => {
+		const text = "Output:\nSome output without a command"
+		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("")
+		expect(result.output).toBe("Some output without a command")
+	})
+
+	it("should not be confused by Output: appearing in the middle of output", () => {
+		const text = `echo "Output: test"
+Output:
+Output: test`
+		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe('echo "Output: test"')
+		expect(result.output).toBe("Output: test")
+	})
+
+	it("should handle commands without shell prompt when Output: separator is present", () => {
+		const text = `npm install
+Output:
+Installing packages...`
+		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("npm install")
+		expect(result.output).toBe("Installing packages...")
+	})
+
+	it("should not parse shell prompts from output when Output: separator exists", () => {
+		const text = `ls -la
+Output:
+$ total 341
+drwxr-xr-x  10 user  staff   320 Jan 22 12:00 .
+drwxr-xr-x  20 user  staff   640 Jan 22 11:00 ..`
+		const result = parseCommandAndOutput(text)
+		expect(result.command).toBe("ls -la")
+		expect(result.output).toContain("$ total 341")
+		expect(result.output).toContain("drwxr-xr-x")
+	})
 })
 
 describe("detectSecurityIssues", () => {

webview-ui/src/utils/commandPatterns.ts

@@ -101,20 +101,30 @@ export function parseCommandAndOutput(text: string): {
 	// First check if the text already has been split by COMMAND_OUTPUT_STRING
 	// This happens when the command has already been executed and we have the output
 	const outputSeparator = "Output:"
-	const outputIndex = text.indexOf(outputSeparator)
+	const outputIndex = text.indexOf(`\n${outputSeparator}`)
 
 	if (outputIndex !== -1) {
 		// Text is already split into command and output
+		// The command is everything before the output separator
 		result.command = text.slice(0, outputIndex).trim()
-		result.output = text.slice(outputIndex + outputSeparator.length).trim()
-	} else {
-		// Try to extract command from the text
-		// Look for patterns like "$ command" or "❯ command" at the start
-		const commandMatch = text.match(/^[$❯>]\s*(.+?)(?:\n|$)/m)
-		if (commandMatch) {
-			result.command = commandMatch[1].trim()
-			result.output = text.substring(commandMatch.index! + commandMatch[0].length).trim()
+		// The output is everything after the output separator
+		// We need to skip the newline and "Output:" text
+		const afterNewline = outputIndex + 1 // Skip the newline
+		const afterSeparator = afterNewline + outputSeparator.length // Skip "Output:"
+		// Check if there's a colon and potential space after it
+		let startOfOutput = afterSeparator
+		if (text[afterSeparator] === "\n") {
+			startOfOutput = afterSeparator + 1 // Skip additional newline after "Output:"
 		}
+		result.output = text.slice(startOfOutput).trim()
+	} else if (text.indexOf(outputSeparator) === 0) {
+		// Edge case: text starts with "Output:" (no command)
+		result.command = ""
+		result.output = text.slice(outputSeparator.length).trim()
+	} else {
+		// No output separator found, the entire text is the command
+		result.command = text.trim()
+		result.output = ""
 	}
 
 	// Look for AI suggestions in the output