Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 52 additions & 2 deletions src/integrations/terminal/BaseTerminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down
9 changes: 7 additions & 2 deletions src/integrations/terminal/TerminalRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
216 changes: 216 additions & 0 deletions src/integrations/terminal/__tests__/BaseTerminal.spec.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
2 changes: 1 addition & 1 deletion src/integrations/terminal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading