From fb720e4204375272e9c88d5025d2e11ad49e0b92 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 2 Feb 2026 03:40:24 +0000 Subject: [PATCH 1/2] fix: prevent conda auto-activation from hijacking terminal command output When a terminal is created with conda auto-activation (or any shell initialization that runs commands), a race condition could occur where VSCode fires onDidStartTerminalShellExecution events for BOTH the conda activation AND Roo's command. The previous implementation blindly set the stream when the event fired, causing Roo to pick up the wrong stream and miss its own command's output. This fix: 1. Adds an optional eventCommand parameter to setActiveStream() 2. Passes the command from the shell execution event for verification 3. Only emits stream_available if the event command matches Roo's command 4. Uses flexible prefix matching to handle PowerShell workarounds Fixes #11148 --- src/integrations/terminal/BaseTerminal.ts | 54 ++++- src/integrations/terminal/TerminalRegistry.ts | 9 +- .../terminal/__tests__/BaseTerminal.spec.ts | 216 ++++++++++++++++++ src/integrations/terminal/types.ts | 2 +- 4 files changed, 276 insertions(+), 5 deletions(-) create mode 100644 src/integrations/terminal/__tests__/BaseTerminal.spec.ts diff --git a/src/integrations/terminal/BaseTerminal.ts b/src/integrations/terminal/BaseTerminal.ts index ee262549346..a266a58dba4 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.warn( + `[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..b9b59b5118f --- /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, "warn").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 From b8690913fc6f88a0f0fc10ed562580e598cf390b Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 2 Feb 2026 04:08:30 +0000 Subject: [PATCH 2/2] chore: reduce log level for expected command mismatch events --- src/integrations/terminal/BaseTerminal.ts | 2 +- src/integrations/terminal/__tests__/BaseTerminal.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integrations/terminal/BaseTerminal.ts b/src/integrations/terminal/BaseTerminal.ts index a266a58dba4..d368789bffd 100644 --- a/src/integrations/terminal/BaseTerminal.ts +++ b/src/integrations/terminal/BaseTerminal.ts @@ -65,7 +65,7 @@ export abstract class BaseTerminal implements RooTerminal { // 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.warn( + console.debug( `[Terminal ${this.provider}/${this.id}] Ignoring shell execution for non-matching command. ` + `Expected: "${this.process.command}", Got: "${eventCommand}"`, ) diff --git a/src/integrations/terminal/__tests__/BaseTerminal.spec.ts b/src/integrations/terminal/__tests__/BaseTerminal.spec.ts index b9b59b5118f..299db35b1e6 100644 --- a/src/integrations/terminal/__tests__/BaseTerminal.spec.ts +++ b/src/integrations/terminal/__tests__/BaseTerminal.spec.ts @@ -161,7 +161,7 @@ describe("BaseTerminal", () => { it("ignores stream when eventCommand does not match process command", () => { const stream = createMockStream() - const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + const consoleSpy = vi.spyOn(console, "debug").mockImplementation(() => {}) terminal.setActiveStream(stream, undefined, "conda activate base")