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.
This commit is contained in:
hannesrudolph 2025-07-22 14:50:52 -06:00
parent 860631cfe3
commit ebf1b241c1
3 changed files with 148 additions and 43 deletions

View file

@ -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", () => {

View file

@ -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,43 +164,50 @@ 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")
@ -205,8 +215,9 @@ Output here...
})
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", () => {

View file

@ -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