diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 3bf87119d5..8ac333c1f6 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -29,6 +29,7 @@ import { } 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" import { listFiles } from "../services/glob/list-files" import { regexSearchFiles } from "../services/ripgrep" @@ -3472,8 +3473,8 @@ export class Cline { details += "\n(No open tabs)" } - const busyTerminals = this.terminalManager.getTerminals(true) - const inactiveTerminals = this.terminalManager.getTerminals(false) + const busyTerminals = TerminalRegistry.getTerminals(true) + const inactiveTerminals = TerminalRegistry.getTerminals(false) // const allTerminals = [...busyTerminals, ...inactiveTerminals] if (busyTerminals.length > 0 && this.didEditFile) { @@ -3485,7 +3486,7 @@ export class Cline { if (busyTerminals.length > 0) { // wait for terminals to cool down // terminalWasBusy = allTerminals.some((t) => this.terminalManager.isProcessHot(t.id)) - await pWaitFor(() => busyTerminals.every((t) => !this.terminalManager.isProcessHot(t.id)), { + await pWaitFor(() => busyTerminals.every((t) => !TerminalRegistry.isProcessHot(t.id)), { interval: 100, timeout: 15_000, }).catch(() => {}) @@ -3517,7 +3518,7 @@ export class Cline { terminalDetails += "\n\n# Actively Running Terminals" for (const busyTerminal of busyTerminals) { terminalDetails += `\n## Original command: \`${busyTerminal.lastCommand}\`` - const newOutput = this.terminalManager.getUnretrievedOutput(busyTerminal.id) + const newOutput = TerminalRegistry.getUnretrievedOutput(busyTerminal.id) if (newOutput) { terminalDetails += `\n### New Output\n${newOutput}` } else { @@ -3529,7 +3530,7 @@ export class Cline { if (inactiveTerminals.length > 0) { const inactiveTerminalOutputs = new Map() for (const inactiveTerminal of inactiveTerminals) { - const newOutput = this.terminalManager.getUnretrievedOutput(inactiveTerminal.id) + const newOutput = TerminalRegistry.getUnretrievedOutput(inactiveTerminal.id) if (newOutput) { inactiveTerminalOutputs.set(inactiveTerminal.id, newOutput) } diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 1b91a378cd..8140d27fbb 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -1,13 +1,15 @@ import * as vscode from "vscode" +import { ExitCodeDetails, TerminalProcess } from "./TerminalProcess" export class Terminal { public terminal: vscode.Terminal public busy: boolean public lastCommand: string public id: number - public stream?: AsyncIterable + private stream?: AsyncIterable public running: boolean - public streamClosed: boolean + private streamClosed: boolean + public process?: TerminalProcess constructor(id: number, terminal: vscode.Terminal) { this.id = id @@ -18,6 +20,57 @@ export class Terminal { this.streamClosed = false } + /** + * Gets the terminal's stream + */ + public getStream(): AsyncIterable | undefined { + return this.stream + } + + /** + * Checks if the stream is closed + */ + public isStreamClosed(): boolean { + return this.streamClosed + } + + /** + * Sets the active stream for this terminal and notifies the process + * @param stream The stream to set, or undefined to clean up + * @throws Error if process is undefined when a stream is provided + */ + public setActiveStream(stream: AsyncIterable | undefined): void { + this.stream = stream + + if (stream) { + // New stream is available + if (!this.process) { + throw new Error(`Cannot set active stream on terminal ${this.id} because process is undefined`) + } + + this.streamClosed = false + this.running = true + this.process.emit("stream_available", this.id, stream) + } else { + // Stream is being closed + this.streamClosed = true + this.running = false + } + } + + /** + * Handles shell execution completion for this terminal + * @param exitDetails The exit details of the shell execution + */ + public shellExecutionComplete(exitDetails: ExitCodeDetails): void { + this.running = false + + if (this.process) { + this.process.emit("shell_execution_complete", this.id, exitDetails) + this.process = undefined + } + } + /** * Gets the terminal contents based on the number of commands to include * @param commands Number of previous commands to include (-1 for all) diff --git a/src/integrations/terminal/TerminalManager.ts b/src/integrations/terminal/TerminalManager.ts index 4f187337a4..d3327a7f1c 100644 --- a/src/integrations/terminal/TerminalManager.ts +++ b/src/integrations/terminal/TerminalManager.ts @@ -97,7 +97,6 @@ declare module "vscode" { export class TerminalManager { private terminalIds: Set = new Set() - private processes: Map = new Map() private disposables: vscode.Disposable[] = [] constructor() { @@ -108,15 +107,9 @@ export class TerminalManager { 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.getTerminalInfoByTerminal(e.terminal) - if (stream && terminalInfo) { - const process = this.processes.get(terminalInfo.id) - if (process) { - terminalInfo.stream = stream - terminalInfo.running = true - terminalInfo.streamClosed = false - process.emit("stream_available", terminalInfo.id, stream) - } + const terminalInfo = TerminalRegistry.getTerminalByVSCETerminal(e.terminal) + if (terminalInfo) { + terminalInfo.setActiveStream(stream) } else { console.error("[TerminalManager] Stream failed, not registered for terminal") } @@ -130,25 +123,16 @@ export class TerminalManager { // onDidEndTerminalShellExecution endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => { - // Find the terminal ID by the VSCode terminal instance - const terminalId = this.findTerminalIdByVscodeTerminal(e.terminal) - const process = terminalId !== undefined ? this.processes.get(terminalId) : undefined + const terminalInfo = TerminalRegistry.getTerminalByVSCETerminal(e.terminal) + const process = terminalInfo?.process const exitDetails = process ? process.interpretExitCode(e?.exitCode) : { exitCode: e?.exitCode } console.info("[TerminalManager] Shell execution ended:", { ...exitDetails, }) // Signal completion to any waiting processes - for (const id of this.terminalIds) { - const info = TerminalRegistry.getTerminal(id) - if (info && info.terminal === e.terminal) { - info.running = false - const process = this.processes.get(id) - if (process) { - process.emit("shell_execution_complete", id, exitDetails) - } - break - } + if (terminalInfo && this.terminalIds.has(terminalInfo.id)) { + terminalInfo.shellExecutionComplete(exitDetails) } }) } catch (error) { @@ -165,50 +149,32 @@ export class TerminalManager { runCommand(terminalInfo: Terminal, command: string): TerminalProcessResultPromise { terminalInfo.busy = true terminalInfo.lastCommand = command - const process = new TerminalProcess() - this.processes.set(terminalInfo.id, process) - process.once("completed", () => { - terminalInfo.busy = false - }) + // Create process immediately + const process = new TerminalProcess(terminalInfo) - // if shell integration is not available, remove terminal so it does not get reused as it may be running a long-running process - process.once("no_shell_integration", () => { - console.log(`no_shell_integration received for terminal ${terminalInfo.id}`) - // Remove the terminal so we can't reuse it (in case it's running a long-running process) - TerminalRegistry.removeTerminal(terminalInfo.id) - this.terminalIds.delete(terminalInfo.id) - this.processes.delete(terminalInfo.id) - }) + // Set process on terminal + terminalInfo.process = process + // Create a promise for command completion const promise = new Promise((resolve, reject) => { - process.once("continue", () => { - resolve() - }) + // Set up event handlers + process.once("continue", () => resolve()) process.once("error", (error) => { console.error(`Error in terminal ${terminalInfo.id}:`, error) reject(error) }) - }) - // Always use pWaitFor, which resolves immediately if shell integration is already available - pWaitFor(() => terminalInfo.terminal.shellIntegration !== undefined, { timeout: 4000 }) - .then(() => { - const existingProcess = this.processes.get(terminalInfo.id) - if (existingProcess) { - existingProcess.run(terminalInfo.terminal, command) - } else { - console.error("[TerminalManager] existingProcess not found for terminal", terminalInfo.id) - } - }) - .catch(() => { - // Shell integration did not become available within timeout - const existingProcess = this.processes.get(terminalInfo.id) - if (existingProcess) { + // Wait for shell integration before executing the command + pWaitFor(() => terminalInfo.terminal.shellIntegration !== undefined, { timeout: 4000 }) + .then(() => { + process.run(command) + }) + .catch(() => { console.log("[TerminalManager] Shell integration not available. Command execution aborted.") - existingProcess.emit("no_shell_integration") - } - }) + process.emit("no_shell_integration") + }) + }) return mergePromise(process, promise) } @@ -247,47 +213,11 @@ export class TerminalManager { return newTerminalInfo } - getTerminals(busy: boolean): { id: number; lastCommand: string }[] { - return Array.from(this.terminalIds) - .map((id) => TerminalRegistry.getTerminal(id)) - .filter((t): t is Terminal => t !== undefined && t.busy === busy) - .map((t) => ({ id: t.id, lastCommand: t.lastCommand })) - } - - getUnretrievedOutput(terminalId: number): string { - if (!this.terminalIds.has(terminalId)) { - return "" - } - const process = this.processes.get(terminalId) - return process ? process.getUnretrievedOutput() : "" - } - - /** - * Finds the terminal ID by the VSCode terminal instance - * @param terminal The VSCode terminal instance - * @returns The terminal ID or undefined if not found - */ - private findTerminalIdByVscodeTerminal(terminal: vscode.Terminal): number | undefined { - for (const id of this.terminalIds) { - const info = TerminalRegistry.getTerminal(id) - if (info && info.terminal === terminal) { - return id - } - } - return undefined - } - - isProcessHot(terminalId: number): boolean { - const process = this.processes.get(terminalId) - return process ? process.isHot : false - } - disposeAll() { // for (const info of this.terminals) { // //info.terminal.dispose() // dont want to dispose terminals when task is aborted // } this.terminalIds.clear() - this.processes.clear() this.disposables.forEach((disposable) => disposable.dispose()) this.disposables = [] } diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index d6f89bac58..0fbb33b418 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -33,11 +33,32 @@ const PROCESS_HOT_TIMEOUT_COMPILING = 15_000 export class TerminalProcess extends EventEmitter { private isListening: boolean = true - private terminalInfo: Terminal | undefined + private terminalInfo: Terminal private lastEmitTime_ms: number = 0 private fullOutput: string = "" private lastRetrievedIndex: number = 0 isHot: boolean = false + constructor(terminal: Terminal) { + super() + + // Store terminal info for later use + this.terminalInfo = terminal + + // Set up event handlers + this.once("completed", () => { + if (this.terminalInfo) { + this.terminalInfo.busy = false + } + }) + + this.once("no_shell_integration", () => { + if (this.terminalInfo) { + console.log(`no_shell_integration received for terminal ${this.terminalInfo.id}`) + TerminalRegistry.removeTerminal(this.terminalInfo.id) + // Note: TerminalManager.terminalIds cleanup would need to be handled + } + }) + } interpretExitCode(exitCode: number | undefined): ExitCodeDetails { if (exitCode === undefined) { @@ -134,23 +155,15 @@ export class TerminalProcess extends EventEmitter { } private hotTimer: NodeJS.Timeout | null = null - async run(terminal: vscode.Terminal, command: string) { - if (terminal.shellIntegration && terminal.shellIntegration.executeCommand) { - // Get terminal info to access stream - const terminalInfo = TerminalRegistry.getTerminalInfoByTerminal(terminal) - if (!terminalInfo) { - console.error("[TerminalProcess] Terminal not found in registry") - this.emit("no_shell_integration") - this.emit("completed") - this.emit("continue") - return - } + async run(command: string) { + const terminal = this.terminalInfo.terminal + if (terminal.shellIntegration && terminal.shellIntegration.executeCommand) { // When executeCommand() is called, onDidStartTerminalShellExecution will fire in TerminalManager // which creates a new stream via execution.read() and emits 'stream_available' const streamAvailable = new Promise>((resolve) => { this.once("stream_available", (id: number, stream: AsyncIterable) => { - if (id === terminalInfo.id) { + if (id === this.terminalInfo.id) { resolve(stream) } }) @@ -159,15 +172,12 @@ export class TerminalProcess extends EventEmitter { // Create promise that resolves when shell execution completes for this terminal const shellExecutionComplete = new Promise((resolve) => { this.once("shell_execution_complete", (id: number, exitDetails: ExitCodeDetails) => { - if (id === terminalInfo.id) { + if (id === this.terminalInfo.id) { resolve(exitDetails) } }) }) - // getUnretrievedOutput needs to know if streamClosed, so store this for later - this.terminalInfo = terminalInfo - // Execute command terminal.shellIntegration.executeCommand(command) this.isHot = true @@ -253,7 +263,7 @@ export class TerminalProcess extends EventEmitter { // Set streamClosed immediately after stream ends if (this.terminalInfo) { - this.terminalInfo.streamClosed = true + this.terminalInfo.setActiveStream(undefined) } // Wait for shell execution to complete and handle exit details @@ -346,7 +356,7 @@ export class TerminalProcess extends EventEmitter { // For active streams: return only complete lines (up to last \n). // For closed streams: return all remaining content. if (endIndex === -1) { - if (!this.terminalInfo?.streamClosed) { + if (this.terminalInfo && !this.terminalInfo.isStreamClosed()) { // Stream still running - only process complete lines endIndex = outputToProcess.lastIndexOf("\n") if (endIndex === -1) { diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 1689190631..ef72ae428b 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -51,7 +51,12 @@ export class TerminalRegistry { } } - static getTerminalInfoByTerminal(terminal: vscode.Terminal): Terminal | undefined { + /** + * Gets a terminal by its VSCode terminal instance + * @param terminal The VSCode terminal instance + * @returns The Terminal object, or undefined if not found + */ + static getTerminalByVSCETerminal(terminal: vscode.Terminal): Terminal | undefined { const terminalInfo = this.terminals.find((t) => t.terminal === terminal) if (terminalInfo && this.isTerminalClosed(terminalInfo.terminal)) { @@ -75,4 +80,39 @@ export class TerminalRegistry { private static isTerminalClosed(terminal: vscode.Terminal): boolean { return terminal.exitStatus !== undefined } + + /** + * Gets unretrieved output from a terminal process + * @param terminalId The terminal ID + * @returns The unretrieved output as a string, or empty string if terminal not found + */ + static getUnretrievedOutput(terminalId: number): string { + const terminal = this.getTerminal(terminalId) + if (!terminal) { + return "" + } + return terminal.process ? terminal.process.getUnretrievedOutput() : "" + } + + /** + * Checks if a terminal process is "hot" (recently active) + * @param terminalId The terminal ID + * @returns True if the process is hot, false otherwise + */ + static isProcessHot(terminalId: number): boolean { + const terminal = this.getTerminal(terminalId) + if (!terminal) { + return false + } + return terminal.process ? terminal.process.isHot : false + } + + /** + * Gets terminals filtered by busy state + * @param busy Whether to get busy or non-busy terminals + * @returns Array of Terminal objects + */ + static getTerminals(busy: boolean): Terminal[] { + return this.getAllTerminals().filter((t) => t.busy === busy) + } } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.test.ts b/src/integrations/terminal/__tests__/TerminalProcess.test.ts index 2556947264..8a9f5c6421 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.test.ts @@ -35,8 +35,6 @@ describe("TerminalProcess", () => { let mockStream: AsyncIterableIterator beforeEach(() => { - terminalProcess = new TerminalProcess() - // Create properly typed mock terminal mockTerminal = { shellIntegration: { @@ -61,6 +59,9 @@ describe("TerminalProcess", () => { mockTerminalInfo = new Terminal(1, mockTerminal) + // Create a process for testing + terminalProcess = new TerminalProcess(mockTerminalInfo) + TerminalRegistry["terminals"].push(mockTerminalInfo) // Reset event listeners @@ -93,7 +94,7 @@ describe("TerminalProcess", () => { mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution) - const runPromise = terminalProcess.run(mockTerminal, "test command") + const runPromise = terminalProcess.run("test command") terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream) await runPromise @@ -102,28 +103,38 @@ describe("TerminalProcess", () => { }) it("handles terminals without shell integration", async () => { + // Create a terminal without shell integration const noShellTerminal = { sendText: jest.fn(), shellIntegration: undefined, + name: "No Shell Terminal", + processId: Promise.resolve(456), + creationOptions: {}, + exitStatus: undefined, + state: { isInteractedWith: true }, + dispose: jest.fn(), + hide: jest.fn(), + show: jest.fn(), } as unknown as vscode.Terminal + // Create new terminal info with the no-shell terminal + const noShellTerminalInfo = new Terminal(2, noShellTerminal) + + // Create new process with the no-shell terminal + const noShellProcess = new TerminalProcess(noShellTerminalInfo) + // Set up event listeners to verify events are emitted - const noShellPromise = new Promise((resolve) => { - terminalProcess.once("no_shell_integration", resolve) - }) - const completedPromise = new Promise((resolve) => { - terminalProcess.once("completed", (_output?: string) => resolve()) - }) - const continuePromise = new Promise((resolve) => { - terminalProcess.once("continue", resolve) - }) + const eventPromises = Promise.all([ + new Promise((resolve) => noShellProcess.once("no_shell_integration", resolve)), + new Promise((resolve) => noShellProcess.once("completed", (_output?: string) => resolve())), + new Promise((resolve) => noShellProcess.once("continue", resolve)), + ]) - await terminalProcess.run(noShellTerminal, "test command") + // Run command and wait for all events + await noShellProcess.run("test command") + await eventPromises - // Verify all expected events are emitted - await Promise.all([noShellPromise, completedPromise, continuePromise]) - - // Verify sendText is called with the command + // Verify sendText was called with the command expect(noShellTerminal.sendText).toHaveBeenCalledWith("test command", true) }) @@ -153,7 +164,7 @@ describe("TerminalProcess", () => { read: jest.fn().mockReturnValue(mockStream), }) - const runPromise = terminalProcess.run(mockTerminal, "npm run build") + const runPromise = terminalProcess.run("npm run build") terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream) expect(terminalProcess.isHot).toBe(true) @@ -192,7 +203,7 @@ describe("TerminalProcess", () => { describe("mergePromise", () => { it("merges promise methods with terminal process", async () => { - const process = new TerminalProcess() + const process = new TerminalProcess(mockTerminalInfo) const promise = Promise.resolve() const merged = mergePromise(process, promise) diff --git a/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts b/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts index 0952e6aef1..0410e43e97 100644 --- a/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts @@ -106,13 +106,13 @@ async function testTerminalCommand( // Add the terminal to the registry TerminalRegistry["terminals"] = [mockTerminalInfo] - // Create a new terminal process - startTime = process.hrtime.bigint() // Start timing from terminal process creation - const terminalProcess = new TerminalProcess() - // Create a terminal manager (this will set up the event handlers) const terminalManager = new TerminalManager() + // Create a new terminal process for testing + startTime = process.hrtime.bigint() // Start timing from terminal process creation + const terminalProcess = new TerminalProcess(mockTerminalInfo) + try { // Set up the mock stream with real command output const mockStream = createRealCommandStream(command) @@ -124,6 +124,9 @@ async function testTerminalCommand( } }) + // Execute the command + terminalProcess.run(command) + // Set up event listeners to capture output let capturedOutput = "" terminalProcess.on("completed", (output) => { @@ -143,13 +146,12 @@ async function testTerminalCommand( }) }) - // Store the process in the manager's processes map - // This is needed for the TerminalManager to find the process when events are triggered - terminalManager["processes"].set(mockTerminalInfo.id, terminalProcess) + // Set the process on the terminal and add terminal ID to manager + mockTerminalInfo.process = terminalProcess terminalManager["terminalIds"].add(mockTerminalInfo.id) - // Run the command - const runPromise = terminalProcess.run(mockTerminal, command) + // Run the command (now handled by constructor) + // We've already created the process, so we'll trigger the events manually // Get the event handlers from the mock const eventHandlers = (vscode as any).__eventHandlers @@ -185,8 +187,6 @@ async function testTerminalCommand( // Wait for the command to complete or timeout await Promise.race([completedPromise, timeoutPromise]) - - await runPromise // Calculate execution time in microseconds // If endTime wasn't set (unlikely but possible), set it now if (!timeRecorded) { diff --git a/src/integrations/terminal/__tests__/TerminalProcessInterpretExitCode.test.ts b/src/integrations/terminal/__tests__/TerminalProcessInterpretExitCode.test.ts index 3b8e0a2fcd..5618e243bf 100644 --- a/src/integrations/terminal/__tests__/TerminalProcessInterpretExitCode.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcessInterpretExitCode.test.ts @@ -1,11 +1,28 @@ import { TerminalProcess } from "../TerminalProcess" import { execSync } from "child_process" +import { Terminal } from "../Terminal" +import * as vscode from "vscode" + +// Mock vscode.Terminal for testing +const mockTerminal = { + name: "Test Terminal", + processId: Promise.resolve(123), + creationOptions: {}, + exitStatus: undefined, + state: { isInteractedWith: true }, + dispose: jest.fn(), + hide: jest.fn(), + show: jest.fn(), + sendText: jest.fn(), +} as unknown as vscode.Terminal describe("TerminalProcess.interpretExitCode", () => { let terminalProcess: TerminalProcess + let mockTerminalInfo: Terminal beforeEach(() => { - terminalProcess = new TerminalProcess() + mockTerminalInfo = new Terminal(1, mockTerminal) + terminalProcess = new TerminalProcess(mockTerminalInfo) }) it("should handle undefined exit code", () => { @@ -89,9 +106,11 @@ describe("TerminalProcess.interpretExitCode", () => { describe("TerminalProcess.interpretExitCode with real commands", () => { let terminalProcess: TerminalProcess + let mockTerminalInfo: Terminal beforeEach(() => { - terminalProcess = new TerminalProcess() + mockTerminalInfo = new Terminal(1, mockTerminal) + terminalProcess = new TerminalProcess(mockTerminalInfo) }) it("should correctly interpret exit code 0 from successful command", () => {