fix: address PR review feedback

- Refactored showSuggestions from state variable to constant SHOW_SUGGESTIONS
- Renamed breakingExps to stopPatterns for better clarity
- Added test coverage for edge cases in command parsing
- Fixed test assertion for multiline content handling
This commit is contained in:
Roo Code 2025-07-21 17:09:58 +00:00 committed by hannesrudolph
parent dd533c2af7
commit b24400b549
3 changed files with 56 additions and 4 deletions

View file

@ -50,7 +50,8 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
const [isExpanded, setIsExpanded] = useState(terminalShellIntegrationDisabled)
const [streamingOutput, setStreamingOutput] = useState("")
const [status, setStatus] = useState<CommandExecutionStatus | null>(null)
const showSuggestions = true
// Show suggestions is always enabled for command pattern management
const SHOW_SUGGESTIONS = true
// The command's output can either come from the text associated with the
// task message (this is the case for completed commands) or from the
@ -195,7 +196,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
<CodeBlock source={command} language="shell" />
<OutputContainer isExpanded={isExpanded} output={output} />
</div>
{showSuggestions && commandPatterns.length > 0 && (
{SHOW_SUGGESTIONS && commandPatterns.length > 0 && (
<CommandPatternSelector
patterns={commandPatterns}
allowedCommands={allowedCommands}

View file

@ -412,5 +412,56 @@ Suggested patterns: npm, npm test, npm run
expect(conflictState.setAllowedCommands).toHaveBeenCalledWith(["git", "git push"])
expect(conflictState.setDeniedCommands).toHaveBeenCalledWith([])
})
it("should handle commands that cannot be parsed and fallback gracefully", () => {
// Test with a command that might cause parsing issues
const unparsableCommand = "echo 'test with unclosed quote"
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-12" text={unparsableCommand} />
</ExtensionStateWrapper>,
)
// Should still render the command
expect(screen.getByTestId("code-block")).toHaveTextContent("echo 'test with unclosed quote")
// Should show pattern selector with at least the main command
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
expect(screen.getByText("echo")).toBeInTheDocument()
})
it("should handle empty or whitespace-only commands", () => {
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-13" text=" " />
</ExtensionStateWrapper>,
)
// Should render without errors
expect(screen.getByTestId("code-block")).toBeInTheDocument()
// Should not show pattern selector for empty commands
expect(screen.queryByTestId("command-pattern-selector")).not.toBeInTheDocument()
})
it("should handle commands with only output and no command prefix", () => {
const outputOnly = `Some output without a command
Multiple lines of output
Without any command prefix`
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-14" text={outputOnly} />
</ExtensionStateWrapper>,
)
// Should treat the entire text as command when no prefix is found
const codeBlock = screen.getByTestId("code-block")
// The mock CodeBlock component renders text content without preserving newlines
expect(codeBlock.textContent).toContain("Some output without a command")
expect(codeBlock.textContent).toContain("Multiple lines of output")
expect(codeBlock.textContent).toContain("Without any command prefix")
})
})
})

View file

@ -52,12 +52,12 @@ function processCommand(cmd: any[], patterns: Set<string>) {
patterns.add(mainCmd)
// Patterns that indicate we should stop looking for subcommands
const breakingExps = [/^-/, /[\\/.~ ]/]
const stopPatterns = [/^-/, /[\\/.~ ]/]
// Build up patterns progressively
for (let i = 1; i < cmd.length; i++) {
const arg = cmd[i]
if (typeof arg !== "string" || breakingExps.some((re) => re.test(arg))) break
if (typeof arg !== "string" || stopPatterns.some((re) => re.test(arg))) break
const pattern = cmd.slice(0, i + 1).join(" ")
patterns.add(pattern)