From 2e660819215ef06dfacad846381a7c637b603712 Mon Sep 17 00:00:00 2001 From: Chris Estreich Date: Fri, 2 May 2025 02:38:40 -0700 Subject: [PATCH] Terminal performance improvements (#3119) --- src/core/tools/executeCommandTool.ts | 9 +- .../terminal/ExecaTerminalProcess.ts | 34 ++- src/schemas/index.ts | 8 +- src/shared/combineCommandSequences.ts | 32 --- webview-ui/src/components/chat/ChatRow.tsx | 10 +- .../src/components/chat/CommandExecution.tsx | 197 ++++++++++-------- 6 files changed, 157 insertions(+), 133 deletions(-) diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index 579627997c..7db12668c8 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/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) => { diff --git a/src/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index 1f41fa082d..63b772e5db 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -6,6 +6,7 @@ import { BaseTerminalProcess } from "./BaseTerminalProcess" export class ExecaTerminalProcess extends BaseTerminalProcess { private terminalRef: WeakRef 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((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() } diff --git a/src/schemas/index.ts b/src/schemas/index.ts index 772832990d..ff8c01f0cf 100644 --- a/src/schemas/index.ts +++ b/src/schemas/index.ts @@ -293,8 +293,14 @@ export type CustomSupportPrompts = z.infer 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(), diff --git a/src/shared/combineCommandSequences.ts b/src/shared/combineCommandSequences.ts index 4e1d2d7bb4..dd171a77ec 100644 --- a/src/shared/combineCommandSequences.ts +++ b/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(""), - } -} diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index ea85bda79c..714c47ee5b 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/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 ( <>
{icon} {title}
- + ) case "use_mcp_server": diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index 7e2acdb492..2a5600d06b 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/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( - ({ 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(null) + const [status, setStatus] = useState(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 (result.data.status === "fallback") { + if (data.executionId !== executionId) { + return + } + + 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], - ) + } + }, + [executionId], + ) - useEvent("message", onMessage) + useEvent("message", onMessage) - return ( -
-
- {command} -
- {status?.status === "running" && ( -
-
-
Running
- {status.pid &&
(PID: {status.pid})
} - -
- )} - {status?.status === "exited" && ( -
-
-
Exited ({status.exitCode})
-
- )} - {lines.length > 0 && ( - - )} -
-
-
+
+ )} + {status?.status === "exited" && ( +
+
+
Exited ({status.exitCode})
+
+ )} {lines.length > 0 && ( - {lines[i]}} - followOutput="auto" - /> + )}
- ) - }, -) +
+ {lines.length > 0 && ( + {lines[i]}} + followOutput="auto" + /> + )} +
+
+ ) +} type LineProps = HTMLAttributes