fix: prevent duplicate terminal handler registration

Move terminal shell execution handlers from TerminalManager to TerminalRegistry to
permanently solve duplicate handler registration issue. Previously handlers were
registered per-task, now they are registered once at extension startup:

- Initialize handlers when extension loads
- Add safety check to prevent multiple initializations by throwing an
  error if initialize() is called more than once.
- Add cleanup on extension deactivation
- Remove handler registration from TerminalManager

Fixes: #1364
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
This commit is contained in:
Eric Wheeler 2025-03-05 17:58:41 -08:00
parent 3157bf29a8
commit 1973f87c6b
4 changed files with 70 additions and 53 deletions

View file

@ -18,6 +18,7 @@ import { CodeActionProvider } from "./core/CodeActionProvider"
import { DIFF_VIEW_URI_SCHEME } from "./integrations/editor/DiffViewProvider"
import { McpServerManager } from "./services/mcp/McpServerManager"
import { telemetryService } from "./services/telemetry/TelemetryService"
import { TerminalRegistry } from "./integrations/terminal/TerminalRegistry"
import { handleUri, registerCommands, registerCodeActions, createRooCodeAPI, registerTerminalActions } from "./activate"
@ -42,6 +43,8 @@ export function activate(context: vscode.ExtensionContext) {
// Initialize telemetry service after environment variables are loaded
telemetryService.initialize()
// Initialize terminal shell execution handlers
TerminalRegistry.initialize()
// Get default commands from configuration.
const defaultCommands = vscode.workspace.getConfiguration("roo-cline").get<string[]>("allowedCommands") || []
@ -108,4 +111,7 @@ export async function deactivate() {
// Clean up MCP server manager
await McpServerManager.cleanup(extensionContext)
telemetryService.shutdown()
// Clean up terminal handlers
TerminalRegistry.cleanup()
}

View file

@ -97,54 +97,6 @@ declare module "vscode" {
export class TerminalManager {
private terminalIds: Set<number> = new Set()
private disposables: vscode.Disposable[] = []
constructor() {
let startDisposable: vscode.Disposable | undefined
let endDisposable: vscode.Disposable | undefined
try {
// onDidStartTerminalShellExecution
startDisposable = (vscode.window as vscode.Window).onDidStartTerminalShellExecution?.(async (e) => {
// Get a handle to the stream as early as possible:
const stream = e?.execution.read()
const terminalInfo = TerminalRegistry.getTerminalByVSCETerminal(e.terminal)
if (terminalInfo) {
terminalInfo.setActiveStream(stream)
} else {
console.error("[TerminalManager] Stream failed, not registered for terminal")
}
console.info("[TerminalManager] Shell execution started:", {
hasExecution: !!e?.execution,
command: e?.execution?.commandLine?.value,
terminalId: terminalInfo?.id,
})
})
// onDidEndTerminalShellExecution
endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => {
const terminalInfo = TerminalRegistry.getTerminalByVSCETerminal(e.terminal)
const process = terminalInfo?.process
const exitDetails = process ? TerminalProcess.interpretExitCode(e?.exitCode) : { exitCode: e?.exitCode }
console.info("[TerminalManager] Shell execution ended:", {
...exitDetails,
})
// Signal completion to any waiting processes
if (terminalInfo && this.terminalIds.has(terminalInfo.id)) {
terminalInfo.shellExecutionComplete(exitDetails)
}
})
} catch (error) {
console.error("[TerminalManager] Error setting up shell execution handlers:", error)
}
if (startDisposable) {
this.disposables.push(startDisposable)
}
if (endDisposable) {
this.disposables.push(endDisposable)
}
}
runCommand(terminalInfo: Terminal, command: string): TerminalProcessResultPromise {
terminalInfo.busy = true
@ -214,11 +166,6 @@ export class TerminalManager {
}
disposeAll() {
// for (const info of this.terminals) {
// //info.terminal.dispose() // dont want to dispose terminals when task is aborted
// }
this.terminalIds.clear()
this.disposables.forEach((disposable) => disposable.dispose())
this.disposables = []
}
}

View file

@ -1,11 +1,65 @@
import * as vscode from "vscode"
import { Terminal } from "./Terminal"
import { TerminalProcess } from "./TerminalProcess"
// Although vscode.window.terminals provides a list of all open terminals, there's no way to know whether they're busy or not (exitStatus does not provide useful information for most commands). In order to prevent creating too many terminals, we need to keep track of terminals through the life of the extension, as well as session specific terminals for the life of a task (to get latest unretrieved output).
// Since we have promises keeping track of terminal processes, we get the added benefit of keep track of busy terminals even after a task is closed.
export class TerminalRegistry {
private static terminals: Terminal[] = []
private static nextTerminalId = 1
private static disposables: vscode.Disposable[] = []
private static isInitialized = false
static initialize() {
if (this.isInitialized) {
throw new Error("TerminalRegistry.initialize() should only be called once")
}
this.isInitialized = true
try {
// onDidStartTerminalShellExecution
const startDisposable = (vscode.window as vscode.Window).onDidStartTerminalShellExecution?.(async (e) => {
// Get a handle to the stream as early as possible:
const stream = e?.execution.read()
const terminalInfo = this.getTerminalByVSCETerminal(e.terminal)
if (terminalInfo) {
terminalInfo.setActiveStream(stream)
} else {
console.error("[TerminalRegistry] Stream failed, not registered for terminal")
}
console.info("[TerminalRegistry] Shell execution started:", {
hasExecution: !!e?.execution,
command: e?.execution?.commandLine?.value,
terminalId: terminalInfo?.id,
})
})
// onDidEndTerminalShellExecution
const endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => {
const terminalInfo = this.getTerminalByVSCETerminal(e.terminal)
const process = terminalInfo?.process
const exitDetails = process ? TerminalProcess.interpretExitCode(e?.exitCode) : { exitCode: e?.exitCode }
console.info("[TerminalRegistry] Shell execution ended:", {
...exitDetails,
})
// Signal completion to any waiting processes
if (terminalInfo) {
terminalInfo.shellExecutionComplete(exitDetails)
}
})
if (startDisposable) {
this.disposables.push(startDisposable)
}
if (endDisposable) {
this.disposables.push(endDisposable)
}
} catch (error) {
console.error("[TerminalRegistry] Error setting up shell execution handlers:", error)
}
}
static createTerminal(cwd?: string | vscode.Uri | undefined): Terminal {
const terminal = vscode.window.createTerminal({
@ -115,4 +169,9 @@ export class TerminalRegistry {
static getTerminals(busy: boolean): Terminal[] {
return this.getAllTerminals().filter((t) => t.busy === busy)
}
static cleanup() {
this.disposables.forEach((disposable) => disposable.dispose())
this.disposables = []
}
}

View file

@ -241,6 +241,11 @@ async function testTerminalCommand(
}
describe("TerminalProcess with Real Command Output", () => {
beforeAll(() => {
// Initialize TerminalRegistry event handlers once globally
TerminalRegistry.initialize()
})
beforeEach(() => {
// Reset the terminals array before each test
TerminalRegistry["terminals"] = []