mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-05 02:41:26 +00:00
Merge b8690913fc into c4547d25c5
This commit is contained in:
commit
9586ed7f68
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
|
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 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
|
* @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 (stream) {
|
||||||
if (!this.process) {
|
if (!this.process) {
|
||||||
this.running = false
|
this.running = false
|
||||||
|
|
@ -56,6 +61,18 @@ export abstract class BaseTerminal implements RooTerminal {
|
||||||
return
|
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.running = true
|
||||||
this.streamClosed = false
|
this.streamClosed = false
|
||||||
this.process.emit("shell_execution_started", pid)
|
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.
|
* Handles shell execution completion for this terminal.
|
||||||
* @param exitDetails The exit details of the shell execution
|
* @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:
|
// Get a handle to the stream as early as possible:
|
||||||
const stream = e.execution.read()
|
const stream = e.execution.read()
|
||||||
const terminal = this.getTerminalByVSCETerminal(e.terminal)
|
const terminal = this.getTerminalByVSCETerminal(e.terminal)
|
||||||
|
const eventCommand = e.execution?.commandLine?.value
|
||||||
|
|
||||||
console.info("[onDidStartTerminalShellExecution]", {
|
console.info("[onDidStartTerminalShellExecution]", {
|
||||||
command: e.execution?.commandLine?.value,
|
command: eventCommand,
|
||||||
terminalId: terminal?.id,
|
terminalId: terminal?.id,
|
||||||
|
processCommand: terminal?.process?.command,
|
||||||
})
|
})
|
||||||
|
|
||||||
if (terminal) {
|
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
|
terminal.busy = true // Mark terminal as busy when shell execution starts
|
||||||
} else {
|
} else {
|
||||||
console.error(
|
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
|
getCurrentWorkingDirectory(): string
|
||||||
isClosed: () => boolean
|
isClosed: () => boolean
|
||||||
runCommand: (command: string, callbacks: RooTerminalCallbacks) => RooTerminalProcessResultPromise
|
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
|
shellExecutionComplete(exitDetails: ExitCodeDetails): void
|
||||||
getProcessesWithOutput(): RooTerminalProcess[]
|
getProcessesWithOutput(): RooTerminalProcess[]
|
||||||
getUnretrievedOutput(): string
|
getUnretrievedOutput(): string
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue