fix: prevent terminal sharing between Roo tasks

This change improves terminal management by tracking which task owns
each terminal and prioritizing terminal selection based on task
ownership.

Terminal selection now follows a priority order:

1. First try to find a terminal already assigned to this task with matching directory
2. If not found, try to find any available terminal with matching directory
3. If still not found, try to find any non-busy terminal
4. Only create a new terminal as a last resort

When a task ends, all terminals associated with it are released for use
by other tasks.

This prevents the issue where multiple Roo task instances could
inadvertently share terminals, which could lead to confusion when
terminal output from one task appears in another task's context.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
This commit is contained in:
Eric Wheeler 2025-03-05 19:19:30 -08:00
parent bf2ce7e1ee
commit bc8cfc919f
3 changed files with 45 additions and 13 deletions

View file

@ -27,7 +27,6 @@ import {
everyLineHasLineNumbers,
truncateOutput,
} from "../integrations/misc/extract-text"
import { TerminalManager } from "../integrations/terminal/TerminalManager"
import { ExitCodeDetails } from "../integrations/terminal/TerminalProcess"
import { TerminalRegistry } from "../integrations/terminal/TerminalRegistry"
import { UrlContentFetcher } from "../services/browser/UrlContentFetcher"
@ -111,7 +110,6 @@ export class Cline {
private rootTask: Cline | undefined = undefined
readonly apiConfiguration: ApiConfiguration
api: ApiHandler
private terminalManager: TerminalManager
private urlContentFetcher: UrlContentFetcher
private browserSession: BrowserSession
private didEditFile: boolean = false
@ -182,7 +180,6 @@ export class Cline {
this.taskNumber = -1
this.apiConfiguration = apiConfiguration
this.api = buildApiHandler(apiConfiguration)
this.terminalManager = new TerminalManager()
this.urlContentFetcher = new UrlContentFetcher(provider.context)
this.browserSession = new BrowserSession(provider.context)
this.customInstructions = customInstructions
@ -906,7 +903,9 @@ export class Cline {
this.abort = true
this.terminalManager.disposeAll()
// Release any terminals associated with this task
TerminalRegistry.releaseTerminalsForTask(this.taskId)
this.urlContentFetcher.closeBrowser()
this.browserSession.closeBrowser()
this.rooIgnoreController?.dispose()
@ -921,7 +920,7 @@ export class Cline {
// Tools
async executeCommandTool(command: string): Promise<[boolean, ToolResponse]> {
const terminalInfo = await this.terminalManager.getOrCreateTerminal(cwd)
const terminalInfo = await TerminalRegistry.getOrCreateTerminal(cwd, this.taskId)
terminalInfo.terminal.show() // weird visual bug when creating new terminals (even manually) where there's an empty space at the top.
const process = terminalInfo.runCommand(command)
@ -3475,17 +3474,13 @@ export class Cline {
const busyTerminals = TerminalRegistry.getTerminals(true)
const inactiveTerminals = TerminalRegistry.getTerminals(false)
// const allTerminals = [...busyTerminals, ...inactiveTerminals]
if (busyTerminals.length > 0 && this.didEditFile) {
// || this.didEditFile
await delay(300) // delay after saving file to let terminals catch up
}
// let terminalWasBusy = false
if (busyTerminals.length > 0) {
// wait for terminals to cool down
// terminalWasBusy = allTerminals.some((t) => this.terminalManager.isProcessHot(t.id))
await pWaitFor(() => busyTerminals.every((t) => !TerminalRegistry.isProcessHot(t.id)), {
interval: 100,
timeout: 15_000,

View file

@ -11,6 +11,7 @@ export class Terminal {
public running: boolean
private streamClosed: boolean
public process?: TerminalProcess
public taskId?: string
constructor(id: number, terminal: vscode.Terminal) {
this.id = id

View file

@ -176,15 +176,47 @@ export class TerminalRegistry {
this.disposables = []
}
/**
* Releases all terminals associated with a task
* @param taskId The task ID
*/
static releaseTerminalsForTask(taskId?: string): void {
if (!taskId) return
this.terminals.forEach((terminal) => {
if (terminal.taskId === taskId) {
terminal.taskId = undefined
}
})
}
/**
* Gets an existing terminal or creates a new one for the given working directory
* @param cwd The working directory path
* @param taskId Optional task ID to associate with the terminal
* @returns A Terminal instance
*/
static async getOrCreateTerminal(cwd: string): Promise<Terminal> {
static async getOrCreateTerminal(cwd: string, taskId?: string): Promise<Terminal> {
const terminals = this.getAllTerminals()
// Find available terminal from our pool first (created for this task)
// First priority: Find a terminal already assigned to this task with matching directory
if (taskId) {
const taskTerminal = terminals.find((t) => {
if (t.busy || t.taskId !== taskId) {
return false
}
const terminalCwd = t.terminal.shellIntegration?.cwd
if (!terminalCwd) {
return false
}
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd.fsPath)
})
if (taskTerminal) {
return taskTerminal
}
}
// Second priority: Find any available terminal with matching directory
const matchingTerminal = terminals.find((t) => {
if (t.busy) {
return false
@ -196,18 +228,22 @@ export class TerminalRegistry {
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd.fsPath)
})
if (matchingTerminal) {
matchingTerminal.taskId = taskId
return matchingTerminal
}
// If no matching terminal exists, try to find any non-busy terminal
// Third priority: Find any non-busy terminal
const availableTerminal = terminals.find((t) => !t.busy)
if (availableTerminal) {
// Navigate back to the desired directory
await availableTerminal.runCommand(`cd "${cwd}"`)
availableTerminal.taskId = taskId
return availableTerminal
}
// If all terminals are busy, create a new one
return this.createTerminal(cwd)
const newTerminal = this.createTerminal(cwd)
newTerminal.taskId = taskId
return newTerminal
}
}