Browse Source

Terminal performance improvements (#3119)

Chris Estreich 10 months ago
parent
commit
2e66081921

+ 5 - 4
src/core/tools/executeCommandTool.ts

@@ -152,15 +152,15 @@ export async function executeCommand(
 
 	const callbacks: RooTerminalCallbacks = {
 		onLine: async (output: string, process: RooTerminalProcess) => {
-			const compressed = Terminal.compressTerminalOutput(output, terminalOutputLineLimit)
-			cline.say("command_output", compressed)
+			const status: CommandExecutionStatus = { executionId, status: "output", output }
+			clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
 
 			if (runInBackground) {
 				return
 			}
 
 			try {
-				const { response, text, images } = await cline.ask("command_output", compressed)
+				const { response, text, images } = await cline.ask("command_output", "")
 				runInBackground = true
 
 				if (response === "messageResponse") {
@@ -171,10 +171,11 @@ export async function executeCommand(
 		},
 		onCompleted: (output: string | undefined) => {
 			result = Terminal.compressTerminalOutput(output ?? "", terminalOutputLineLimit)
+			cline.say("command_output", result)
 			completed = true
 		},
 		onShellExecutionStarted: (pid: number | undefined) => {
-			const status: CommandExecutionStatus = { executionId, status: "running", pid }
+			const status: CommandExecutionStatus = { executionId, status: "started", pid, command }
 			clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
 		},
 		onShellExecutionComplete: (details: ExitCodeDetails) => {

+ 33 - 1
src/integrations/terminal/ExecaTerminalProcess.ts

@@ -6,6 +6,7 @@ import { BaseTerminalProcess } from "./BaseTerminalProcess"
 export class ExecaTerminalProcess extends BaseTerminalProcess {
 	private terminalRef: WeakRef<RooTerminal>
 	private controller?: AbortController
+	private aborted = false
 
 	constructor(terminal: RooTerminal) {
 		super()
@@ -45,11 +46,15 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 			this.terminal.setActiveStream(stream, subprocess.pid)
 
 			for await (const line of stream) {
+				if (this.aborted) {
+					break
+				}
+
 				this.fullOutput += line
 
 				const now = Date.now()
 
-				if (this.isListening && (now - this.lastEmitTime_ms > 250 || this.lastEmitTime_ms === 0)) {
+				if (this.isListening && (now - this.lastEmitTime_ms > 500 || this.lastEmitTime_ms === 0)) {
 					this.emitRemainingBufferIfListening()
 					this.lastEmitTime_ms = now
 				}
@@ -57,6 +62,32 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 				this.startHotTimer(line)
 			}
 
+			if (this.aborted) {
+				let timeoutId: NodeJS.Timeout | undefined
+
+				const kill = new Promise<void>((resolve) => {
+					timeoutId = setTimeout(() => {
+						try {
+							subprocess.kill("SIGKILL")
+						} catch (e) {}
+
+						resolve()
+					}, 5_000)
+				})
+
+				try {
+					await Promise.race([subprocess, kill])
+				} catch (error) {
+					console.log(
+						`[ExecaTerminalProcess] subprocess termination error: ${error instanceof Error ? error.message : String(error)}`,
+					)
+				}
+
+				if (timeoutId) {
+					clearTimeout(timeoutId)
+				}
+			}
+
 			this.emit("shell_execution_complete", { exitCode: 0 })
 		} catch (error) {
 			if (error instanceof ExecaError) {
@@ -84,6 +115,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 	}
 
 	public override abort() {
+		this.aborted = true
 		this.controller?.abort()
 	}
 

+ 7 - 1
src/schemas/index.ts

@@ -293,8 +293,14 @@ export type CustomSupportPrompts = z.infer<typeof customSupportPromptsSchema>
 export const commandExecutionStatusSchema = z.discriminatedUnion("status", [
 	z.object({
 		executionId: z.string(),
-		status: z.literal("running"),
+		status: z.literal("started"),
 		pid: z.number().optional(),
+		command: z.string(),
+	}),
+	z.object({
+		executionId: z.string(),
+		status: z.literal("output"),
+		output: z.string(),
 	}),
 	z.object({
 		executionId: z.string(),

+ 0 - 32
src/shared/combineCommandSequences.ts

@@ -77,35 +77,3 @@ export function combineCommandSequences(messages: ClineMessage[]): ClineMessage[
 			return msg
 		})
 }
-
-export const splitCommandOutput = (text: string) => {
-	const outputIndex = text.indexOf(COMMAND_OUTPUT_STRING)
-
-	if (outputIndex === -1) {
-		return { command: text, output: "" }
-	}
-
-	return {
-		command: text.slice(0, outputIndex).trim(),
-
-		output: text
-			.slice(outputIndex + COMMAND_OUTPUT_STRING.length)
-			.trim()
-			.split("")
-			.map((char) => {
-				switch (char) {
-					case "\t":
-						return "→   "
-					case "\b":
-						return "⌫"
-					case "\f":
-						return "⏏"
-					case "\v":
-						return "⇳"
-					default:
-						return char
-				}
-			})
-			.join(""),
-	}
-}

+ 2 - 8
webview-ui/src/components/chat/ChatRow.tsx

@@ -5,7 +5,7 @@ import deepEqual from "fast-deep-equal"
 import { VSCodeBadge, VSCodeButton } from "@vscode/webview-ui-toolkit/react"
 
 import { ClineApiReqInfo, ClineAskUseMcpServer, ClineMessage, ClineSayTool } from "@roo/shared/ExtensionMessage"
-import { splitCommandOutput, COMMAND_OUTPUT_STRING } from "@roo/shared/combineCommandSequences"
+import { COMMAND_OUTPUT_STRING } from "@roo/shared/combineCommandSequences"
 import { safeJsonParse } from "@roo/shared/safeJsonParse"
 
 import { useCopyToClipboard } from "@src/utils/clipboard"
@@ -979,19 +979,13 @@ export const ChatRowContent = ({
 						</>
 					)
 				case "command":
-					const { command, output } = splitCommandOutput(message.text || "")
-
 					return (
 						<>
 							<div style={headerStyle}>
 								{icon}
 								{title}
 							</div>
-							<CommandExecution
-								executionId={message.progressStatus?.id}
-								command={command}
-								output={output}
-							/>
+							<CommandExecution executionId={message.progressStatus?.id} text={message.text} />
 						</>
 					)
 				case "use_mcp_server":

+ 112 - 89
webview-ui/src/components/chat/CommandExecution.tsx

@@ -1,4 +1,4 @@
-import { HTMLAttributes, forwardRef, useCallback, useMemo, useState } from "react"
+import { HTMLAttributes, useCallback, useEffect, useMemo, useState } from "react"
 import { useEvent } from "react-use"
 import { Virtuoso } from "react-virtuoso"
 import { ChevronDown, Skull } from "lucide-react"
@@ -6,6 +6,7 @@ import { ChevronDown, Skull } from "lucide-react"
 import { CommandExecutionStatus, commandExecutionStatusSchema } from "@roo/schemas"
 import { ExtensionMessage } from "@roo/shared/ExtensionMessage"
 import { safeJsonParse } from "@roo/shared/safeJsonParse"
+import { COMMAND_OUTPUT_STRING } from "@roo/shared/combineCommandSequences"
 
 import { vscode } from "@src/utils/vscode"
 import { useExtensionState } from "@src/context/ExtensionStateContext"
@@ -14,112 +15,134 @@ import { Button } from "@src/components/ui"
 
 interface CommandExecutionProps {
 	executionId?: string
-	command: string
-	output: string
+	text?: string
 }
 
-export const CommandExecution = forwardRef<HTMLDivElement, CommandExecutionProps>(
-	({ executionId, command, output }, ref) => {
-		const { terminalShellIntegrationDisabled = false } = useExtensionState()
+export const CommandExecution = ({ executionId, text }: CommandExecutionProps) => {
+	const { terminalShellIntegrationDisabled = false } = useExtensionState()
 
-		// If we aren't opening the VSCode terminal for this command then we default
-		// to expanding the command execution output.
-		const [isExpanded, setIsExpanded] = useState(terminalShellIntegrationDisabled)
+	// If we aren't opening the VSCode terminal for this command then we default
+	// to expanding the command execution output.
+	const [isExpanded, setIsExpanded] = useState(terminalShellIntegrationDisabled)
 
-		const [status, setStatus] = useState<CommandExecutionStatus | null>(null)
+	const [status, setStatus] = useState<CommandExecutionStatus | null>(null)
+	const [output, setOutput] = useState("")
+	const [command, setCommand] = useState("")
 
-		const lines = useMemo(
-			() => [`$ ${command}`, ...output.split("\n").filter((line) => line.trim() !== "")],
-			[command, output],
-		)
+	const lines = useMemo(
+		() => [`$ ${command}`, ...output.split("\n").filter((line) => line.trim() !== "")],
+		[output, command],
+	)
 
-		const onMessage = useCallback(
-			(event: MessageEvent) => {
-				if (!executionId) {
-					return
-				}
+	const onMessage = useCallback(
+		(event: MessageEvent) => {
+			if (!executionId) {
+				return
+			}
 
-				const message: ExtensionMessage = event.data
+			const message: ExtensionMessage = event.data
 
-				if (message.type === "commandExecutionStatus") {
-					const result = commandExecutionStatusSchema.safeParse(safeJsonParse(message.text, {}))
+			if (message.type === "commandExecutionStatus") {
+				const result = commandExecutionStatusSchema.safeParse(safeJsonParse(message.text, {}))
 
-					if (result.success) {
-						if (result.data.executionId !== executionId) {
-							return
-						}
+				if (result.success) {
+					const data = result.data
+
+					if (data.executionId !== executionId) {
+						return
+					}
 
-						if (result.data.status === "fallback") {
+					switch (data.status) {
+						case "started":
+							setCommand(data.command)
+							setStatus(data)
+							break
+						case "output":
+							setOutput((output) => output + data.output)
+							break
+						case "fallback":
 							setIsExpanded(true)
-						} else {
-							setStatus(result.data)
-						}
+							break
+						default:
+							setStatus(data)
+							break
 					}
 				}
-			},
-			[executionId],
-		)
-
-		useEvent("message", onMessage)
-
-		return (
-			<div ref={ref} className="w-full p-2 rounded-xs bg-vscode-editor-background">
-				<div className="flex flex-row items-center justify-between gap-2 px-1">
-					<Line className="text-sm whitespace-nowrap overflow-hidden text-ellipsis">{command}</Line>
-					<div className="flex flex-row items-center gap-1">
-						{status?.status === "running" && (
-							<div className="flex flex-row items-center gap-2 font-mono text-xs">
-								<div className="rounded-full size-1.5 bg-lime-400" />
-								<div>Running</div>
-								{status.pid && <div className="whitespace-nowrap">(PID: {status.pid})</div>}
-								<Button
-									variant="ghost"
-									size="icon"
-									onClick={() =>
-										vscode.postMessage({ type: "terminalOperation", terminalOperation: "abort" })
-									}>
-									<Skull />
-								</Button>
-							</div>
-						)}
-						{status?.status === "exited" && (
-							<div className="flex flex-row items-center gap-2 font-mono text-xs">
-								<div
-									className={cn(
-										"rounded-full size-1.5",
-										status.exitCode === 0 ? "bg-lime-400" : "bg-red-400",
-									)}
-								/>
-								<div className="whitespace-nowrap">Exited ({status.exitCode})</div>
-							</div>
-						)}
-						{lines.length > 0 && (
-							<Button variant="ghost" size="icon" onClick={() => setIsExpanded(!isExpanded)}>
-								<ChevronDown
-									className={cn("size-4 transition-transform duration-300", {
-										"rotate-180": isExpanded,
-									})}
-								/>
+			}
+		},
+		[executionId],
+	)
+
+	useEvent("message", onMessage)
+
+	useEffect(() => {
+		if (!status && text) {
+			const index = text.indexOf(COMMAND_OUTPUT_STRING)
+
+			if (index !== -1) {
+				setCommand(text.slice(0, index))
+				setOutput(text.slice(index + COMMAND_OUTPUT_STRING.length))
+			}
+		}
+	}, [status, text])
+
+	return (
+		<div className="w-full p-2 rounded-xs bg-vscode-editor-background">
+			<div className="flex flex-row items-center justify-between gap-2 px-1">
+				<Line className="text-sm whitespace-nowrap overflow-hidden text-ellipsis">{command}</Line>
+				<div className="flex flex-row items-center gap-1">
+					{status?.status === "started" && (
+						<div className="flex flex-row items-center gap-2 font-mono text-xs">
+							<div className="rounded-full size-1.5 bg-lime-400" />
+							<div>Running</div>
+							{status.pid && <div className="whitespace-nowrap">(PID: {status.pid})</div>}
+							<Button
+								variant="ghost"
+								size="icon"
+								onClick={() =>
+									vscode.postMessage({ type: "terminalOperation", terminalOperation: "abort" })
+								}>
+								<Skull />
 							</Button>
-						)}
-					</div>
-				</div>
-				<div
-					className={cn("mt-1 pt-1 border-t border-border/25", { hidden: !isExpanded })}
-					style={{ height: Math.min((lines.length + 1) * 16, 200) }}>
+						</div>
+					)}
+					{status?.status === "exited" && (
+						<div className="flex flex-row items-center gap-2 font-mono text-xs">
+							<div
+								className={cn(
+									"rounded-full size-1.5",
+									status.exitCode === 0 ? "bg-lime-400" : "bg-red-400",
+								)}
+							/>
+							<div className="whitespace-nowrap">Exited ({status.exitCode})</div>
+						</div>
+					)}
 					{lines.length > 0 && (
-						<Virtuoso
-							className="h-full"
-							totalCount={lines.length}
-							itemContent={(i) => <Line className="text-sm">{lines[i]}</Line>}
-							followOutput="auto"
-						/>
+						<Button variant="ghost" size="icon" onClick={() => setIsExpanded(!isExpanded)}>
+							<ChevronDown
+								className={cn("size-4 transition-transform duration-300", {
+									"rotate-180": isExpanded,
+								})}
+							/>
+						</Button>
 					)}
 				</div>
 			</div>
-		)
-	},
-)
+			<div
+				className={cn("mt-1 pt-1 border-t border-border/25", { hidden: !isExpanded })}
+				style={{ height: Math.min((lines.length + 1) * 16, 200) }}>
+				{lines.length > 0 && (
+					<Virtuoso
+						className="h-full"
+						totalCount={lines.length}
+						itemContent={(i) => <Line className="text-sm">{lines[i]}</Line>}
+						followOutput="auto"
+					/>
+				)}
+			</div>
+		</div>
+	)
+}
 
 type LineProps = HTMLAttributes<HTMLDivElement>