From 80a365ae78c250102d59527a628d91268303c015 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sun, 3 Aug 2025 15:52:12 +0000 Subject: [PATCH] fix: persist conda environment state across execute_command calls - Add activeEnvironment property to track conda environments in terminals - Update TerminalRegistry to prefer terminals with matching conda environments - Track conda activate/deactivate commands and update terminal state - Add comprehensive tests for conda environment persistence Fixes #6628 --- src/integrations/terminal/BaseTerminal.ts | 2 + .../terminal/ExecaTerminalProcess.ts | 24 ++ src/integrations/terminal/TerminalProcess.ts | 23 ++ src/integrations/terminal/TerminalRegistry.ts | 57 +++- .../CondaEnvironmentPersistence.spec.ts | 282 ++++++++++++++++++ src/integrations/terminal/types.ts | 1 + 6 files changed, 384 insertions(+), 5 deletions(-) create mode 100644 src/integrations/terminal/__tests__/CondaEnvironmentPersistence.spec.ts diff --git a/src/integrations/terminal/BaseTerminal.ts b/src/integrations/terminal/BaseTerminal.ts index a79d417b07..738b4449b4 100644 --- a/src/integrations/terminal/BaseTerminal.ts +++ b/src/integrations/terminal/BaseTerminal.ts @@ -22,6 +22,7 @@ export abstract class BaseTerminal implements RooTerminal { public taskId?: string public process?: RooTerminalProcess public completedProcesses: RooTerminalProcess[] = [] + public activeEnvironment?: string constructor(provider: RooTerminalProvider, id: number, cwd: string) { this.provider = provider @@ -30,6 +31,7 @@ export abstract class BaseTerminal implements RooTerminal { this.busy = false this.running = false this.streamClosed = false + this.activeEnvironment = undefined } public getCurrentWorkingDirectory(): string { diff --git a/src/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index 2f8ebfa7a8..567b8fd44f 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -4,6 +4,7 @@ import process from "process" import type { RooTerminal } from "./types" import { BaseTerminalProcess } from "./BaseTerminalProcess" +import { TerminalRegistry } from "./TerminalRegistry" export class ExecaTerminalProcess extends BaseTerminalProcess { private terminalRef: WeakRef @@ -35,6 +36,10 @@ export class ExecaTerminalProcess extends BaseTerminalProcess { public override async run(command: string) { this.command = command + // Check if this is a conda activate/deactivate command + const condaActivateMatch = command.match(/^\s*conda\s+activate\s+(.+)$/i) + const condaDeactivateMatch = command.match(/^\s*conda\s+deactivate\s*$/i) + try { this.isHot = true @@ -145,6 +150,25 @@ export class ExecaTerminalProcess extends BaseTerminalProcess { this.terminal.setActiveStream(undefined) this.emitRemainingBufferIfListening() this.stopHotTimer() + + // Update the terminal's active environment based on the command and output + if (condaActivateMatch) { + // Extract the environment name from the command + const envName = condaActivateMatch[1].trim() + // Check if activation was successful by looking for common error indicators in output + const cleanOutput = this.fullOutput.toLowerCase() + if (!cleanOutput.includes("error") && !cleanOutput.includes("not found")) { + this.terminal.activeEnvironment = envName + TerminalRegistry.setLastActiveEnvironment(envName) + console.log(`[ExecaTerminalProcess] Conda environment activated: ${envName}`) + } + } else if (condaDeactivateMatch) { + // Clear the active environment on deactivation + this.terminal.activeEnvironment = undefined + TerminalRegistry.setLastActiveEnvironment(undefined) + console.log("[ExecaTerminalProcess] Conda environment deactivated") + } + this.emit("completed", this.fullOutput) this.emit("continue") this.subprocess = undefined diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index eb0424fe8d..57a44ef913 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -13,6 +13,7 @@ import { inspect } from "util" import type { ExitCodeDetails } from "./types" import { BaseTerminalProcess } from "./BaseTerminalProcess" import { Terminal } from "./Terminal" +import { TerminalRegistry } from "./TerminalRegistry" export class TerminalProcess extends BaseTerminalProcess { private terminalRef: WeakRef @@ -47,6 +48,10 @@ export class TerminalProcess extends BaseTerminalProcess { public override async run(command: string) { this.command = command + // Check if this is a conda activate/deactivate command + const condaActivateMatch = command.match(/^\s*conda\s+activate\s+(.+)$/i) + const condaDeactivateMatch = command.match(/^\s*conda\s+deactivate\s*$/i) + const terminal = this.terminal.terminal const isShellIntegrationAvailable = terminal.shellIntegration && terminal.shellIntegration.executeCommand @@ -248,6 +253,24 @@ export class TerminalProcess extends BaseTerminalProcess { this.fullOutput = match } + // Update the terminal's active environment based on the command and output + if (condaActivateMatch) { + // Extract the environment name from the command + const envName = condaActivateMatch[1].trim() + // Check if activation was successful by looking for common success indicators in output + const cleanOutput = this.removeEscapeSequences(this.fullOutput).toLowerCase() + if (!cleanOutput.includes("error") && !cleanOutput.includes("not found")) { + this.terminal.activeEnvironment = envName + TerminalRegistry.setLastActiveEnvironment(envName) + console.log(`[TerminalProcess] Conda environment activated: ${envName}`) + } + } else if (condaDeactivateMatch) { + // Clear the active environment on deactivation + this.terminal.activeEnvironment = undefined + TerminalRegistry.setLastActiveEnvironment(undefined) + console.log("[TerminalProcess] Conda environment deactivated") + } + // For now we don't want this delaying requests since we don't send // diagnostics automatically anymore (previous: "even though the // command is finished, we still want to consider it 'hot' in case diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index af334611c3..a5419f6201 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -22,6 +22,7 @@ export class TerminalRegistry { private static nextTerminalId = 1 private static disposables: vscode.Disposable[] = [] private static isInitialized = false + private static lastActiveEnvironment?: string public static initialize() { if (this.isInitialized) { @@ -160,7 +161,7 @@ export class TerminalRegistry { let terminal: RooTerminal | undefined // First priority: Find a terminal already assigned to this task with - // matching directory. + // matching directory and environment. if (taskId) { terminal = terminals.find((t) => { if (t.busy || t.taskId !== taskId || t.provider !== provider) { @@ -173,11 +174,34 @@ export class TerminalRegistry { return false } - return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd) + // Prefer terminals with matching environment + const envMatch = this.lastActiveEnvironment ? t.activeEnvironment === this.lastActiveEnvironment : true + + return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd) && envMatch }) } - // Second priority: Find any available terminal with matching directory. + // Second priority: Find any available terminal with matching directory and environment. + if (!terminal) { + terminal = terminals.find((t) => { + if (t.busy || t.provider !== provider) { + return false + } + + const terminalCwd = t.getCurrentWorkingDirectory() + + if (!terminalCwd) { + return false + } + + // Prefer terminals with matching environment + const envMatch = this.lastActiveEnvironment ? t.activeEnvironment === this.lastActiveEnvironment : true + + return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd) && envMatch + }) + } + + // Third priority: Find any available terminal with matching directory (ignore environment). if (!terminal) { terminal = terminals.find((t) => { if (t.busy || t.provider !== provider) { @@ -194,8 +218,14 @@ export class TerminalRegistry { }) } - // Third priority: Find any non-busy terminal (only if directory is not - // required). + // Fourth priority: Find any non-busy terminal with matching environment (only if directory is not required). + if (!terminal && !requiredCwd && this.lastActiveEnvironment) { + terminal = terminals.find( + (t) => !t.busy && t.provider === provider && t.activeEnvironment === this.lastActiveEnvironment, + ) + } + + // Fifth priority: Find any non-busy terminal (only if directory is not required). if (!terminal && !requiredCwd) { terminal = terminals.find((t) => !t.busy && t.provider === provider) } @@ -333,4 +363,21 @@ export class TerminalRegistry { ShellIntegrationManager.zshCleanupTmpDir(id) this.terminals = this.terminals.filter((t) => t.id !== id) } + + /** + * Updates the last active conda environment + * @param environment The environment name or undefined to clear + */ + public static setLastActiveEnvironment(environment?: string): void { + this.lastActiveEnvironment = environment + console.log(`[TerminalRegistry] Last active environment updated: ${environment || "none"}`) + } + + /** + * Gets the last active conda environment + * @returns The last active environment name or undefined + */ + public static getLastActiveEnvironment(): string | undefined { + return this.lastActiveEnvironment + } } diff --git a/src/integrations/terminal/__tests__/CondaEnvironmentPersistence.spec.ts b/src/integrations/terminal/__tests__/CondaEnvironmentPersistence.spec.ts new file mode 100644 index 0000000000..658862fc10 --- /dev/null +++ b/src/integrations/terminal/__tests__/CondaEnvironmentPersistence.spec.ts @@ -0,0 +1,282 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import * as vscode from "vscode" +import { TerminalRegistry } from "../TerminalRegistry" +import { Terminal } from "../Terminal" +import { TerminalProcess } from "../TerminalProcess" +import { BaseTerminal } from "../BaseTerminal" + +// Mock vscode module +vi.mock("vscode", () => ({ + window: { + createTerminal: vi.fn(), + onDidCloseTerminal: vi.fn(), + onDidStartTerminalShellExecution: vi.fn(), + onDidEndTerminalShellExecution: vi.fn(), + }, + workspace: { + getConfiguration: vi.fn().mockReturnValue({ + get: vi.fn().mockReturnValue(0), + }), + }, + Uri: { + file: vi.fn((path: string) => ({ fsPath: path })), + }, + ThemeIcon: vi.fn(), +})) + +describe("Conda Environment Persistence", () => { + let mockTerminal: any + let mockShellIntegration: any + let isInitialized = false + + beforeEach(() => { + // Reset the terminal registry + vi.clearAllMocks() + + // Mock shell integration + mockShellIntegration = { + executeCommand: vi.fn(), + cwd: vscode.Uri.file("/test/path"), + } + + // Mock VSCode terminal + mockTerminal = { + shellIntegration: mockShellIntegration, + show: vi.fn(), + sendText: vi.fn(), + exitStatus: undefined, + } + + // Mock createTerminal to return our mock terminal + vi.mocked(vscode.window.createTerminal).mockReturnValue(mockTerminal) + + // Mock event handlers to return disposables + vi.mocked(vscode.window.onDidCloseTerminal).mockReturnValue({ dispose: vi.fn() }) + vi.mocked(vscode.window.onDidStartTerminalShellExecution).mockReturnValue({ dispose: vi.fn() }) + vi.mocked(vscode.window.onDidEndTerminalShellExecution).mockReturnValue({ dispose: vi.fn() }) + + // Initialize TerminalRegistry only once + if (!isInitialized) { + try { + TerminalRegistry.initialize() + isInitialized = true + } catch (e) { + // Already initialized + } + } + + // Clear the last active environment for each test + TerminalRegistry.setLastActiveEnvironment(undefined) + }) + + afterEach(() => { + // Don't call cleanup in tests as it affects the singleton state + }) + + describe("Terminal Environment Tracking", () => { + it("should track conda environment activation in Terminal", async () => { + const terminal = new Terminal(1, mockTerminal, "/test/path") + + // Initially, no environment should be active + expect(terminal.activeEnvironment).toBeUndefined() + + // Simulate conda activate command + const process = new TerminalProcess(terminal) + process.command = "conda activate myenv" + + // Mock successful activation (no error in output) + process["fullOutput"] = "Activating environment at /path/to/myenv" + + // Simulate the conda activation detection logic + const condaActivateMatch = process.command.match(/^\s*conda\s+activate\s+(.+)$/i) + if (condaActivateMatch) { + const envName = condaActivateMatch[1].trim() + const cleanOutput = process["fullOutput"].toLowerCase() + if (!cleanOutput.includes("error") && !cleanOutput.includes("not found")) { + terminal.activeEnvironment = envName + } + } + + expect(terminal.activeEnvironment).toBe("myenv") + }) + + it("should clear environment on conda deactivate", async () => { + const terminal = new Terminal(1, mockTerminal, "/test/path") + + // Set an active environment + terminal.activeEnvironment = "myenv" + expect(terminal.activeEnvironment).toBe("myenv") + + // Simulate conda deactivate command + const process = new TerminalProcess(terminal) + process.command = "conda deactivate" + + // Simulate the conda deactivation detection logic + const condaDeactivateMatch = process.command.match(/^\s*conda\s+deactivate\s*$/i) + if (condaDeactivateMatch) { + terminal.activeEnvironment = undefined + } + + expect(terminal.activeEnvironment).toBeUndefined() + }) + + it("should not activate environment if command fails", async () => { + const terminal = new Terminal(1, mockTerminal, "/test/path") + + // Simulate conda activate command with error + const process = new TerminalProcess(terminal) + process.command = "conda activate nonexistent" + + // Mock error in output + process["fullOutput"] = "EnvironmentNotFoundError: Could not find conda environment: nonexistent" + + // Simulate the conda activation detection logic + const condaActivateMatch = process.command.match(/^\s*conda\s+activate\s+(.+)$/i) + if (condaActivateMatch) { + const envName = condaActivateMatch[1].trim() + const cleanOutput = process["fullOutput"].toLowerCase() + if (!cleanOutput.includes("error") && !cleanOutput.includes("not found")) { + terminal.activeEnvironment = envName + } + } + + // Environment should not be activated due to error + expect(terminal.activeEnvironment).toBeUndefined() + }) + }) + + describe("TerminalRegistry Environment Persistence", () => { + it("should track last active environment in TerminalRegistry", () => { + // Initially no environment + expect(TerminalRegistry.getLastActiveEnvironment()).toBeUndefined() + + // Set an environment + TerminalRegistry.setLastActiveEnvironment("myenv") + expect(TerminalRegistry.getLastActiveEnvironment()).toBe("myenv") + + // Clear environment + TerminalRegistry.setLastActiveEnvironment(undefined) + expect(TerminalRegistry.getLastActiveEnvironment()).toBeUndefined() + }) + + it("should prefer terminals with matching conda environment", async () => { + // Get the current terminals count to track new ones + const initialTerminals = TerminalRegistry.getTerminals(false) + + // Create two terminals with different environments + const terminal1 = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + terminal1.activeEnvironment = "env1" + terminal1.busy = false + + const terminal2 = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + terminal2.activeEnvironment = "env2" + terminal2.busy = false + + // Set last active environment to env2 + TerminalRegistry.setLastActiveEnvironment("env2") + + // Request a terminal - should prefer terminal2 + const selectedTerminal = await TerminalRegistry.getOrCreateTerminal( + "/test/path", + false, + undefined, + "vscode", + ) + expect(selectedTerminal.activeEnvironment).toBe("env2") + }) + + it("should create new terminal if no matching environment found", async () => { + // Create a terminal with env1 + const terminal1 = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + terminal1.activeEnvironment = "env1" + terminal1.busy = false + + // Set last active environment to env2 (no matching terminal) + TerminalRegistry.setLastActiveEnvironment("env2") + + // Count terminals before request + const terminalsBefore = TerminalRegistry.getTerminals(false).length + + // Request a terminal with required cwd - should create new one since env doesn't match + const selectedTerminal = await TerminalRegistry.getOrCreateTerminal( + "/different/path", + true, + undefined, + "vscode", + ) + + // Count terminals after request + const terminalsAfter = TerminalRegistry.getTerminals(false).length + + // Should have created a new terminal + expect(terminalsAfter).toBe(terminalsBefore + 1) + expect(selectedTerminal).not.toBe(terminal1) + }) + + it("should fallback to any available terminal if environment doesn't match and cwd not required", async () => { + // Get all existing terminals and mark them as busy + const existingTerminals = TerminalRegistry.getTerminals(false) + existingTerminals.forEach((t) => (t.busy = true)) + + // Create a terminal with env1 + const terminal1 = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + terminal1.activeEnvironment = "env1" + terminal1.busy = false + + // Clear last active environment (no preference) + TerminalRegistry.setLastActiveEnvironment(undefined) + + // Request a terminal without required cwd - should use terminal1 since no env preference + const selectedTerminal = await TerminalRegistry.getOrCreateTerminal( + "/test/path", + false, + undefined, + "vscode", + ) + expect(selectedTerminal.id).toBe(terminal1.id) + }) + }) + + describe("Integration with execute_command", () => { + it("should maintain conda environment across multiple commands in same task", async () => { + const taskId = "test-task-123" + + // First command: activate conda environment + const terminal1 = (await TerminalRegistry.getOrCreateTerminal( + "/test/path", + false, + taskId, + "vscode", + )) as Terminal + terminal1.activeEnvironment = "myenv" + TerminalRegistry.setLastActiveEnvironment("myenv") + terminal1.busy = false + + // Second command: should reuse the same terminal with active environment + const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, taskId, "vscode") + + expect(terminal2).toBe(terminal1) + expect(terminal2.activeEnvironment).toBe("myenv") + }) + + it("should prefer environment-matched terminal even across different tasks", async () => { + // Create terminal with env for task1 + const terminal1 = (await TerminalRegistry.getOrCreateTerminal( + "/test/path", + false, + "task1", + "vscode", + )) as Terminal + terminal1.activeEnvironment = "myenv" + TerminalRegistry.setLastActiveEnvironment("myenv") + terminal1.busy = false + terminal1.taskId = undefined // Release from task1 + + // Request terminal for task2 - should still prefer terminal1 due to environment match + const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task2", "vscode") + + expect(terminal2).toBe(terminal1) + expect(terminal2.activeEnvironment).toBe("myenv") + }) + }) +}) diff --git a/src/integrations/terminal/types.ts b/src/integrations/terminal/types.ts index 65d521ba6e..308d5c6a56 100644 --- a/src/integrations/terminal/types.ts +++ b/src/integrations/terminal/types.ts @@ -9,6 +9,7 @@ export interface RooTerminal { running: boolean taskId?: string process?: RooTerminalProcess + activeEnvironment?: string getCurrentWorkingDirectory(): string isClosed: () => boolean runCommand: (command: string, callbacks: RooTerminalCallbacks) => RooTerminalProcessResultPromise