mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-07 02:58:15 +00:00
Fix the UI for approving chained commands (#6623)
This commit is contained in:
Matt Rubens
GitHub
parent
3f966dfaa3
commit
82a007a211
4 changed files with 53 additions and 52 deletions
|
|
@ -192,7 +192,6 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
|
|||
</div>
|
||||
{command && command.trim() && (
|
||||
<CommandPatternSelector
|
||||
command={command}
|
||||
patterns={commandPatterns}
|
||||
allowedCommands={allowedCommands}
|
||||
deniedCommands={deniedCommands}
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ interface CommandPattern {
|
|||
}
|
||||
|
||||
interface CommandPatternSelectorProps {
|
||||
command: string
|
||||
patterns: CommandPattern[]
|
||||
allowedCommands: string[]
|
||||
deniedCommands: string[]
|
||||
|
|
@ -20,7 +19,6 @@ interface CommandPatternSelectorProps {
|
|||
}
|
||||
|
||||
export const CommandPatternSelector: React.FC<CommandPatternSelectorProps> = ({
|
||||
command,
|
||||
patterns,
|
||||
allowedCommands,
|
||||
deniedCommands,
|
||||
|
|
@ -37,13 +35,8 @@ export const CommandPatternSelector: React.FC<CommandPatternSelectorProps> = ({
|
|||
|
||||
// Create a combined list with full command first, then patterns
|
||||
const allPatterns = useMemo(() => {
|
||||
// Trim the command to ensure consistency with extracted patterns
|
||||
const trimmedCommand = command.trim()
|
||||
const fullCommandPattern: CommandPattern = { pattern: trimmedCommand }
|
||||
|
||||
// Create a set to track unique patterns we've already seen
|
||||
const seenPatterns = new Set<string>()
|
||||
seenPatterns.add(trimmedCommand) // Add the trimmed full command first
|
||||
|
||||
// Filter out any patterns that are duplicates or are the same as the full command
|
||||
const uniquePatterns = patterns.filter((p) => {
|
||||
|
|
@ -54,8 +47,8 @@ export const CommandPatternSelector: React.FC<CommandPatternSelectorProps> = ({
|
|||
return true
|
||||
})
|
||||
|
||||
return [fullCommandPattern, ...uniquePatterns]
|
||||
}, [command, patterns])
|
||||
return uniquePatterns
|
||||
}, [patterns])
|
||||
|
||||
const getPatternStatus = (pattern: string): "allowed" | "denied" | "none" => {
|
||||
if (allowedCommands.includes(pattern)) return "allowed"
|
||||
|
|
|
|||
|
|
@ -22,11 +22,13 @@ vi.mock("../../common/CodeBlock", () => ({
|
|||
}))
|
||||
|
||||
vi.mock("../CommandPatternSelector", () => ({
|
||||
CommandPatternSelector: ({ command, onAllowPatternChange, onDenyPatternChange }: any) => (
|
||||
CommandPatternSelector: ({ patterns, onAllowPatternChange, onDenyPatternChange }: any) => (
|
||||
<div data-testid="command-pattern-selector">
|
||||
<span>{command}</span>
|
||||
<button onClick={() => onAllowPatternChange(command)}>Allow {command}</button>
|
||||
<button onClick={() => onDenyPatternChange(command)}>Deny {command}</button>
|
||||
{patterns.map((pattern: any, index: number) => (
|
||||
<span key={index}>{pattern.pattern}</span>
|
||||
))}
|
||||
<button onClick={() => onAllowPatternChange(patterns[0]?.pattern)}>Allow</button>
|
||||
<button onClick={() => onDenyPatternChange(patterns[0]?.pattern)}>Deny</button>
|
||||
</div>
|
||||
),
|
||||
}))
|
||||
|
|
@ -104,7 +106,7 @@ describe("CommandExecution", () => {
|
|||
</ExtensionStateWrapper>,
|
||||
)
|
||||
|
||||
const allowButton = screen.getByText("Allow git push")
|
||||
const allowButton = screen.getByText("Allow")
|
||||
fireEvent.click(allowButton)
|
||||
|
||||
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith(["npm", "git push"])
|
||||
|
|
@ -120,7 +122,7 @@ describe("CommandExecution", () => {
|
|||
</ExtensionStateWrapper>,
|
||||
)
|
||||
|
||||
const denyButton = screen.getByText("Deny docker run")
|
||||
const denyButton = screen.getByText("Deny")
|
||||
fireEvent.click(denyButton)
|
||||
|
||||
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith(["npm"])
|
||||
|
|
@ -143,7 +145,7 @@ describe("CommandExecution", () => {
|
|||
</ExtensionStateContext.Provider>,
|
||||
)
|
||||
|
||||
const allowButton = screen.getByText("Allow npm test")
|
||||
const allowButton = screen.getByText("Allow")
|
||||
fireEvent.click(allowButton)
|
||||
|
||||
// "npm test" is already in allowedCommands, so it should be removed
|
||||
|
|
@ -167,7 +169,7 @@ describe("CommandExecution", () => {
|
|||
</ExtensionStateContext.Provider>,
|
||||
)
|
||||
|
||||
const denyButton = screen.getByText("Deny rm -rf")
|
||||
const denyButton = screen.getByText("Deny")
|
||||
fireEvent.click(denyButton)
|
||||
|
||||
// "rm -rf" is already in deniedCommands, so it should be removed
|
||||
|
|
@ -223,7 +225,8 @@ Suggested patterns: npm, npm install, npm run`
|
|||
|
||||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
expect(selector).toHaveTextContent("ls -la | grep test")
|
||||
// Should show one of the individual commands from the pipe
|
||||
expect(selector.textContent).toMatch(/ls -la|grep test/)
|
||||
})
|
||||
|
||||
it("should handle commands with && operator", () => {
|
||||
|
|
@ -235,7 +238,8 @@ Suggested patterns: npm, npm install, npm run`
|
|||
|
||||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
expect(selector).toHaveTextContent("npm install && npm test")
|
||||
// Should show one of the individual commands from the && chain
|
||||
expect(selector.textContent).toMatch(/npm install|npm test|npm/)
|
||||
})
|
||||
|
||||
it("should not show pattern selector for empty commands", () => {
|
||||
|
|
@ -301,7 +305,7 @@ Output here`
|
|||
</ExtensionStateContext.Provider>,
|
||||
)
|
||||
|
||||
const allowButton = screen.getByText("Allow rm file.txt")
|
||||
const allowButton = screen.getByText("Allow")
|
||||
fireEvent.click(allowButton)
|
||||
|
||||
// "rm file.txt" should be removed from denied and added to allowed
|
||||
|
|
@ -321,7 +325,8 @@ Output here`
|
|||
|
||||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
expect(selector).toHaveTextContent("npm install && npm test || echo 'failed'")
|
||||
// Should show one of the individual commands from the complex chain
|
||||
expect(selector.textContent).toMatch(/npm install|npm test|echo|npm/)
|
||||
})
|
||||
|
||||
it("should handle commands with output", () => {
|
||||
|
|
@ -356,7 +361,8 @@ Other output here`
|
|||
|
||||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
expect(selector).toHaveTextContent("echo $(whoami) && git status")
|
||||
// Should show one of the individual commands
|
||||
expect(selector.textContent).toMatch(/echo|whoami|git status|git/)
|
||||
})
|
||||
|
||||
it("should handle commands with backtick subshells", () => {
|
||||
|
|
@ -368,7 +374,8 @@ Other output here`
|
|||
|
||||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
expect(selector).toHaveTextContent("git commit -m `date`")
|
||||
// Should show one of the individual commands
|
||||
expect(selector.textContent).toMatch(/git commit|date|git/)
|
||||
})
|
||||
|
||||
it("should handle commands with special characters", () => {
|
||||
|
|
@ -380,7 +387,8 @@ Other output here`
|
|||
|
||||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
expect(selector).toHaveTextContent("cd ~/projects && npm start")
|
||||
// Should show one of the individual commands
|
||||
expect(selector.textContent).toMatch(/cd ~\/projects|npm start|cd|npm/)
|
||||
})
|
||||
|
||||
it("should handle commands with mixed content including output", () => {
|
||||
|
|
@ -421,7 +429,7 @@ Running tests...
|
|||
)
|
||||
|
||||
// Click to allow "git push origin main"
|
||||
const allowButton = screen.getByText("Allow git push origin main")
|
||||
const allowButton = screen.getByText("Allow")
|
||||
fireEvent.click(allowButton)
|
||||
|
||||
// Should add to allowed and remove from denied
|
||||
|
|
@ -442,10 +450,10 @@ Running tests...
|
|||
// Should still render the command
|
||||
expect(screen.getByTestId("code-block")).toHaveTextContent("echo 'test with unclosed quote")
|
||||
|
||||
// Should show pattern selector with the full command
|
||||
// Should show pattern selector with a command pattern
|
||||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
expect(selector).toHaveTextContent("echo 'test with unclosed quote")
|
||||
expect(selector.textContent).toMatch(/echo/)
|
||||
})
|
||||
|
||||
it("should handle empty or whitespace-only commands", () => {
|
||||
|
|
@ -525,8 +533,8 @@ Output:
|
|||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
|
||||
// Should show the full command in the selector
|
||||
expect(selector).toHaveTextContent("wc -l *.go *.java")
|
||||
// Should show a command pattern
|
||||
expect(selector.textContent).toMatch(/wc/)
|
||||
|
||||
// The output should still be displayed in the code block
|
||||
expect(codeBlocks.length).toBeGreaterThan(1)
|
||||
|
|
@ -548,8 +556,8 @@ Output:
|
|||
const selector = screen.getByTestId("command-pattern-selector")
|
||||
expect(selector).toBeInTheDocument()
|
||||
|
||||
// Should show the full command in the selector
|
||||
expect(selector).toHaveTextContent("wc -l *.go *.java")
|
||||
// Should show a command pattern
|
||||
expect(selector.textContent).toMatch(/wc/)
|
||||
|
||||
// The output should still be displayed in the code block
|
||||
const codeBlocks = screen.getAllByTestId("code-block")
|
||||
|
|
|
|||
|
|
@ -26,8 +26,8 @@ const TestWrapper = ({ children }: { children: React.ReactNode }) => <TooltipPro
|
|||
|
||||
describe("CommandPatternSelector", () => {
|
||||
const defaultProps = {
|
||||
command: "npm install express",
|
||||
patterns: [
|
||||
{ pattern: "npm install express", description: "Full command" },
|
||||
{ pattern: "npm install", description: "Install npm packages" },
|
||||
{ pattern: "npm *", description: "Any npm command" },
|
||||
],
|
||||
|
|
@ -51,7 +51,7 @@ describe("CommandPatternSelector", () => {
|
|||
expect(screen.getByText("chat:commandExecution.manageCommands")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should show full command as first pattern when expanded", () => {
|
||||
it("should show patterns when expanded", () => {
|
||||
render(
|
||||
<TestWrapper>
|
||||
<CommandPatternSelector {...defaultProps} />
|
||||
|
|
@ -62,8 +62,9 @@ describe("CommandPatternSelector", () => {
|
|||
const expandButton = screen.getByRole("button")
|
||||
fireEvent.click(expandButton)
|
||||
|
||||
// Check that the full command is shown
|
||||
// Check that the patterns are shown
|
||||
expect(screen.getByText("npm install express")).toBeInTheDocument()
|
||||
expect(screen.getByText("- Full command")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should show extracted patterns when expanded", () => {
|
||||
|
|
@ -95,9 +96,9 @@ describe("CommandPatternSelector", () => {
|
|||
const expandButton = screen.getByRole("button")
|
||||
fireEvent.click(expandButton)
|
||||
|
||||
// Click on the full command pattern
|
||||
const fullCommandDiv = screen.getByText("npm install express").closest("div")
|
||||
fireEvent.click(fullCommandDiv!)
|
||||
// Click on a pattern
|
||||
const patternDiv = screen.getByText("npm install express").closest("div")
|
||||
fireEvent.click(patternDiv!)
|
||||
|
||||
// An input should appear
|
||||
const input = screen.getByDisplayValue("npm install express") as HTMLInputElement
|
||||
|
|
@ -168,9 +169,9 @@ describe("CommandPatternSelector", () => {
|
|||
const expandButton = screen.getByRole("button")
|
||||
fireEvent.click(expandButton)
|
||||
|
||||
// Find the full command pattern row and click allow
|
||||
const fullCommandPattern = screen.getByText("npm install express").closest(".ml-5")
|
||||
const allowButton = fullCommandPattern?.querySelector('button[aria-label*="addToAllowed"]')
|
||||
// Find a pattern row and click allow
|
||||
const patternRow = screen.getByText("npm install express").closest(".ml-5")
|
||||
const allowButton = patternRow?.querySelector('button[aria-label*="addToAllowed"]')
|
||||
fireEvent.click(allowButton!)
|
||||
|
||||
// Check that the callback was called with the pattern
|
||||
|
|
@ -194,9 +195,9 @@ describe("CommandPatternSelector", () => {
|
|||
const expandButton = screen.getByRole("button")
|
||||
fireEvent.click(expandButton)
|
||||
|
||||
// Find the full command pattern row and click deny
|
||||
const fullCommandPattern = screen.getByText("npm install express").closest(".ml-5")
|
||||
const denyButton = fullCommandPattern?.querySelector('button[aria-label*="addToDenied"]')
|
||||
// Find a pattern row and click deny
|
||||
const patternRow = screen.getByText("npm install express").closest(".ml-5")
|
||||
const denyButton = patternRow?.querySelector('button[aria-label*="addToDenied"]')
|
||||
fireEvent.click(denyButton!)
|
||||
|
||||
// Check that the callback was called with the pattern
|
||||
|
|
@ -220,11 +221,11 @@ describe("CommandPatternSelector", () => {
|
|||
const expandButton = screen.getByRole("button")
|
||||
fireEvent.click(expandButton)
|
||||
|
||||
// Click on the full command pattern to edit
|
||||
const fullCommandDiv = screen.getByText("npm install express").closest("div")
|
||||
fireEvent.click(fullCommandDiv!)
|
||||
// Click on a pattern to edit
|
||||
const patternDiv = screen.getByText("npm install express").closest("div")
|
||||
fireEvent.click(patternDiv!)
|
||||
|
||||
// Edit the command
|
||||
// Edit the pattern
|
||||
const input = screen.getByDisplayValue("npm install express") as HTMLInputElement
|
||||
fireEvent.change(input, { target: { value: "npm install react" } })
|
||||
|
||||
|
|
@ -254,11 +255,11 @@ describe("CommandPatternSelector", () => {
|
|||
const expandButton = screen.getByRole("button")
|
||||
fireEvent.click(expandButton)
|
||||
|
||||
// Click on the full command pattern to edit
|
||||
const fullCommandDiv = screen.getByText("npm install express").closest("div")
|
||||
fireEvent.click(fullCommandDiv!)
|
||||
// Click on a pattern to edit
|
||||
const patternDiv = screen.getByText("npm install express").closest("div")
|
||||
fireEvent.click(patternDiv!)
|
||||
|
||||
// Edit the command
|
||||
// Edit the pattern
|
||||
const input = screen.getByDisplayValue("npm install express") as HTMLInputElement
|
||||
fireEvent.change(input, { target: { value: "npm install react" } })
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue