diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index d42eba082cb3..ce23893d9a02 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -485,10 +485,13 @@ export class DiffViewProvider { return new Promise((resolve, reject) => { const fileName = path.basename(uri.fsPath) const fileExists = this.editType === "modify" - const DIFF_EDITOR_TIMEOUT = 10_000 // ms + const DIFF_EDITOR_TIMEOUT = 15_000 // Increased from 10s to 15s to accommodate slower systems + const MAX_RETRIES = 2 // Allow up to 2 retries on timeout + let currentRetry = 0 let timeoutId: NodeJS.Timeout | undefined - const disposables: vscode.Disposable[] = [] + let disposables: vscode.Disposable[] = [] + let isResolved = false const cleanup = () => { if (timeoutId) { @@ -499,77 +502,122 @@ export class DiffViewProvider { disposables.length = 0 } - // Set timeout for the entire operation - timeoutId = setTimeout(() => { - cleanup() - reject( - new Error( - `Failed to open diff editor for ${uri.fsPath} within ${DIFF_EDITOR_TIMEOUT / 1000} seconds. The editor may be blocked or VS Code may be unresponsive.`, - ), + const attemptOpenDiffEditor = () => { + // Set timeout for the current attempt + timeoutId = setTimeout(() => { + if (!isResolved) { + cleanup() + + // Try retrying if we haven't exceeded MAX_RETRIES + if (currentRetry < MAX_RETRIES) { + currentRetry++ + console.warn( + `[DiffViewProvider] Diff editor timeout for ${uri.fsPath}, retrying... (attempt ${currentRetry + 1}/${MAX_RETRIES + 1})`, + ) + // Reset and retry + disposables = [] + attemptOpenDiffEditor() + } else { + // All retries exhausted + isResolved = true + reject( + new Error( + `Failed to open diff editor for ${uri.fsPath} after ${MAX_RETRIES + 1} attempts (${DIFF_EDITOR_TIMEOUT / 1000}s timeout each). The editor may be blocked or VS Code may be unresponsive.`, + ), + ) + } + } + }, DIFF_EDITOR_TIMEOUT) + + // Listen for document open events - more efficient than scanning all tabs + disposables.push( + vscode.workspace.onDidOpenTextDocument(async (document) => { + if (isResolved) return + + // Only match file:// scheme documents to avoid git diffs + if (document.uri.scheme === "file" && arePathsEqual(document.uri.fsPath, uri.fsPath)) { + // Wait a tick for the editor to be available + await new Promise((r) => setTimeout(r, 50)) // Slightly longer wait + + // Find the editor for this document + const editor = vscode.window.visibleTextEditors.find( + (e) => + e.document.uri.scheme === "file" && + arePathsEqual(e.document.uri.fsPath, uri.fsPath), + ) + + if (editor && !isResolved) { + isResolved = true + cleanup() + resolve(editor) + } + } + }), ) - }, DIFF_EDITOR_TIMEOUT) - - // Listen for document open events - more efficient than scanning all tabs - disposables.push( - vscode.workspace.onDidOpenTextDocument(async (document) => { - // Only match file:// scheme documents to avoid git diffs - if (document.uri.scheme === "file" && arePathsEqual(document.uri.fsPath, uri.fsPath)) { - // Wait a tick for the editor to be available - await new Promise((r) => setTimeout(r, 0)) - - // Find the editor for this document - const editor = vscode.window.visibleTextEditors.find( - (e) => e.document.uri.scheme === "file" && arePathsEqual(e.document.uri.fsPath, uri.fsPath), - ) - if (editor) { + // Also listen for visible editor changes as a fallback + disposables.push( + vscode.window.onDidChangeVisibleTextEditors((editors) => { + if (isResolved) return + + const editor = editors.find((e) => { + const isFileScheme = e.document.uri.scheme === "file" + const pathMatches = arePathsEqual(e.document.uri.fsPath, uri.fsPath) + return isFileScheme && pathMatches + }) + if (editor && !isResolved) { + isResolved = true cleanup() resolve(editor) } - } - }), - ) - - // Also listen for visible editor changes as a fallback - disposables.push( - vscode.window.onDidChangeVisibleTextEditors((editors) => { - const editor = editors.find((e) => { - const isFileScheme = e.document.uri.scheme === "file" - const pathMatches = arePathsEqual(e.document.uri.fsPath, uri.fsPath) - return isFileScheme && pathMatches - }) - if (editor) { - cleanup() - resolve(editor) - } - }), - ) + }), + ) - // Pre-open the file as a text document to ensure it doesn't open in preview mode - // This fixes issues with files that have custom editor associations (like markdown preview) - vscode.window - .showTextDocument(uri, { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true }) - .then(() => { - // Execute the diff command after ensuring the file is open as text - return vscode.commands.executeCommand( - "vscode.diff", - vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${fileName}`).with({ - query: Buffer.from(this.originalContent ?? "").toString("base64"), - }), - uri, - `${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`, - { preserveFocus: true }, - ) + // Pre-open the file as a text document to ensure it doesn't open in preview mode + // This fixes issues with files that have custom editor associations (like markdown preview) + // Use setImmediate to avoid blocking other operations + setImmediate(() => { + if (isResolved) return + + vscode.window + .showTextDocument(uri, { + preview: false, + viewColumn: vscode.ViewColumn.Active, + preserveFocus: true, + }) + .then(() => { + if (isResolved) return + + // Execute the diff command after ensuring the file is open as text + return vscode.commands.executeCommand( + "vscode.diff", + vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${fileName}`).with({ + query: Buffer.from(this.originalContent ?? "").toString("base64"), + }), + uri, + `${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`, + { preserveFocus: true }, + ) + }) + .then( + () => { + // Command executed successfully, now wait for the editor to appear + }, + (err: any) => { + if (!isResolved) { + isResolved = true + cleanup() + reject( + new Error(`Failed to execute diff command for ${uri.fsPath}: ${err.message}`), + ) + } + }, + ) }) - .then( - () => { - // Command executed successfully, now wait for the editor to appear - }, - (err: any) => { - cleanup() - reject(new Error(`Failed to execute diff command for ${uri.fsPath}: ${err.message}`)) - }, - ) + } + + // Start the first attempt + attemptOpenDiffEditor() }) } diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 6e0531bebe8e..983abd2aeffa 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -47,25 +47,35 @@ export class TerminalRegistry { try { const startDisposable = vscode.window.onDidStartTerminalShellExecution?.( - async (e: vscode.TerminalShellExecutionStartEvent) => { - // Get a handle to the stream as early as possible: - const stream = e.execution.read() - const terminal = this.getTerminalByVSCETerminal(e.terminal) - - console.info("[onDidStartTerminalShellExecution]", { - command: e.execution?.commandLine?.value, - terminalId: terminal?.id, + (e: vscode.TerminalShellExecutionStartEvent) => { + // Process the event asynchronously to avoid blocking + setImmediate(async () => { + try { + // Get a handle to the stream as early as possible: + const stream = e.execution.read() + const terminal = this.getTerminalByVSCETerminal(e.terminal) + + console.info("[onDidStartTerminalShellExecution]", { + command: e.execution?.commandLine?.value, + terminalId: terminal?.id, + }) + + if (terminal) { + terminal.setActiveStream(stream) + terminal.busy = true // Mark terminal as busy when shell execution starts + } else { + // Only log as debug to avoid spamming logs for non-Roo terminals + console.debug( + "[onDidStartTerminalShellExecution] Shell execution started from non-Roo terminal", + ) + } + } catch (error) { + console.error( + "[onDidStartTerminalShellExecution] Error handling shell execution start:", + error, + ) + } }) - - if (terminal) { - terminal.setActiveStream(stream) - terminal.busy = true // Mark terminal as busy when shell execution starts - } else { - console.error( - "[onDidStartTerminalShellExecution] Shell execution started, but not from a Roo-registered terminal:", - e, - ) - } }, ) @@ -74,48 +84,54 @@ export class TerminalRegistry { } const endDisposable = vscode.window.onDidEndTerminalShellExecution?.( - async (e: vscode.TerminalShellExecutionEndEvent) => { - const terminal = this.getTerminalByVSCETerminal(e.terminal) - const process = terminal?.process - const exitDetails = TerminalProcess.interpretExitCode(e.exitCode) - - console.info("[onDidEndTerminalShellExecution]", { - command: e.execution?.commandLine?.value, - terminalId: terminal?.id, - ...exitDetails, + (e: vscode.TerminalShellExecutionEndEvent) => { + // Process the event asynchronously to avoid blocking + setImmediate(async () => { + try { + const terminal = this.getTerminalByVSCETerminal(e.terminal) + const process = terminal?.process + const exitDetails = TerminalProcess.interpretExitCode(e.exitCode) + + console.info("[onDidEndTerminalShellExecution]", { + command: e.execution?.commandLine?.value, + terminalId: terminal?.id, + ...exitDetails, + }) + + if (!terminal) { + // Only log as debug to avoid spamming logs for non-Roo terminals + console.debug( + "[onDidEndTerminalShellExecution] Shell execution ended from non-Roo terminal", + ) + return + } + + if (!terminal.running) { + console.warn( + "[TerminalRegistry] Shell execution end event received, but process is not running for terminal:", + { terminalId: terminal?.id, command: process?.command, exitCode: e.exitCode }, + ) + + terminal.busy = false + return + } + + if (!process) { + console.error( + "[TerminalRegistry] Shell execution end event received on running terminal, but process is undefined:", + { terminalId: terminal.id, exitCode: e.exitCode }, + ) + + return + } + + // Signal completion to any waiting processes. + terminal.shellExecutionComplete(exitDetails) + terminal.busy = false // Mark terminal as not busy when shell execution ends + } catch (error) { + console.error("[onDidEndTerminalShellExecution] Error handling shell execution end:", error) + } }) - - if (!terminal) { - console.error( - "[onDidEndTerminalShellExecution] Shell execution ended, but not from a Roo-registered terminal:", - e, - ) - - return - } - - if (!terminal.running) { - console.error( - "[TerminalRegistry] Shell execution end event received, but process is not running for terminal:", - { terminalId: terminal?.id, command: process?.command, exitCode: e.exitCode }, - ) - - terminal.busy = false - return - } - - if (!process) { - console.error( - "[TerminalRegistry] Shell execution end event received on running terminal, but process is undefined:", - { terminalId: terminal.id, exitCode: e.exitCode }, - ) - - return - } - - // Signal completion to any waiting processes. - terminal.shellExecutionComplete(exitDetails) - terminal.busy = false // Mark terminal as not busy when shell execution ends }, ) @@ -161,6 +177,7 @@ export class TerminalRegistry { // matching directory. if (taskId) { terminal = terminals.find((t) => { + // Only reuse terminals that belong to this specific task if (t.busy || t.taskId !== taskId || t.provider !== provider) { return false } @@ -175,8 +192,27 @@ export class TerminalRegistry { }) } - // Second priority: Find any available terminal with matching directory. - if (!terminal) { + // Second priority: Find any available terminal with matching directory + // BUT only if it has no task assignment (to avoid cross-task interference) + if (!terminal && taskId) { + terminal = terminals.find((t) => { + // Only use unassigned terminals to prevent cross-task issues + if (t.busy || t.taskId !== undefined || t.provider !== provider) { + return false + } + + const terminalCwd = t.getCurrentWorkingDirectory() + + if (!terminalCwd) { + return false + } + + return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd) + }) + } + + // For non-task terminals, allow reuse of any available terminal + if (!terminal && !taskId) { terminal = terminals.find((t) => { if (t.busy || t.provider !== provider) { return false @@ -197,6 +233,7 @@ export class TerminalRegistry { terminal = this.createTerminal(cwd, provider) } + // Assign the task ID to ensure proper isolation terminal.taskId = taskId return terminal @@ -284,7 +321,14 @@ export class TerminalRegistry { public static releaseTerminalsForTask(taskId: string): void { this.terminals.forEach((terminal) => { if (terminal.taskId === taskId) { + // Mark the terminal as not busy to allow reuse + terminal.busy = false + // Clear the task association terminal.taskId = undefined + // Clear any active process to prevent interference + if (terminal.process) { + terminal.process = undefined + } } }) } @@ -311,7 +355,15 @@ export class TerminalRegistry { * @returns The Terminal object, or undefined if not found */ private static getTerminalByVSCETerminal(vsceTerminal: vscode.Terminal): RooTerminal | undefined { - const found = this.terminals.find((t) => t instanceof Terminal && t.terminal === vsceTerminal) + // Filter to only our Terminal instances (not ExecaTerminal) + const found = this.terminals.find((t) => { + // Must be a Terminal instance (not ExecaTerminal) + if (!(t instanceof Terminal)) { + return false + } + // Must match the VSCode terminal instance + return t.terminal === vsceTerminal + }) if (found?.isClosed()) { this.removeTerminal(found.id) diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.concurrent.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.concurrent.spec.ts new file mode 100644 index 000000000000..82b886076dd7 --- /dev/null +++ b/src/integrations/terminal/__tests__/TerminalRegistry.concurrent.spec.ts @@ -0,0 +1,409 @@ +import { describe, it, expect, beforeEach, afterEach, vi, Mock } from "vitest" +import { TerminalRegistry } from "../TerminalRegistry" +import { Terminal } from "../Terminal" +import { ExecaTerminal } from "../ExecaTerminal" +import { TerminalProcess } from "../TerminalProcess" + +// Mocks need to be defined in factory functions to avoid hoisting issues +vi.mock("vscode", () => { + const mockOnDidCloseTerminal = vi.fn() + const mockOnDidStartTerminalShellExecution = vi.fn() + const mockOnDidEndTerminalShellExecution = vi.fn() + + return { + window: { + onDidCloseTerminal: mockOnDidCloseTerminal, + onDidStartTerminalShellExecution: mockOnDidStartTerminalShellExecution, + onDidEndTerminalShellExecution: mockOnDidEndTerminalShellExecution, + }, + Uri: { + file: vi.fn((path: string) => ({ fsPath: path })), + }, + } +}) + +vi.mock("../Terminal") +vi.mock("../ExecaTerminal") +vi.mock("../ShellIntegrationManager", () => ({ + ShellIntegrationManager: { + zshCleanupTmpDir: vi.fn(), + clear: vi.fn(), + terminalTmpDirs: new Map(), + }, +})) +vi.mock("../TerminalProcess", () => ({ + TerminalProcess: { + interpretExitCode: vi.fn((code: any) => ({ exitCode: code })), + }, +})) + +// Import vscode after mocks are set up +import * as vscode from "vscode" + +describe("TerminalRegistry - Concurrent Operations", () => { + let mockDisposable: vscode.Disposable + let mockVsCodeWindow: any + + beforeEach(() => { + // Reset the terminals array for each test + ;(TerminalRegistry as any).terminals = [] + ;(TerminalRegistry as any).nextTerminalId = 1 + ;(TerminalRegistry as any).isInitialized = false + ;(TerminalRegistry as any).disposables = [] + + mockDisposable = { dispose: vi.fn() } + + // Get mocked window object + mockVsCodeWindow = vi.mocked(vscode.window) + + // Mock vscode event handlers to return disposable + mockVsCodeWindow.onDidCloseTerminal.mockReturnValue(mockDisposable) + mockVsCodeWindow.onDidStartTerminalShellExecution.mockReturnValue(mockDisposable) + mockVsCodeWindow.onDidEndTerminalShellExecution.mockReturnValue(mockDisposable) + }) + + afterEach(() => { + vi.clearAllMocks() + try { + TerminalRegistry.cleanup() + } catch (e) { + // Ignore cleanup errors in tests + } + }) + + describe("Non-blocking Shell Execution Handlers", () => { + it("should handle shell execution start events asynchronously", async () => { + TerminalRegistry.initialize() + + const mockTerminal = { + id: 1, + provider: "vscode", + setActiveStream: vi.fn(), + busy: false, + } + + const mockVSCodeTerminal = {} as vscode.Terminal + const mockExecution = { + read: vi.fn().mockReturnValue("mock-stream"), + commandLine: { value: "test-command" }, + } + + const mockEvent = { + terminal: mockVSCodeTerminal, + execution: mockExecution, + } as any + + // Add the terminal to registry + ;(TerminalRegistry as any).terminals = [mockTerminal] + + // Mock getTerminalByVSCETerminal to return our mock terminal + const getTerminalSpy = vi + .spyOn(TerminalRegistry as any, "getTerminalByVSCETerminal") + .mockReturnValue(mockTerminal) + + // Get the handler that was registered + const startHandler = mockVsCodeWindow.onDidStartTerminalShellExecution.mock.calls[0][0] + + // Call the handler with our mock event + startHandler(mockEvent) + + // Use setImmediate to wait for async processing + await new Promise((resolve) => setImmediate(resolve)) + + // Verify the terminal was processed + expect(mockTerminal.setActiveStream).toHaveBeenCalledWith("mock-stream") + expect(mockTerminal.busy).toBe(true) + }) + + it("should handle shell execution end events asynchronously", async () => { + TerminalRegistry.initialize() + + const mockProcess = { + hasUnretrievedOutput: vi.fn().mockReturnValue(false), + emit: vi.fn(), + } + + const mockTerminal = { + id: 1, + provider: "vscode", + busy: true, + running: true, + process: mockProcess, + shellExecutionComplete: vi.fn(), + completedProcesses: [], + } + + const mockVSCodeTerminal = {} as vscode.Terminal + const mockExecution = { + commandLine: { value: "test-command" }, + } + + const mockEvent = { + terminal: mockVSCodeTerminal, + execution: mockExecution, + exitCode: 0, + } as any + + // Add the terminal to registry + ;(TerminalRegistry as any).terminals = [mockTerminal] + + // Mock getTerminalByVSCETerminal to return our mock terminal + vi.spyOn(TerminalRegistry as any, "getTerminalByVSCETerminal").mockReturnValue(mockTerminal) + + // Get the handler that was registered + const endHandler = mockVsCodeWindow.onDidEndTerminalShellExecution.mock.calls[0][0] + + // Call the handler with our mock event + endHandler(mockEvent) + + // Use setImmediate to wait for async processing + await new Promise((resolve) => setImmediate(resolve)) + + // Verify the terminal was processed + expect(mockTerminal.shellExecutionComplete).toHaveBeenCalled() + expect(mockTerminal.busy).toBe(false) + }) + + it("should not block when processing multiple concurrent events", async () => { + TerminalRegistry.initialize() + + const terminals = Array.from({ length: 5 }, (_, i) => ({ + id: i + 1, + provider: "vscode" as const, + busy: false, + running: false, + setActiveStream: vi.fn(), + shellExecutionComplete: vi.fn(), + process: undefined, + })) + + // Add all terminals to registry + ;(TerminalRegistry as any).terminals = terminals + + const startHandler = mockVsCodeWindow.onDidStartTerminalShellExecution.mock.calls[0][0] + + // Create multiple concurrent events + const events = terminals.map((terminal, i) => ({ + terminal: {} as vscode.Terminal, + execution: { + read: vi.fn().mockReturnValue(`stream-${i}`), + commandLine: { value: `command-${i}` }, + }, + })) + + // Mock getTerminalByVSCETerminal to return corresponding terminal + const getTerminalSpy = vi.spyOn(TerminalRegistry as any, "getTerminalByVSCETerminal") + getTerminalSpy.mockImplementation((vsceTerminal) => { + const index = events.findIndex((e) => e.terminal === vsceTerminal) + return index >= 0 ? terminals[index] : undefined + }) + + // Fire all events simultaneously + const startTime = Date.now() + events.forEach((event) => startHandler(event as any)) + const syncTime = Date.now() - startTime + + // Handlers should return immediately (non-blocking) + expect(syncTime).toBeLessThan(10) + + // Wait for all async processing to complete + await new Promise((resolve) => setImmediate(resolve)) + + // Verify all terminals were processed + terminals.forEach((terminal, i) => { + expect(terminal.setActiveStream).toHaveBeenCalledWith(`stream-${i}`) + expect(terminal.busy).toBe(true) + }) + }) + }) + + describe("Task Isolation", () => { + it("should not reuse terminals across different tasks", async () => { + const cwd = "/test/project" + + // Mock Terminal constructor + const mockTerminals: any[] = [] + ;(Terminal as any).mockImplementation(function (this: any, id: number, _terminal: any, cwd: string) { + const terminal = { + id, + provider: "vscode", + initialCwd: cwd, + busy: false, + taskId: undefined, + getCurrentWorkingDirectory: vi.fn().mockReturnValue(cwd), + isClosed: vi.fn().mockReturnValue(false), + } + mockTerminals.push(terminal) + return terminal + }) + + // Create terminal for task1 + const terminal1 = await TerminalRegistry.getOrCreateTerminal(cwd, "task1") + expect(terminal1.taskId).toBe("task1") + + // Mark terminal1 as not busy to make it available + terminal1.busy = false + + // Request terminal for task2 should create a new one, not reuse task1's terminal + const terminal2 = await TerminalRegistry.getOrCreateTerminal(cwd, "task2") + + expect(terminal2).not.toBe(terminal1) + expect(terminal2.taskId).toBe("task2") + expect(terminal1.taskId).toBe("task1") // task1's terminal should remain unchanged + }) + + it("should allow terminal reuse within the same task", async () => { + const cwd = "/test/project" + + // Mock Terminal constructor + const mockTerminal = { + id: 1, + provider: "vscode", + initialCwd: cwd, + busy: false, + taskId: undefined, + getCurrentWorkingDirectory: vi.fn().mockReturnValue(cwd), + isClosed: vi.fn().mockReturnValue(false), + } + + ;(Terminal as any).mockImplementation(() => mockTerminal) + + // Create terminal for task1 + const terminal1 = await TerminalRegistry.getOrCreateTerminal(cwd, "task1") + terminal1.busy = false + + // Request another terminal for task1 should reuse the existing one + const terminal2 = await TerminalRegistry.getOrCreateTerminal(cwd, "task1") + + expect(terminal2).toBe(terminal1) + expect(terminal2.taskId).toBe("task1") + }) + + it("should properly release terminals when task completes", () => { + const terminals = [ + { id: 1, taskId: "task1", busy: true, process: {} }, + { id: 2, taskId: "task2", busy: true, process: {} }, + { id: 3, taskId: "task1", busy: false, process: {} }, + ] + + ;(TerminalRegistry as any).terminals = terminals + + TerminalRegistry.releaseTerminalsForTask("task1") + + // Verify task1 terminals were released + expect(terminals[0].taskId).toBeUndefined() + expect(terminals[0].busy).toBe(false) + expect(terminals[0].process).toBeUndefined() + + expect(terminals[2].taskId).toBeUndefined() + expect(terminals[2].busy).toBe(false) + expect(terminals[2].process).toBeUndefined() + + // task2 terminal should be unaffected + expect(terminals[1].taskId).toBe("task2") + expect(terminals[1].busy).toBe(true) + expect(terminals[1].process).toBeDefined() + }) + + it("should only use unassigned terminals for new tasks", async () => { + const cwd = "/test/project" + + // Create an unassigned terminal + const unassignedTerminal: any = { + id: 1, + provider: "vscode" as const, + busy: false, + taskId: undefined, + getCurrentWorkingDirectory: vi.fn().mockReturnValue(cwd), + isClosed: vi.fn().mockReturnValue(false), + } + + // Create a terminal assigned to another task + const assignedTerminal: any = { + id: 2, + provider: "vscode" as const, + busy: false, + taskId: "other-task", + getCurrentWorkingDirectory: vi.fn().mockReturnValue(cwd), + isClosed: vi.fn().mockReturnValue(false), + } + + ;(TerminalRegistry as any).terminals = [assignedTerminal, unassignedTerminal] + + // Request terminal for a new task + const terminal = await TerminalRegistry.getOrCreateTerminal(cwd, "new-task") + + // Should use the unassigned terminal, not the one assigned to other-task + expect(terminal).toBe(unassignedTerminal) + expect(unassignedTerminal.taskId).toBe("new-task") + expect(assignedTerminal.taskId).toBe("other-task") // Should remain unchanged + }) + }) + + describe("Error Handling", () => { + it("should handle errors in shell execution start handler", async () => { + TerminalRegistry.initialize() + + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + + const mockEvent = { + terminal: {} as vscode.Terminal, + execution: { + read: vi.fn().mockImplementation(() => { + throw new Error("Stream read failed") + }), + commandLine: { value: "test-command" }, + }, + } as any + + const startHandler = mockVsCodeWindow.onDidStartTerminalShellExecution.mock.calls[0][0] + + // Should not throw + expect(() => startHandler(mockEvent)).not.toThrow() + + // Wait for async processing + await new Promise((resolve) => setImmediate(resolve)) + + // Should log the error + expect(consoleSpy).toHaveBeenCalledWith( + "[onDidStartTerminalShellExecution] Error handling shell execution start:", + expect.any(Error), + ) + + consoleSpy.mockRestore() + }) + + it("should handle errors in shell execution end handler", async () => { + TerminalRegistry.initialize() + + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + + // Mock getTerminalByVSCETerminal to throw + vi.spyOn(TerminalRegistry as any, "getTerminalByVSCETerminal").mockImplementation(() => { + throw new Error("Terminal lookup failed") + }) + + const mockEvent = { + terminal: {} as vscode.Terminal, + execution: { commandLine: { value: "test-command" } }, + exitCode: 0, + } as any + + const endHandler = mockVsCodeWindow.onDidEndTerminalShellExecution.mock.calls[0][0] + + // Should not throw + expect(() => endHandler(mockEvent)).not.toThrow() + + // Wait for async processing + await new Promise((resolve) => setImmediate(resolve)) + + // Should log the error + expect(consoleSpy).toHaveBeenCalledWith( + "[onDidEndTerminalShellExecution] Error handling shell execution end:", + expect.any(Error), + ) + + consoleSpy.mockRestore() + }) + }) +})