More cleanup

Author: cte

src/core/Cline.ts

@@ -1986,6 +1986,7 @@ export class Cline extends EventEmitter<ClineEvents> {
 
 		// It could be useful for cline to know if the user went from one or no file to another between messages, so we always include this context
 		details += "\n\n# VSCode Visible Files"
+
 		const visibleFilePaths = vscode.window.visibleTextEditors
 			?.map((editor) => editor.document?.uri?.fsPath)
 			.filter(Boolean)
@@ -2039,28 +2040,31 @@ export class Cline extends EventEmitter<ClineEvents> {
 		}
 
 		if (busyTerminals.length > 0) {
-			// wait for terminals to cool down
+			// Wait for terminals to cool down.
 			await pWaitFor(() => busyTerminals.every((t) => !TerminalRegistry.isProcessHot(t.id)), {
 				interval: 100,
 				timeout: 15_000,
 			}).catch(() => {})
 		}
 
-		this.didEditFile = false // reset, this lets us know when to wait for saved files to update terminals
+		// Reset, this lets us know when to wait for saved files to update terminals.
+		this.didEditFile = false
 
-		// waiting for updated diagnostics lets terminal output be the most up-to-date possible
+		// Waiting for updated diagnostics lets terminal output be the most
+		// up-to-date possible.
 		let terminalDetails = ""
+
 		if (busyTerminals.length > 0) {
-			// terminals are cool, let's retrieve their output
+			// Terminals are cool, let's retrieve their output.
 			terminalDetails += "\n\n# Actively Running Terminals"
+
 			for (const busyTerminal of busyTerminals) {
 				terminalDetails += `\n## Original command: \`${busyTerminal.getLastCommand()}\``
 				let newOutput = TerminalRegistry.getUnretrievedOutput(busyTerminal.id)
+
 				if (newOutput) {
 					newOutput = Terminal.compressTerminalOutput(newOutput, terminalOutputLineLimit)
 					terminalDetails += `\n### New Output\n${newOutput}`
-				} else {
-					// details += `\n(Still running, no new output)` // don't want to show this right after running the command
 				}
 			}
 		}

src/integrations/terminal/BaseTerminalProcess.ts

@@ -4,7 +4,9 @@ import type { RooTerminalProcess, RooTerminalProcessEvents, ExitCodeDetails } fr
 
 export abstract class BaseTerminalProcess extends EventEmitter<RooTerminalProcessEvents> implements RooTerminalProcess {
 	public command: string = ""
+
 	public isHot: boolean = false
+	protected hotTimer: NodeJS.Timeout | null = null
 
 	protected isListening: boolean = true
 	protected lastEmitTime_ms: number = 0
@@ -134,4 +136,51 @@ export abstract class BaseTerminalProcess extends EventEmitter<RooTerminalProces
 	 * @returns The unretrieved output
 	 */
 	abstract getUnretrievedOutput(): string
+
+	protected startHotTimer(data: string) {
+		this.isHot = true
+
+		if (this.hotTimer) {
+			clearTimeout(this.hotTimer)
+		}
+
+		this.hotTimer = setTimeout(() => (this.isHot = false), BaseTerminalProcess.isCompiling(data) ? 15_000 : 2_000)
+	}
+
+	protected stopHotTimer() {
+		if (this.hotTimer) {
+			clearTimeout(this.hotTimer)
+		}
+
+		this.isHot = false
+	}
+
+	// These markers indicate the command is some kind of local dev
+	// server recompiling the app, which we want to wait for output
+	// of before sending request to Roo Code.
+	private static compilingMarkers = ["compiling", "building", "bundling", "transpiling", "generating", "starting"]
+
+	private static compilingMarkerNullifiers = [
+		"compiled",
+		"success",
+		"finish",
+		"complete",
+		"succeed",
+		"done",
+		"end",
+		"stop",
+		"exit",
+		"terminate",
+		"error",
+		"fail",
+	]
+
+	private static isCompiling(data: string): boolean {
+		return (
+			BaseTerminalProcess.compilingMarkers.some((marker) => data.toLowerCase().includes(marker.toLowerCase())) &&
+			!BaseTerminalProcess.compilingMarkerNullifiers.some((nullifier) =>
+				data.toLowerCase().includes(nullifier.toLowerCase()),
+			)
+		)
+	}
 }

src/integrations/terminal/ExecaTerminalProcess.ts

@@ -6,7 +6,6 @@ import { BaseTerminalProcess } from "./BaseTerminalProcess"
 export class ExecaTerminalProcess extends BaseTerminalProcess {
 	private terminalRef: WeakRef<RooTerminal>
 	private controller?: AbortController
-	private isStreamClosed: boolean = false
 
 	constructor(terminal: RooTerminal) {
 		super()
@@ -29,6 +28,8 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 		this.controller = new AbortController()
 
 		try {
+			this.isHot = true
+
 			const subprocess = execa({
 				shell: true,
 				cwd: this.terminal.getCurrentWorkingDirectory(),
@@ -38,19 +39,17 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 			this.emit("line", "")
 
 			for await (const line of subprocess) {
-				this.fullOutput += line
-				this.fullOutput += "\n"
+				this.fullOutput += `${line}\n`
 
 				const now = Date.now()
 
 				if (this.isListening && (now - this.lastEmitTime_ms > 250 || this.lastEmitTime_ms === 0)) {
 					this.emitRemainingBufferIfListening()
 					this.lastEmitTime_ms = now
 				}
-			}
 
-			this.isStreamClosed = true
-			this.emitRemainingBufferIfListening()
+				this.startHotTimer(line)
+			}
 
 			this.emit("shell_execution_complete", { exitCode: 0 })
 		} catch (error) {
@@ -65,6 +64,8 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 			}
 		}
 
+		this.emitRemainingBufferIfListening()
+		this.stopHotTimer()
 		this.emit("completed", this.fullOutput)
 		this.emit("continue")
 	}
@@ -85,20 +86,14 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 
 	public override getUnretrievedOutput() {
 		let outputToProcess = this.fullOutput.slice(this.lastRetrievedIndex)
-		let endIndex = -1
-
-		if (this.isStreamClosed) {
-			endIndex = outputToProcess.length
-		} else {
-			endIndex = outputToProcess.lastIndexOf("\n")
+		let endIndex = outputToProcess.lastIndexOf("\n")
 
-			if (endIndex === -1) {
-				return ""
-			}
-
-			endIndex++
+		if (endIndex === -1) {
+			return ""
 		}
 
+		endIndex++
+
 		this.lastRetrievedIndex += endIndex
 		return outputToProcess.slice(0, endIndex)
 	}

src/integrations/terminal/README.md

@@ -0,0 +1,66 @@
+NOTICE TO DEVELOPERS:
+
+The Terminal classes are very sensitive to change, partially because of
+the complicated way that shell integration works with VSCE, and
+partially because of the way that Cline interacts with the Terminal\*
+class abstractions that make VSCE shell integration easier to work with.
+
+At the point that PR #1365 is merged, it is unlikely that any Terminal\*
+classes will need to be modified substantially. Generally speaking, we
+should think of this as a stable interface and minimize changes.
+
+`TerminalProcess` class is particularly critical because it
+provides all input handling and event notifications related to terminal
+output to send it to the rest of the program. User interfaces for working
+with data from terminals should only be as follows:
+
+1. By listening to the events:
+
+    - this.on("completed", fullOutput) - provides full output upon completion
+    - this.on("line") - provides new lines, probably more than one
+
+2. By calling `this.getUnretrievedOutput()`
+
+This implementation intentionally returns all terminal output to the user
+interfaces listed above. Any throttling or other stream modification _must_
+be implemented outside of this class.
+
+All other interfaces are private.
+
+Warning: Modifying the `TerminalProcess` class without fully understanding VSCE shell integration architecture may affect the reliability or performance of reading terminal output.
+
+`TerminalProcess` was carefully designed for performance and accuracy:
+
+Performance is obtained by: - Throttling event output on 100ms intervals - Using only indexes to access the output array - Maintaining a zero-copy implementation with a fullOutput string for storage - The fullOutput array is never split on carriage returns
+as this was found to be very slow - Allowing multi-line chunks - Minimizing regular expression calls, as they have been tested to be
+500x slower than the use of string parsing functions for large outputs
+in this implementation
+
+Accuracy is obtained by: - Using only indexes against fullOutput - Paying close attention to off-by-one errors when indexing any content - Always returning exactly the content that was printed by the terminal,
+including all carriage returns which may (or may not) have been in the
+input stream
+
+Additional resources:
+
+- This implementation was rigorously tested using:
+
+    - https://github.com/KJ7LNW/vsce-test-terminal-integration
+
+- There was a serious upstream bug that may not be fully solved,
+  or that may resurface in future VSCE releases, simply due to
+  the complexity of reliably handling terminal-provided escape
+  sequences across multiple shell implementations. This implementation
+  attempts to work around the problems and provide backwards
+  compatibility for VSCE releases that may not have the fix in
+  upstream bug #237208, but there still may be some unhandled
+  corner cases. See this ticket for more detail:
+
+    - https://github.com/microsoft/vscode/issues/237208
+
+- The original Cline PR has quite a bit of information:
+    - https://github.com/cline/cline/pull/1089
+
+Contact me if you have any questions: - GitHub: KJ7LNW - Discord: kj7lnw - [roo-cline at z.ewheeler.org]
+
+Cheers,
+-Eric, KJ7LNW