diff --git a/src/integrations/terminal/BaseTerminal.ts b/src/integrations/terminal/BaseTerminal.ts index 121dc343136..7573ca9be6c 100644 --- a/src/integrations/terminal/BaseTerminal.ts +++ b/src/integrations/terminal/BaseTerminal.ts @@ -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 | undefined, pid?: number): void { + public setActiveStream(stream: AsyncIterable | 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 diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 6e0531bebe8..4e6ea569926 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -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( diff --git a/src/integrations/terminal/__tests__/BaseTerminal.spec.ts b/src/integrations/terminal/__tests__/BaseTerminal.spec.ts new file mode 100644 index 00000000000..299db35b1e6 --- /dev/null +++ b/src/integrations/terminal/__tests__/BaseTerminal.spec.ts @@ -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 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 { + 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) + }) + }) +}) diff --git a/src/integrations/terminal/types.ts b/src/integrations/terminal/types.ts index a0c5cde5d50..1a30bcc90b2 100644 --- a/src/integrations/terminal/types.ts +++ b/src/integrations/terminal/types.ts @@ -12,7 +12,7 @@ export interface RooTerminal { getCurrentWorkingDirectory(): string isClosed: () => boolean runCommand: (command: string, callbacks: RooTerminalCallbacks) => RooTerminalProcessResultPromise - setActiveStream(stream: AsyncIterable | undefined, pid?: number): void + setActiveStream(stream: AsyncIterable | undefined, pid?: number, eventCommand?: string): void shellExecutionComplete(exitDetails: ExitCodeDetails): void getProcessesWithOutput(): RooTerminalProcess[] getUnretrievedOutput(): string