mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Merge b8690913fc into b867ec9145
This commit is contained in:
commit
4158feb618
4 changed files with 276 additions and 5 deletions
|
|
@ -40,11 +40,16 @@ export abstract class BaseTerminal implements RooTerminal {
|
|||
abstract runCommand(command: string, callbacks: RooTerminalCallbacks): RooTerminalProcessResultPromise
|
||||
|
||||
/**
|
||||
* Sets the active stream for this terminal and notifies the process
|
||||
* Sets the active stream for this terminal and notifies the process.
|
||||
* When eventCommand is provided, the stream is only set if it matches the process's command.
|
||||
* This prevents conda/other auto-activation commands from interfering with Roo's command execution.
|
||||
*
|
||||
* @param stream The stream to set, or undefined to clean up
|
||||
* @param pid The process ID from the shell execution event
|
||||
* @param eventCommand The command from the shell execution event, used to verify this is Roo's command
|
||||
* @throws Error if process is undefined when a stream is provided
|
||||
*/
|
||||
public setActiveStream(stream: AsyncIterable<string> | undefined, pid?: number): void {
|
||||
public setActiveStream(stream: AsyncIterable<string> | undefined, pid?: number, eventCommand?: string): void {
|
||||
if (stream) {
|
||||
if (!this.process) {
|
||||
this.running = false
|
||||
|
|
@ -56,6 +61,18 @@ export abstract class BaseTerminal implements RooTerminal {
|
|||
return
|
||||
}
|
||||
|
||||
// If eventCommand is provided, verify it matches the process's expected command
|
||||
// This prevents conda/pyenv/other auto-activation commands from hijacking the stream
|
||||
if (eventCommand !== undefined && this.process.command) {
|
||||
if (!BaseTerminal.commandsMatch(this.process.command, eventCommand)) {
|
||||
console.debug(
|
||||
`[Terminal ${this.provider}/${this.id}] Ignoring shell execution for non-matching command. ` +
|
||||
`Expected: "${this.process.command}", Got: "${eventCommand}"`,
|
||||
)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
this.running = true
|
||||
this.streamClosed = false
|
||||
this.process.emit("shell_execution_started", pid)
|
||||
|
|
@ -65,6 +82,39 @@ export abstract class BaseTerminal implements RooTerminal {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if two commands match. Uses flexible matching to handle cases where
|
||||
* the executed command might be modified (e.g., PowerShell counter workaround).
|
||||
*
|
||||
* @param expectedCommand The command Roo is trying to execute
|
||||
* @param actualCommand The command from the shell execution event
|
||||
* @returns true if the commands match
|
||||
*/
|
||||
public static commandsMatch(expectedCommand: string, actualCommand: string): boolean {
|
||||
// Normalize commands by trimming whitespace
|
||||
const expected = expectedCommand.trim()
|
||||
const actual = actualCommand.trim()
|
||||
|
||||
// Exact match (handles both empty string case and exact matches)
|
||||
if (expected === actual) {
|
||||
return true
|
||||
}
|
||||
|
||||
// If expected is empty but actual is not, they don't match
|
||||
// (this prevents empty process command from matching any event command)
|
||||
if (expected.length === 0) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check if expected is a prefix of actual (handles PowerShell counter workaround
|
||||
// which appends extra commands like ` ; "(Roo/PS Workaround: N)" > $null`)
|
||||
if (actual.startsWith(expected)) {
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* Handles shell execution completion for this terminal.
|
||||
* @param exitDetails The exit details of the shell execution
|
||||
|
|
|
|||
|
|
@ -51,14 +51,19 @@ export class TerminalRegistry {
|
|||
// Get a handle to the stream as early as possible:
|
||||
const stream = e.execution.read()
|
||||
const terminal = this.getTerminalByVSCETerminal(e.terminal)
|
||||
const eventCommand = e.execution?.commandLine?.value
|
||||
|
||||
console.info("[onDidStartTerminalShellExecution]", {
|
||||
command: e.execution?.commandLine?.value,
|
||||
command: eventCommand,
|
||||
terminalId: terminal?.id,
|
||||
processCommand: terminal?.process?.command,
|
||||
})
|
||||
|
||||
if (terminal) {
|
||||
terminal.setActiveStream(stream)
|
||||
// Pass the eventCommand to setActiveStream for verification.
|
||||
// This prevents conda/pyenv/other auto-activation commands from
|
||||
// hijacking the stream when the terminal first opens.
|
||||
terminal.setActiveStream(stream, undefined, eventCommand)
|
||||
terminal.busy = true // Mark terminal as busy when shell execution starts
|
||||
} else {
|
||||
console.error(
|
||||
|
|
|
|||
216
src/integrations/terminal/__tests__/BaseTerminal.spec.ts
Normal file
216
src/integrations/terminal/__tests__/BaseTerminal.spec.ts
Normal file
|
|
@ -0,0 +1,216 @@
|
|||
// npx vitest run src/integrations/terminal/__tests__/BaseTerminal.spec.ts
|
||||
|
||||
import { BaseTerminal } from "../BaseTerminal"
|
||||
import type { RooTerminalProcess } from "../types"
|
||||
|
||||
// Create a concrete implementation of BaseTerminal for testing
|
||||
class TestTerminal extends BaseTerminal {
|
||||
constructor(id: number = 1, cwd: string = "/test") {
|
||||
super("vscode", id, cwd)
|
||||
}
|
||||
|
||||
isClosed(): boolean {
|
||||
return false
|
||||
}
|
||||
|
||||
runCommand(): never {
|
||||
throw new Error("Not implemented")
|
||||
}
|
||||
}
|
||||
|
||||
// Create a mock process for testing
|
||||
function createMockProcess(command: string): RooTerminalProcess {
|
||||
const events: Record<string, ((...args: any[]) => void)[]> = {}
|
||||
|
||||
return {
|
||||
command,
|
||||
isHot: false,
|
||||
run: vi.fn(),
|
||||
continue: vi.fn(),
|
||||
abort: vi.fn(),
|
||||
hasUnretrievedOutput: vi.fn().mockReturnValue(false),
|
||||
getUnretrievedOutput: vi.fn().mockReturnValue(""),
|
||||
trimRetrievedOutput: vi.fn(),
|
||||
on: vi.fn((event: string, handler: (...args: any[]) => void) => {
|
||||
if (!events[event]) {
|
||||
events[event] = []
|
||||
}
|
||||
events[event].push(handler)
|
||||
}),
|
||||
once: vi.fn((event: string, handler: (...args: any[]) => void) => {
|
||||
if (!events[event]) {
|
||||
events[event] = []
|
||||
}
|
||||
events[event].push(handler)
|
||||
}),
|
||||
emit: vi.fn((event: string, ...args: any[]) => {
|
||||
const handlers = events[event] || []
|
||||
handlers.forEach((handler) => handler(...args))
|
||||
return true
|
||||
}),
|
||||
off: vi.fn(),
|
||||
removeListener: vi.fn(),
|
||||
removeAllListeners: vi.fn(),
|
||||
listeners: vi.fn(),
|
||||
rawListeners: vi.fn(),
|
||||
listenerCount: vi.fn(),
|
||||
prependListener: vi.fn(),
|
||||
prependOnceListener: vi.fn(),
|
||||
eventNames: vi.fn(),
|
||||
addListener: vi.fn(),
|
||||
setMaxListeners: vi.fn(),
|
||||
getMaxListeners: vi.fn(),
|
||||
} as unknown as RooTerminalProcess
|
||||
}
|
||||
|
||||
// Create a mock async iterable stream for testing
|
||||
async function* createMockStream(): AsyncGenerator<string> {
|
||||
yield "test output"
|
||||
}
|
||||
|
||||
describe("BaseTerminal", () => {
|
||||
describe("commandsMatch", () => {
|
||||
it("returns true for exact match", () => {
|
||||
expect(BaseTerminal.commandsMatch("npm test", "npm test")).toBe(true)
|
||||
})
|
||||
|
||||
it("returns true for exact match with whitespace trimming", () => {
|
||||
expect(BaseTerminal.commandsMatch(" npm test ", "npm test")).toBe(true)
|
||||
expect(BaseTerminal.commandsMatch("npm test", " npm test ")).toBe(true)
|
||||
})
|
||||
|
||||
it("returns true when actual command starts with expected command (PowerShell workaround)", () => {
|
||||
// PowerShell counter workaround appends extra commands
|
||||
const expected = "npm test"
|
||||
const actual = 'npm test ; "(Roo/PS Workaround: 1)" > $null'
|
||||
expect(BaseTerminal.commandsMatch(expected, actual)).toBe(true)
|
||||
})
|
||||
|
||||
it("returns true when actual command has trailing sleep command", () => {
|
||||
const expected = "npm test"
|
||||
const actual = "npm test ; start-sleep -milliseconds 50"
|
||||
expect(BaseTerminal.commandsMatch(expected, actual)).toBe(true)
|
||||
})
|
||||
|
||||
it("returns false for completely different commands", () => {
|
||||
expect(BaseTerminal.commandsMatch("npm test", "conda activate base")).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false when commands are similar but not matching", () => {
|
||||
expect(BaseTerminal.commandsMatch("npm install", "npm run build")).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false for reversed prefix (actual is prefix of expected)", () => {
|
||||
// This case should not match - the expected command should be a prefix of actual
|
||||
expect(BaseTerminal.commandsMatch("npm test --coverage", "npm test")).toBe(false)
|
||||
})
|
||||
|
||||
it("handles empty strings", () => {
|
||||
expect(BaseTerminal.commandsMatch("", "")).toBe(true)
|
||||
expect(BaseTerminal.commandsMatch("npm test", "")).toBe(false)
|
||||
expect(BaseTerminal.commandsMatch("", "npm test")).toBe(false)
|
||||
})
|
||||
|
||||
it("handles commands with special characters", () => {
|
||||
const expected = 'echo "hello world"'
|
||||
const actual = 'echo "hello world"'
|
||||
expect(BaseTerminal.commandsMatch(expected, actual)).toBe(true)
|
||||
})
|
||||
|
||||
it("handles conda activation commands correctly", () => {
|
||||
// Roo's command should not match conda's auto-activation
|
||||
expect(BaseTerminal.commandsMatch("npm test", "conda activate base")).toBe(false)
|
||||
expect(BaseTerminal.commandsMatch("npm test", "source activate myenv")).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("setActiveStream", () => {
|
||||
let terminal: TestTerminal
|
||||
let mockProcess: RooTerminalProcess
|
||||
|
||||
beforeEach(() => {
|
||||
terminal = new TestTerminal()
|
||||
mockProcess = createMockProcess("npm test")
|
||||
terminal.process = mockProcess
|
||||
})
|
||||
|
||||
it("sets stream when no eventCommand is provided (backwards compatibility)", () => {
|
||||
const stream = createMockStream()
|
||||
terminal.setActiveStream(stream)
|
||||
|
||||
expect(terminal.running).toBe(true)
|
||||
expect(mockProcess.emit).toHaveBeenCalledWith("shell_execution_started", undefined)
|
||||
expect(mockProcess.emit).toHaveBeenCalledWith("stream_available", stream)
|
||||
})
|
||||
|
||||
it("sets stream when eventCommand matches process command", () => {
|
||||
const stream = createMockStream()
|
||||
terminal.setActiveStream(stream, undefined, "npm test")
|
||||
|
||||
expect(terminal.running).toBe(true)
|
||||
expect(mockProcess.emit).toHaveBeenCalledWith("stream_available", stream)
|
||||
})
|
||||
|
||||
it("sets stream when eventCommand starts with process command (PowerShell case)", () => {
|
||||
const stream = createMockStream()
|
||||
terminal.setActiveStream(stream, undefined, 'npm test ; "(Roo/PS Workaround: 1)" > $null')
|
||||
|
||||
expect(terminal.running).toBe(true)
|
||||
expect(mockProcess.emit).toHaveBeenCalledWith("stream_available", stream)
|
||||
})
|
||||
|
||||
it("ignores stream when eventCommand does not match process command", () => {
|
||||
const stream = createMockStream()
|
||||
const consoleSpy = vi.spyOn(console, "debug").mockImplementation(() => {})
|
||||
|
||||
terminal.setActiveStream(stream, undefined, "conda activate base")
|
||||
|
||||
expect(terminal.running).toBe(false)
|
||||
expect(mockProcess.emit).not.toHaveBeenCalledWith("stream_available", expect.anything())
|
||||
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("Ignoring shell execution"))
|
||||
|
||||
consoleSpy.mockRestore()
|
||||
})
|
||||
|
||||
it("accepts stream when process has no command set (backward compatibility)", () => {
|
||||
mockProcess.command = ""
|
||||
const stream = createMockStream()
|
||||
|
||||
// When process.command is empty string, the command verification is skipped
|
||||
// for backward compatibility. The process should accept any stream.
|
||||
terminal.setActiveStream(stream, undefined, "conda activate base")
|
||||
|
||||
// Since process.command is empty, no verification is done and stream is accepted
|
||||
expect(terminal.running).toBe(true)
|
||||
expect(mockProcess.emit).toHaveBeenCalledWith("stream_available", stream)
|
||||
})
|
||||
|
||||
it("cleans up when stream is undefined", () => {
|
||||
terminal.setActiveStream(undefined)
|
||||
|
||||
expect(terminal.isStreamClosed).toBe(true)
|
||||
})
|
||||
|
||||
it("handles missing process gracefully", () => {
|
||||
terminal.process = undefined
|
||||
const stream = createMockStream()
|
||||
const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
|
||||
|
||||
terminal.setActiveStream(stream)
|
||||
|
||||
expect(terminal.running).toBe(false)
|
||||
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("process is undefined"))
|
||||
|
||||
consoleSpy.mockRestore()
|
||||
})
|
||||
|
||||
it("passes pid to shell_execution_started event", () => {
|
||||
const stream = createMockStream()
|
||||
const pid = 12345
|
||||
|
||||
terminal.setActiveStream(stream, pid, "npm test")
|
||||
|
||||
expect(mockProcess.emit).toHaveBeenCalledWith("shell_execution_started", pid)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -12,7 +12,7 @@ export interface RooTerminal {
|
|||
getCurrentWorkingDirectory(): string
|
||||
isClosed: () => boolean
|
||||
runCommand: (command: string, callbacks: RooTerminalCallbacks) => RooTerminalProcessResultPromise
|
||||
setActiveStream(stream: AsyncIterable<string> | undefined, pid?: number): void
|
||||
setActiveStream(stream: AsyncIterable<string> | undefined, pid?: number, eventCommand?: string): void
|
||||
shellExecutionComplete(exitDetails: ExitCodeDetails): void
|
||||
getProcessesWithOutput(): RooTerminalProcess[]
|
||||
getUnretrievedOutput(): string
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue