From 070a36baa24a2b4bf84fb29ce3e9b561139e1248 Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Fri, 7 Mar 2025 20:10:02 -0800 Subject: [PATCH] Revert "Smart truncation for terminal output" This reverts commit 7eee3e08788c5b033af5262e223f306c51544c28. Middle-out truncation is a really great feature and it should still be implemented, however it unnecessarily interferes with #1365 because it hooked into the low-level chunk management that comes directly from VSCE shell integration. The best place to hook OutputBuilder is as follows depending on the state of terminal interaction: 1. Foreground terminals: Cline.ts: executeCommandTool(...) { process.on("line", (line) => { lines.push(line) ... } } 2. For background terminals: hook in at the point that getUnretrievedOutput is consumed for active or inactive terminals in Cline.ts:getEnvironmentDetails() Please note: 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 is a stable interface and minimize changes. Reverts: #1390 --- src/core/Cline.ts | 73 +++-- src/core/mentions/index.ts | 4 +- src/core/webview/ClineProvider.ts | 18 +- .../misc/__tests__/extract-text.test.ts | 66 ++++- src/integrations/misc/extract-text.ts | 34 +++ src/integrations/terminal/OutputBuilder.ts | 183 ------------ src/integrations/terminal/TerminalManager.ts | 23 +- src/integrations/terminal/TerminalProcess.ts | 249 ++++++++-------- .../terminal/__tests__/OutputBuilder.test.ts | 272 ------------------ .../__tests__/TerminalProcess.test.ts | 32 ++- .../terminal/__tests__/mergePromise.test.ts | 20 -- ...TerminalOutput.ts => get-latest-output.ts} | 0 src/integrations/terminal/mergePromise.ts | 23 -- src/shared/ExtensionMessage.ts | 2 +- src/shared/WebviewMessage.ts | 2 +- src/shared/globalState.ts | 2 +- src/shared/terminal.ts | 1 - src/utils/git.ts | 16 +- .../components/settings/AdvancedSettings.tsx | 26 +- .../settings/ExperimentalSettings.tsx | 2 +- .../src/components/settings/SettingsView.tsx | 9 +- .../src/context/ExtensionStateContext.tsx | 16 +- 22 files changed, 336 insertions(+), 737 deletions(-) delete mode 100644 src/integrations/terminal/OutputBuilder.ts delete mode 100644 src/integrations/terminal/__tests__/OutputBuilder.test.ts delete mode 100644 src/integrations/terminal/__tests__/mergePromise.test.ts rename src/integrations/terminal/{getLatestTerminalOutput.ts => get-latest-output.ts} (100%) delete mode 100644 src/integrations/terminal/mergePromise.ts delete mode 100644 src/shared/terminal.ts diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 9163b2a79e..ecc22a1fd9 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -25,6 +25,7 @@ import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers, + truncateOutput, } from "../integrations/misc/extract-text" import { TerminalManager, ExitCodeDetails } from "../integrations/terminal/TerminalManager" import { UrlContentFetcher } from "../services/browser/UrlContentFetcher" @@ -59,7 +60,7 @@ import { calculateApiCostAnthropic } from "../utils/cost" import { fileExistsAtPath } from "../utils/fs" import { arePathsEqual, getReadablePath } from "../utils/path" import { parseMentions } from "./mentions" -import { RooIgnoreController } from "./ignore/RooIgnoreController" +import { RooIgnoreController, LOCK_TEXT_SYMBOL } from "./ignore/RooIgnoreController" import { AssistantMessageContent, parseAssistantMessage, ToolParamName, ToolUseName } from "./assistant-message" import { formatResponse } from "./prompts/responses" import { SYSTEM_PROMPT } from "./prompts/system" @@ -70,7 +71,6 @@ import { BrowserSession } from "../services/browser/BrowserSession" import { McpHub } from "../services/mcp/McpHub" import crypto from "crypto" import { insertGroups } from "./diff/insert-groups" -import { OutputBuilder } from "../integrations/terminal/OutputBuilder" import { telemetryService } from "../services/telemetry/TelemetryService" const cwd = @@ -919,43 +919,30 @@ export class Cline { // Tools async executeCommandTool(command: string): Promise<[boolean, ToolResponse]> { - const { terminalOutputLimit } = (await this.providerRef.deref()?.getState()) ?? {} - const terminalInfo = await this.terminalManager.getOrCreateTerminal(cwd) - // Weird visual bug when creating new terminals (even manually) where - // there's an empty space at the top. - terminalInfo.terminal.show() - const process = this.terminalManager.runCommand(terminalInfo, command, terminalOutputLimit) + terminalInfo.terminal.show() // weird visual bug when creating new terminals (even manually) where there's an empty space at the top. + const process = this.terminalManager.runCommand(terminalInfo, command) let userFeedback: { text?: string; images?: string[] } | undefined let didContinue = false - - const sendCommandOutput = async (line: string) => { + const sendCommandOutput = async (line: string): Promise => { try { const { response, text, images } = await this.ask("command_output", line) - if (response === "yesButtonClicked") { - // Proceed while running. + // proceed while running } else { userFeedback = { text, images } } - didContinue = true - process.continue() // Continue past the await. + process.continue() // continue past the await } catch { - // This can only happen if this ask promise was ignored, so ignore this error. + // This can only happen if this ask promise was ignored, so ignore this error } } - let completed = false - let exitDetails: ExitCodeDetails | undefined - - let builder = new OutputBuilder({ maxSize: terminalOutputLimit }) - let output: string | undefined = undefined - + let lines: string[] = [] process.on("line", (line) => { - builder.append(line) - + lines.push(line) if (!didContinue) { sendCommandOutput(line) } else { @@ -963,8 +950,13 @@ export class Cline { } }) - process.once("completed", (buffer?: string) => { - output = buffer + let completed = false + let exitDetails: ExitCodeDetails | undefined + process.once("completed", (output?: string) => { + // Use provided output if available, otherwise keep existing result. + if (output) { + lines = output.split("\n") + } completed = true }) @@ -980,17 +972,19 @@ export class Cline { await process - // Wait for a short delay to ensure all messages are sent to the webview. + // Wait for a short delay to ensure all messages are sent to the webview // This delay allows time for non-awaited promises to be created and // for their associated messages to be sent to the webview, maintaining // the correct order of messages (although the webview is smart about - // grouping command_output messages despite any gaps anyways). + // grouping command_output messages despite any gaps anyways) await delay(50) - const result = output || builder.content + + const { terminalOutputLineLimit } = (await this.providerRef.deref()?.getState()) ?? {} + const output = truncateOutput(lines.join("\n"), terminalOutputLineLimit) + const result = output.trim() if (userFeedback) { await this.say("user_feedback", userFeedback.text, userFeedback.images) - return [ true, formatResponse.toolResult( @@ -1004,11 +998,9 @@ export class Cline { if (completed) { let exitStatus = "No exit code available" - if (exitDetails !== undefined) { if (exitDetails.signal) { exitStatus = `Process terminated by signal ${exitDetails.signal} (${exitDetails.signalName})` - if (exitDetails.coreDumpPossible) { exitStatus += " - core dump possible" } @@ -1016,16 +1008,15 @@ export class Cline { exitStatus = `Exit code: ${exitDetails.exitCode}` } } - return [false, `Command executed. ${exitStatus}${result.length > 0 ? `\nOutput:\n${result}` : ""}`] + } else { + return [ + false, + `Command is still running in the user's terminal.${ + result.length > 0 ? `\nHere's the output so far:\n${result}` : "" + }\n\nYou will be updated on the terminal status and new output in the future.`, + ] } - - return [ - false, - `Command is still running in the user's terminal.${ - result.length > 0 ? `\nHere's the output so far:\n${result}` : "" - }\n\nYou will be updated on the terminal status and new output in the future.`, - ] } async *attemptApiRequest(previousApiReqIndex: number, retryAttempt: number = 0): ApiStream { @@ -3525,7 +3516,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.readLine(busyTerminal.id) + const newOutput = this.terminalManager.getUnretrievedOutput(busyTerminal.id) if (newOutput) { terminalDetails += `\n### New Output\n${newOutput}` } else { @@ -3537,7 +3528,7 @@ export class Cline { if (inactiveTerminals.length > 0) { const inactiveTerminalOutputs = new Map() for (const inactiveTerminal of inactiveTerminals) { - const newOutput = this.terminalManager.readLine(inactiveTerminal.id) + const newOutput = this.terminalManager.getUnretrievedOutput(inactiveTerminal.id) if (newOutput) { inactiveTerminalOutputs.set(inactiveTerminal.id, newOutput) } diff --git a/src/core/mentions/index.ts b/src/core/mentions/index.ts index 445701c40f..e5f2785eba 100644 --- a/src/core/mentions/index.ts +++ b/src/core/mentions/index.ts @@ -2,13 +2,13 @@ import * as vscode from "vscode" import * as path from "path" import { openFile } from "../../integrations/misc/open-file" import { UrlContentFetcher } from "../../services/browser/UrlContentFetcher" -import { mentionRegexGlobal } from "../../shared/context-mentions" +import { mentionRegexGlobal, formatGitSuggestion, type MentionSuggestion } from "../../shared/context-mentions" import fs from "fs/promises" import { extractTextFromFile } from "../../integrations/misc/extract-text" import { isBinaryFile } from "isbinaryfile" import { diagnosticsToProblemsString } from "../../integrations/diagnostics" import { getCommitInfo, getWorkingState } from "../../utils/git" -import { getLatestTerminalOutput } from "../../integrations/terminal/getLatestTerminalOutput" +import { getLatestTerminalOutput } from "../../integrations/terminal/get-latest-output" export async function openMention(mention?: string): Promise { if (!mention) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index f98f19e4ff..3ebf8fe3fe 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -10,18 +10,17 @@ import simpleGit from "simple-git" import { setPanel } from "../../activate/registerCommands" import { ApiConfiguration, ApiProvider, ModelInfo, API_CONFIG_KEYS } from "../../shared/api" +import { CheckpointStorage } from "../../shared/checkpoints" import { findLast } from "../../shared/array" -import { supportPrompt } from "../../shared/support-prompt" +import { CustomSupportPrompts, supportPrompt } from "../../shared/support-prompt" import { GlobalFileNames } from "../../shared/globalFileNames" import { SecretKey, GlobalStateKey, SECRET_KEYS, GLOBAL_STATE_KEYS } from "../../shared/globalState" import { HistoryItem } from "../../shared/HistoryItem" -import { CheckpointStorage } from "../../shared/checkpoints" import { ApiConfigMeta, ExtensionMessage } from "../../shared/ExtensionMessage" import { checkoutDiffPayloadSchema, checkoutRestorePayloadSchema, WebviewMessage } from "../../shared/WebviewMessage" -import { Mode, PromptComponent, defaultModeSlug, ModeConfig } from "../../shared/modes" +import { Mode, CustomModePrompts, PromptComponent, defaultModeSlug, ModeConfig } from "../../shared/modes" import { checkExistKey } from "../../shared/checkExistApiConfig" import { EXPERIMENT_IDS, experiments as Experiments, experimentDefault, ExperimentId } from "../../shared/experiments" -import { TERMINAL_OUTPUT_LIMIT } from "../../shared/terminal" import { downloadTask } from "../../integrations/misc/export-markdown" import { openFile, openImage } from "../../integrations/misc/open-file" import { selectImages } from "../../integrations/misc/process-images" @@ -1255,6 +1254,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { await this.postStateToWebview() break case "checkpointStorage": + console.log(`[ClineProvider] checkpointStorage: ${message.text}`) const checkpointStorage = message.text ?? "task" await this.updateGlobalState("checkpointStorage", checkpointStorage) await this.postStateToWebview() @@ -1387,8 +1387,8 @@ export class ClineProvider implements vscode.WebviewViewProvider { await this.updateGlobalState("writeDelayMs", message.value) await this.postStateToWebview() break - case "terminalOutputLimit": - await this.updateGlobalState("terminalOutputLimit", message.value) + case "terminalOutputLineLimit": + await this.updateGlobalState("terminalOutputLineLimit", message.value) await this.postStateToWebview() break case "mode": @@ -2315,7 +2315,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { remoteBrowserEnabled, preferredLanguage, writeDelayMs, - terminalOutputLimit, + terminalOutputLineLimit, fuzzyMatchThreshold, mcpEnabled, enableMcpServerCreation, @@ -2375,7 +2375,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { remoteBrowserEnabled: remoteBrowserEnabled ?? false, preferredLanguage: preferredLanguage ?? "English", writeDelayMs: writeDelayMs ?? 1000, - terminalOutputLimit: terminalOutputLimit ?? TERMINAL_OUTPUT_LIMIT, + terminalOutputLineLimit: terminalOutputLineLimit ?? 500, fuzzyMatchThreshold: fuzzyMatchThreshold ?? 1.0, mcpEnabled: mcpEnabled ?? true, enableMcpServerCreation: enableMcpServerCreation ?? true, @@ -2530,7 +2530,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { remoteBrowserEnabled: stateValues.remoteBrowserEnabled ?? false, fuzzyMatchThreshold: stateValues.fuzzyMatchThreshold ?? 1.0, writeDelayMs: stateValues.writeDelayMs ?? 1000, - terminalOutputLimit: stateValues.terminalOutputLimit ?? TERMINAL_OUTPUT_LIMIT, + terminalOutputLineLimit: stateValues.terminalOutputLineLimit ?? 500, mode: stateValues.mode ?? defaultModeSlug, preferredLanguage: stateValues.preferredLanguage ?? diff --git a/src/integrations/misc/__tests__/extract-text.test.ts b/src/integrations/misc/__tests__/extract-text.test.ts index a8aacd9f34..7e084d010c 100644 --- a/src/integrations/misc/__tests__/extract-text.test.ts +++ b/src/integrations/misc/__tests__/extract-text.test.ts @@ -1,4 +1,4 @@ -import { addLineNumbers, everyLineHasLineNumbers, stripLineNumbers } from "../extract-text" +import { addLineNumbers, everyLineHasLineNumbers, stripLineNumbers, truncateOutput } from "../extract-text" describe("addLineNumbers", () => { it("should add line numbers starting from 1 by default", () => { @@ -101,3 +101,67 @@ describe("stripLineNumbers", () => { expect(stripLineNumbers(input)).toBe(expected) }) }) + +describe("truncateOutput", () => { + it("returns original content when no line limit provided", () => { + const content = "line1\nline2\nline3" + expect(truncateOutput(content)).toBe(content) + }) + + it("returns original content when lines are under limit", () => { + const content = "line1\nline2\nline3" + expect(truncateOutput(content, 5)).toBe(content) + }) + + it("truncates content with 20/80 split when over limit", () => { + // Create 25 lines of content + const lines = Array.from({ length: 25 }, (_, i) => `line${i + 1}`) + const content = lines.join("\n") + + // Set limit to 10 lines + const result = truncateOutput(content, 10) + + // Should keep: + // - First 2 lines (20% of 10) + // - Last 8 lines (80% of 10) + // - Omission indicator in between + const expectedLines = [ + "line1", + "line2", + "", + "[...15 lines omitted...]", + "", + "line18", + "line19", + "line20", + "line21", + "line22", + "line23", + "line24", + "line25", + ] + expect(result).toBe(expectedLines.join("\n")) + }) + + it("handles empty content", () => { + expect(truncateOutput("", 10)).toBe("") + }) + + it("handles single line content", () => { + expect(truncateOutput("single line", 10)).toBe("single line") + }) + + it("handles windows-style line endings", () => { + // Create content with windows line endings + const lines = Array.from({ length: 15 }, (_, i) => `line${i + 1}`) + const content = lines.join("\r\n") + + const result = truncateOutput(content, 5) + + // Should keep first line (20% of 5 = 1) and last 4 lines (80% of 5 = 4) + // Split result by either \r\n or \n to normalize line endings + const resultLines = result.split(/\r?\n/) + const expectedLines = ["line1", "", "[...10 lines omitted...]", "", "line12", "line13", "line14", "line15"] + expect(resultLines).toEqual(expectedLines) + }) +}) diff --git a/src/integrations/misc/extract-text.ts b/src/integrations/misc/extract-text.ts index 2ce90d7e38..0354570706 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -89,3 +89,37 @@ export function stripLineNumbers(content: string): string { const lineEnding = content.includes("\r\n") ? "\r\n" : "\n" return processedLines.join(lineEnding) } + +/** + * Truncates multi-line output while preserving context from both the beginning and end. + * When truncation is needed, it keeps 20% of the lines from the start and 80% from the end, + * with a clear indicator of how many lines were omitted in between. + * + * @param content The multi-line string to truncate + * @param lineLimit Optional maximum number of lines to keep. If not provided or 0, returns the original content + * @returns The truncated string with an indicator of omitted lines, or the original content if no truncation needed + * + * @example + * // With 10 line limit on 25 lines of content: + * // - Keeps first 2 lines (20% of 10) + * // - Keeps last 8 lines (80% of 10) + * // - Adds "[...15 lines omitted...]" in between + */ +export function truncateOutput(content: string, lineLimit?: number): string { + if (!lineLimit) { + return content + } + + const lines = content.split("\n") + if (lines.length <= lineLimit) { + return content + } + + const beforeLimit = Math.floor(lineLimit * 0.2) // 20% of lines before + const afterLimit = lineLimit - beforeLimit // remaining 80% after + return [ + ...lines.slice(0, beforeLimit), + `\n[...${lines.length - lineLimit} lines omitted...]\n`, + ...lines.slice(-afterLimit), + ].join("\n") +} diff --git a/src/integrations/terminal/OutputBuilder.ts b/src/integrations/terminal/OutputBuilder.ts deleted file mode 100644 index 4d7d922dff..0000000000 --- a/src/integrations/terminal/OutputBuilder.ts +++ /dev/null @@ -1,183 +0,0 @@ -import { TERMINAL_OUTPUT_LIMIT } from "../../shared/terminal" - -interface OutputBuilderOptions { - maxSize?: number // Max size of the buffer. - preserveStartPercent?: number // % of `maxSize` to preserve at start. - preserveEndPercent?: number // % of `maxSize` to preserve at end - truncationMessage?: string -} - -/** - * OutputBuilder manages terminal output with intelligent middle truncation. - * - * When output exceeds a specified size limit, this class truncates content - * primarily from the middle, preserving both the beginning (command context) - * and the end (recent output) of the buffer for better diagnostic context. - */ -export class OutputBuilder { - public readonly preserveStartSize: number - public readonly preserveEndSize: number - public readonly truncationMessage: string - - private startBuffer = "" - private endBuffer = "" - private _bytesProcessed = 0 - private _bytesRemoved = 0 - private _cursor = 0 - - constructor({ - maxSize = TERMINAL_OUTPUT_LIMIT, // 100KB - preserveStartPercent = 50, // 50% of `maxSize` - preserveEndPercent = 50, // 50% of `maxSize` - truncationMessage = "\n[... OUTPUT TRUNCATED ...]\n", - }: OutputBuilderOptions = {}) { - this.preserveStartSize = Math.floor((preserveStartPercent / 100) * maxSize) - this.preserveEndSize = Math.floor((preserveEndPercent / 100) * maxSize) - - if (this.preserveStartSize + this.preserveEndSize > maxSize) { - throw new Error("Invalid configuration: preserve sizes exceed maxSize") - } - - this.truncationMessage = truncationMessage - } - - append(content: string): this { - if (content.length === 0) { - return this - } - - this._bytesProcessed += content.length - - if (!this.isTruncated) { - this.startBuffer += content - - const excessBytes = this.startBuffer.length - (this.preserveStartSize + this.preserveEndSize) - - if (excessBytes <= 0) { - return this - } - - this.endBuffer = this.startBuffer.slice(-this.preserveEndSize) - this.startBuffer = this.startBuffer.slice(0, this.preserveStartSize) - this._bytesRemoved += excessBytes - } else { - // Already in truncation mode; append to `endBuffer`. - this.endBuffer += content - - // If `endBuffer` gets too large, trim it. - if (this.endBuffer.length > this.preserveEndSize) { - const excessBytes = this.endBuffer.length - this.preserveEndSize - this.endBuffer = this.endBuffer.slice(excessBytes) - this._bytesRemoved += excessBytes - } - } - - return this - } - - /** - * Reads unprocessed content from the current cursor position, handling both - * truncated and non-truncated states. - * - * The algorithm handles three cases: - * 1. Non-truncated buffer: - * - Simply returns remaining content from cursor position. - * - * 2. Truncated buffer, cursor in start portion: - * - Returns remaining start content plus all end content. - * - This ensures we don't miss the transition between buffers. - * - * 3. Truncated buffer, cursor in end portion: - * - Adjusts cursor position by subtracting removed bytes and start buffer length. - * - Uses Math.max to prevent negative indices if cursor adjustment overshoots. - * - Returns remaining content from adjusted position in end buffer. - * - * This approach ensures continuous reading even across truncation - * boundaries, while properly tracking position in both start and end - * portions of truncated content. - */ - read() { - let output - - if (!this.isTruncated) { - output = this.startBuffer.slice(this.cursor) - } else if (this.cursor < this.startBuffer.length) { - output = this.startBuffer.slice(this.cursor) + this.endBuffer - } else { - output = this.endBuffer.slice(Math.max(this.cursor - this.bytesRemoved - this.startBuffer.length, 0)) - } - - this._cursor = this.bytesProcessed - return output - } - - /** - * Same as above, but read only line at a time. - */ - readLine() { - let output - let index = -1 - - if (!this.isTruncated) { - output = this.startBuffer.slice(this.cursor) - index = output.indexOf("\n") - } else if (this.cursor < this.startBuffer.length) { - output = this.startBuffer.slice(this.cursor) - index = output.indexOf("\n") - - if (index === -1) { - output = output + this.endBuffer - index = output.indexOf("\n") - } - } else { - output = this.endBuffer.slice(Math.max(this.cursor - this.bytesRemoved - this.startBuffer.length, 0)) - index = output.indexOf("\n") - } - - if (index >= 0) { - this._cursor = this.bytesProcessed - (output.length - index) + 1 - return output.slice(0, index + 1) - } - - this._cursor = this.bytesProcessed - return output - } - - public reset(content?: string) { - this.startBuffer = "" - this.endBuffer = "" - this._bytesProcessed = 0 - this._bytesRemoved = 0 - this._cursor = 0 - - if (content) { - this.append(content) - } - } - - public get content() { - return this.isTruncated ? this.startBuffer + this.truncationMessage + this.endBuffer : this.startBuffer - } - - public get size() { - return this.isTruncated - ? this.startBuffer.length + this.truncationMessage.length + this.endBuffer.length - : this.startBuffer.length - } - - public get isTruncated() { - return this._bytesRemoved > 0 - } - - public get bytesProcessed() { - return this._bytesProcessed - } - - public get bytesRemoved() { - return this._bytesRemoved - } - - public get cursor() { - return this._cursor - } -} diff --git a/src/integrations/terminal/TerminalManager.ts b/src/integrations/terminal/TerminalManager.ts index 6dd0a57c46..a55f7867d4 100644 --- a/src/integrations/terminal/TerminalManager.ts +++ b/src/integrations/terminal/TerminalManager.ts @@ -1,11 +1,8 @@ import pWaitFor from "p-wait-for" import * as vscode from "vscode" - -import { TERMINAL_OUTPUT_LIMIT } from "../../shared/terminal" import { arePathsEqual } from "../../utils/path" -import { TerminalProcess } from "./TerminalProcess" +import { mergePromise, TerminalProcess, TerminalProcessResultPromise } from "./TerminalProcess" import { TerminalInfo, TerminalRegistry } from "./TerminalRegistry" -import { mergePromise, TerminalProcessResultPromise } from "./mergePromise" /* TerminalManager: @@ -18,6 +15,8 @@ TerminalProcess extends EventEmitter and implements Promise: - process.continue() resolves promise and stops event emission - Allows real-time output handling or background execution +getUnretrievedOutput() fetches latest output for ongoing commands + Enables flexible command execution: - Await for completion - Listen to real-time events @@ -31,6 +30,7 @@ Supported shells: Linux/macOS: bash, fish, pwsh, zsh Windows: pwsh + Example: const terminalManager = new TerminalManager(context); @@ -49,7 +49,7 @@ await process; process.continue(); // Later, if you need to get the unretrieved output: -const unretrievedOutput = terminalManager.readLine(terminalId); +const unretrievedOutput = terminalManager.getUnretrievedOutput(terminalId); console.log('Unretrieved output:', unretrievedOutput); Resources: @@ -259,14 +259,10 @@ export class TerminalManager { } } - runCommand( - terminalInfo: TerminalInfo, - command: string, - terminalOutputLimit = TERMINAL_OUTPUT_LIMIT, - ): TerminalProcessResultPromise { + runCommand(terminalInfo: TerminalInfo, command: string): TerminalProcessResultPromise { terminalInfo.busy = true terminalInfo.lastCommand = command - const process = new TerminalProcess(terminalOutputLimit) + const process = new TerminalProcess() this.processes.set(terminalInfo.id, process) process.once("completed", () => { @@ -351,13 +347,12 @@ export class TerminalManager { .map((t) => ({ id: t.id, lastCommand: t.lastCommand })) } - readLine(terminalId: number): string { + getUnretrievedOutput(terminalId: number): string { if (!this.terminalIds.has(terminalId)) { return "" } - const process = this.processes.get(terminalId) - return process ? process.readLine() : "" + return process ? process.getUnretrievedOutput() : "" } isProcessHot(terminalId: number): boolean { diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 047465e3fe..99ef215e78 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -5,34 +5,6 @@ import { inspect } from "util" import { ExitCodeDetails } from "./TerminalManager" import { TerminalInfo, TerminalRegistry } from "./TerminalRegistry" -import { OutputBuilder } from "./OutputBuilder" - -// How long to wait after a process outputs anything before we consider it -// "cool" again -const PROCESS_HOT_TIMEOUT_NORMAL = 2_000 -const PROCESS_HOT_TIMEOUT_COMPILING = 15_000 - -// 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. -const COMPILE_MARKERS = ["compiling", "building", "bundling", "transpiling", "generating", "starting"] - -const COMPILE_MARKER_NULLIFIERS = [ - "compiled", - "success", - "finish", - "complete", - "succeed", - "done", - "end", - "stop", - "exit", - "terminate", - "error", - "fail", -] - -const EMIT_INTERVAL = 250 export interface TerminalProcessEvents { line: [line: string] @@ -49,28 +21,20 @@ export interface TerminalProcessEvents { stream_available: [id: number, stream: AsyncIterable] } +// how long to wait after a process outputs anything before we consider it "cool" again +const PROCESS_HOT_TIMEOUT_NORMAL = 2_000 +const PROCESS_HOT_TIMEOUT_COMPILING = 15_000 + export class TerminalProcess extends EventEmitter { - public waitForShellIntegration = true - private _isHot = false - - private isListening = true + waitForShellIntegration: boolean = true + private isListening: boolean = true private terminalInfo: TerminalInfo | undefined - private lastEmitTime_ms = 0 - private outputBuilder?: OutputBuilder + private lastEmitTime_ms: number = 0 + private fullOutput: string = "" + private lastRetrievedIndex: number = 0 + isHot: boolean = false private hotTimer: NodeJS.Timeout | null = null - public get isHot() { - return this._isHot - } - - private set isHot(value: boolean) { - this._isHot = value - } - - constructor(private readonly terminalOutputLimit: number) { - super() - } - async run(terminal: vscode.Terminal, command: string) { if (terminal.shellIntegration && terminal.shellIntegration.executeCommand) { // Get terminal info to access stream @@ -102,7 +66,7 @@ export class TerminalProcess extends EventEmitter { }) }) - // readLine needs to know if streamClosed, so store this for later + // getUnretrievedOutput needs to know if streamClosed, so store this for later this.terminalInfo = terminalInfo // Execute command @@ -125,58 +89,61 @@ export class TerminalProcess extends EventEmitter { * - OSC 633 ; E ; [; ] ST - Explicitly set command line with optional nonce */ - this.outputBuilder = new OutputBuilder({ maxSize: this.terminalOutputLimit }) - - /** - * Some commands won't result in output flushing until the command - * completes. This locks the UI. Should we set a timer to prompt - * the user to continue? - */ - + // Process stream data for await (let data of stream) { - // Check for command output start marker. + // Check for command output start marker if (!commandOutputStarted) { preOutput += data const match = this.matchAfterVsceStartMarkers(data) - if (match !== undefined) { commandOutputStarted = true data = match - this.outputBuilder.reset() // Reset output when command actually starts. + this.fullOutput = "" // Reset fullOutput when command actually starts } else { continue } } // Command output started, accumulate data without filtering. - // Notice to future programmers: do not add escape sequence - // filtering here: output cannot change in length (see `readLine`), + // notice to future programmers: do not add escape sequence + // filtering here: fullOutput cannot change in length (see getUnretrievedOutput), // and chunks may not be complete so you cannot rely on detecting or removing escape sequences mid-stream. - this.outputBuilder.append(data) + this.fullOutput += data // For non-immediately returning commands we want to show loading spinner - // right away but this wouldn't happen until it emits a line break, so - // as soon as we get any output we emit to let webview know to show spinner. + // right away but this wouldnt happen until it emits a line break, so + // as soon as we get any output we emit to let webview know to show spinner const now = Date.now() - const timeSinceLastEmit = now - this.lastEmitTime_ms - - if (this.isListening && timeSinceLastEmit > EMIT_INTERVAL) { - this.flushLine() + if (this.isListening && (now - this.lastEmitTime_ms > 100 || this.lastEmitTime_ms === 0)) { + this.emitRemainingBufferIfListening() this.lastEmitTime_ms = now } - // Set isHot depending on the command. + // 2. Set isHot depending on the command. // This stalls API requests until terminal is cool again. this.isHot = true - if (this.hotTimer) { clearTimeout(this.hotTimer) } - + // 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 cline + const compilingMarkers = ["compiling", "building", "bundling", "transpiling", "generating", "starting"] + const markerNullifiers = [ + "compiled", + "success", + "finish", + "complete", + "succeed", + "done", + "end", + "stop", + "exit", + "terminate", + "error", + "fail", + ] const isCompiling = - COMPILE_MARKERS.some((marker) => data.toLowerCase().includes(marker.toLowerCase())) && - !COMPILE_MARKER_NULLIFIERS.some((nullifier) => data.toLowerCase().includes(nullifier.toLowerCase())) - + compilingMarkers.some((marker) => data.toLowerCase().includes(marker.toLowerCase())) && + !markerNullifiers.some((nullifier) => data.toLowerCase().includes(nullifier.toLowerCase())) this.hotTimer = setTimeout( () => { this.isHot = false @@ -185,18 +152,18 @@ export class TerminalProcess extends EventEmitter { ) } - // Set streamClosed immediately after stream ends. + // Set streamClosed immediately after stream ends if (this.terminalInfo) { this.terminalInfo.streamClosed = true } - // Wait for shell execution to complete and handle exit details. - await shellExecutionComplete + // Wait for shell execution to complete and handle exit details + const exitDetails = await shellExecutionComplete this.isHot = false if (commandOutputStarted) { - // Emit any remaining output before completing. - this.flushAll() + // Emit any remaining output before completing + this.emitRemainingBufferIfListening() } else { console.error( "[Terminal Process] VSCE output start escape sequence (]633;C or ]133;C) not received! VSCE Bug? preOutput: " + @@ -204,77 +171,62 @@ export class TerminalProcess extends EventEmitter { ) } - // Output begins after C marker so we only need to trim off D marker - // (if D exists, see VSCode bug# 237208): - const match = this.matchBeforeVsceEndMarkers(this.outputBuilder.content) + // console.debug("[Terminal Process] raw output: " + inspect(output, { colors: false, breakLength: Infinity })) + // fullOutput begins after C marker so we only need to trim off D marker + // (if D exists, see VSCode bug# 237208): + const match = this.matchBeforeVsceEndMarkers(this.fullOutput) if (match !== undefined) { - this.outputBuilder.reset(match) + this.fullOutput = match } - // For now we don't want this delaying requests since we don't send - // diagnostics automatically anymore (previous: "even though the - // command is finished, we still want to consider it 'hot' in case - // so that api request stalls to let diagnostics catch up"). + // console.debug(`[Terminal Process] processed output via ${matchSource}: ` + inspect(output, { colors: false, breakLength: Infinity })) + + // for now we don't want this delaying requests since we don't send diagnostics automatically anymore (previous: "even though the command is finished, we still want to consider it 'hot' in case so that api request stalls to let diagnostics catch up") if (this.hotTimer) { clearTimeout(this.hotTimer) } - this.isHot = false - this.emit("completed", this.removeEscapeSequences(this.outputBuilder.content)) + this.emit("completed", this.removeEscapeSequences(this.fullOutput)) this.emit("continue") } else { terminal.sendText(command, true) - // For terminals without shell integration, we can't know when the command completes. - // So we'll just emit the continue event. + // For terminals without shell integration, we can't know when the command completes + // So we'll just emit the continue event after a delay this.emit("completed") this.emit("continue") this.emit("no_shell_integration") + // setTimeout(() => { + // console.log(`Emitting continue after delay for terminal`) + // // can't emit completed since we don't if the command actually completed, it could still be running server + // }, 500) // Adjust this delay as needed } } - public readLine() { - return this.processOutput(this.outputBuilder?.readLine() || "") + private emitRemainingBufferIfListening() { + if (this.isListening) { + const remainingBuffer = this.getUnretrievedOutput() + if (remainingBuffer !== "") { + this.emit("line", remainingBuffer) + } + } } - public read() { - return this.processOutput(this.outputBuilder?.read() || "") - } - - public continue() { - this.flushAll() + continue() { + this.emitRemainingBufferIfListening() this.isListening = false this.removeAllListeners("line") this.emit("continue") } - private flushLine() { - if (!this.isListening) { - return - } + // Returns complete lines with their carriage returns. + // The final line may lack a carriage return if the program didn't send one. + getUnretrievedOutput(): string { + // Get raw unretrieved output + let outputToProcess = this.fullOutput.slice(this.lastRetrievedIndex) - const line = this.readLine() - - if (line) { - this.emit("line", line) - } - } - - private flushAll() { - if (!this.isListening) { - return - } - - const buffer = this.read() - - if (buffer) { - this.emit("line", buffer) - } - } - - private processOutput(outputToProcess: string) { - // Check for VSCE command end markers. + // Check for VSCE command end markers const index633 = outputToProcess.indexOf("\x1b]633;D") const index133 = outputToProcess.indexOf("\x1b]133;D") let endIndex = -1 @@ -287,7 +239,32 @@ export class TerminalProcess extends EventEmitter { endIndex = index133 } - return this.removeEscapeSequences(endIndex >= 0 ? outputToProcess.slice(0, endIndex) : outputToProcess) + // If no end markers were found yet (possibly due to VSCode bug#237208): + // 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) { + // Stream still running - only process complete lines + endIndex = outputToProcess.lastIndexOf("\n") + if (endIndex === -1) { + // No complete lines + return "" + } + + // Include carriage return + endIndex++ + } else { + // Stream closed - process all remaining output + endIndex = outputToProcess.length + } + } + + // Update index and slice output + this.lastRetrievedIndex += endIndex + outputToProcess = outputToProcess.slice(0, endIndex) + + // Clean and return output + return this.removeEscapeSequences(outputToProcess) } private stringIndexMatch( @@ -305,20 +282,18 @@ export class TerminalProcess extends EventEmitter { prefixLength = 0 } else { startIndex = data.indexOf(prefix) - if (startIndex === -1) { return undefined } - if (bell.length > 0) { // Find the bell character after the prefix const bellIndex = data.indexOf(bell, startIndex + prefix.length) - if (bellIndex === -1) { return undefined } const distanceToBell = bellIndex - startIndex + prefixLength = distanceToBell + bell.length } else { prefixLength = prefix.length @@ -332,7 +307,6 @@ export class TerminalProcess extends EventEmitter { endIndex = data.length } else { endIndex = data.indexOf(suffix, contentStart) - if (endIndex === -1) { return undefined } @@ -349,7 +323,7 @@ export class TerminalProcess extends EventEmitter { // This method could be extended to handle other escape sequences, but any additions // should be carefully considered to ensure they only remove control codes and don't // alter the actual content or behavior of the output stream. - private removeEscapeSequences(str: string) { + private removeEscapeSequences(str: string): string { return stripAnsi(str.replace(/\x1b\]633;[^\x07]+\x07/gs, "").replace(/\x1b\]133;[^\x07]+\x07/gs, "")) } @@ -422,3 +396,20 @@ export class TerminalProcess extends EventEmitter { return match133 !== undefined ? match133 : match633 } } + +export type TerminalProcessResultPromise = TerminalProcess & Promise + +// Similar to execa's ResultPromise, this lets us create a mixin of both a TerminalProcess and a Promise: https://github.com/sindresorhus/execa/blob/main/lib/methods/promise.js +export function mergePromise(process: TerminalProcess, promise: Promise): TerminalProcessResultPromise { + const nativePromisePrototype = (async () => {})().constructor.prototype + const descriptors = ["then", "catch", "finally"].map( + (property) => [property, Reflect.getOwnPropertyDescriptor(nativePromisePrototype, property)] as const, + ) + for (const [property, descriptor] of descriptors) { + if (descriptor) { + const value = descriptor.value.bind(promise) + Reflect.defineProperty(process, property, { ...descriptor, value }) + } + } + return process as TerminalProcessResultPromise +} diff --git a/src/integrations/terminal/__tests__/OutputBuilder.test.ts b/src/integrations/terminal/__tests__/OutputBuilder.test.ts deleted file mode 100644 index acb228053a..0000000000 --- a/src/integrations/terminal/__tests__/OutputBuilder.test.ts +++ /dev/null @@ -1,272 +0,0 @@ -// npx jest src/integrations/terminal/__tests__/OutputBuilder.test.ts - -import { OutputBuilder } from "../OutputBuilder" - -describe("OutputBuilder", () => { - describe("basic functionality", () => { - it("should create instance with default settings", () => { - const builder = new OutputBuilder() - expect(builder).toBeInstanceOf(OutputBuilder) - expect(builder.content).toBe("") - expect(builder.isTruncated).toBe(false) - expect(builder.size).toBe(0) - }) - - it("should append and retrieve content", () => { - const builder = new OutputBuilder() - builder.append("Hello, ") - builder.append("world!") - - expect(builder.content).toBe("Hello, world!") - expect(builder.isTruncated).toBe(false) - expect(builder.size).toBe(13) - }) - - it("should reset content properly", () => { - const builder = new OutputBuilder() - builder.append("Hello, world!") - builder.reset() - - expect(builder.content).toBe("") - expect(builder.isTruncated).toBe(false) - expect(builder.size).toBe(0) - }) - }) - - describe("truncation behavior", () => { - it("should not truncate content below max size", () => { - // Create with 100 byte limit. - const builder = new OutputBuilder({ - maxSize: 100, - preserveStartPercent: 20, - preserveEndPercent: 80, - }) - - // Add 50 bytes of content. - builder.append("a".repeat(50)) - - expect(builder.content).toBe("a".repeat(50)) - expect(builder.isTruncated).toBe(false) - expect(builder.size).toBe(50) - }) - - it("should truncate content correctly when exceeding max size", () => { - // Small buffer for testing - const maxSize = 100 - const truncationMessage = "[...TRUNCATED...]" - const builder = new OutputBuilder({ - maxSize, - preserveStartPercent: 20, - preserveEndPercent: 80, - truncationMessage, - }) - - // Calculate preserve sizes. - const preserveStartSize = Math.floor(0.2 * maxSize) // 20 bytes - const preserveEndSize = Math.floor(0.8 * maxSize) // 80 bytes - - // Add content that exceeds the 100 byte limit. - builder.append("a".repeat(120)) - - // Check truncation happened. - expect(builder.isTruncated).toBe(true) - - // Verify content structure. - const content = builder.content - - // Should have this structure: - // [start 20 chars] + [truncation message] + [end 80 chars] - expect(content).toBe("a".repeat(preserveStartSize) + truncationMessage + "a".repeat(preserveEndSize)) - - // Size should be: startSize + truncationMessage.length + endSize - expect(builder.size).toBe(preserveStartSize + truncationMessage.length + preserveEndSize) - }) - - it("should preserve start and end with different percentages", () => { - // Small buffer with 50/50 split. - const builder = new OutputBuilder({ - maxSize: 100, - preserveStartPercent: 50, - preserveEndPercent: 50, - truncationMessage: "[...]", - }) - - // Add 200 bytes. - builder.append("a".repeat(200)) - - // Should preserve 50 at start, 50 at end. - expect(builder.content).toBe("a".repeat(50) + "[...]" + "a".repeat(50)) - expect(builder.isTruncated).toBe(true) - }) - - it("should handle multiple content additions after truncation", () => { - const builder = new OutputBuilder({ - maxSize: 100, - preserveStartPercent: 30, - preserveEndPercent: 70, - truncationMessage: "[...]", - }) - - // Initial content that triggers truncation. - builder.append("a".repeat(120)) - expect(builder.isTruncated).toBe(true) - - // Add more content - should update end portion. - builder.append("b".repeat(20)) - - // Should contain start (a's), truncation message, and end with both a's and b's. - const content = builder.content - expect(content.startsWith("a".repeat(30))).toBe(true) - expect(content.indexOf("[...]")).toBe(30) - expect(content.endsWith("b".repeat(20))).toBe(true) - }) - }) - - describe("edge cases", () => { - it("should handle empty string appends", () => { - const builder = new OutputBuilder({ maxSize: 100 }) - builder.append("") - expect(builder.content).toBe("") - expect(builder.size).toBe(0) - }) - - it("should handle content exactly at size limit", () => { - const builder = new OutputBuilder({ maxSize: 100 }) - builder.append("a".repeat(100)) - - // Should not trigger truncation at exactly the limit. - expect(builder.isTruncated).toBe(false) - expect(builder.size).toBe(100) - }) - - it("should handle very small max sizes", () => { - // 10 byte max with 3 byte start, 7 byte end. - const builder = new OutputBuilder({ - maxSize: 10, - preserveStartPercent: 30, - preserveEndPercent: 70, - truncationMessage: "...", - }) - - builder.append("1234567890abc") - - // Get result and validate structure (start + message + end). - const result = builder.content - expect(result.startsWith("123")).toBe(true) - expect(result.indexOf("...")).toBe(3) - - // For small buffers, there might be differences in exact content - // based on implementation details. - // But the combined length should be correct: - // startSize(3) + message(3) + endSize(7) = 13 - expect(result.length).toBe(13) - }) - - it("should throw error for invalid configuration", () => { - // Preserve percentages that add up to more than 100%. - expect(() => { - new OutputBuilder({ - maxSize: 100, - preserveStartPercent: 60, - preserveEndPercent: 60, - }) - }).toThrow() - }) - - it("should handle continuous appending beyond multiple truncations", () => { - // Small buffer for testing multiple truncations. - const builder = new OutputBuilder({ - maxSize: 20, - preserveStartPercent: 25, // 5 bytes - preserveEndPercent: 75, // 15 bytes - truncationMessage: "...", - }) - - // First append - triggers truncation. - builder.append("a".repeat(30)) - expect(builder.isTruncated).toBe(true) - expect(builder.content).toBe("a".repeat(5) + "..." + "a".repeat(15)) - - // Second append with different character. - builder.append("b".repeat(10)) - - // Should maintain start buffer, but end buffer should now have some b's. - const expectedEndBuffer = "a".repeat(5) + "b".repeat(10) - expect(builder.content).toBe("a".repeat(5) + "..." + expectedEndBuffer) - - // Third append with another character. - builder.append("c".repeat(5)) - - // End buffer should shift again. - const finalEndBuffer = "a".repeat(0) + "b".repeat(10) + "c".repeat(5) - expect(builder.content).toBe("a".repeat(5) + "..." + finalEndBuffer) - }) - }) - - describe("read", () => { - it("handles truncated output", () => { - const builder = new OutputBuilder({ - maxSize: 60, - preserveStartPercent: 40, - preserveEndPercent: 60, - truncationMessage: " ... ", - }) - - builder.append("Beginning content that will partially remain. ") - expect(builder.content).toBe("Beginning content that will partially remain. ") - expect(builder.bytesProcessed).toBe(46) - expect(builder.bytesRemoved).toBe(0) - expect(builder.read()).toBe("Beginning content that will partially remain. ") - expect(builder.cursor).toBe(46) - - builder.append("Ending content that will remain until another append. ") - expect(builder.content).toBe("Beginning content that w ... t will remain until another append. ") - expect(builder.bytesProcessed).toBe(100) - expect(builder.bytesRemoved).toBe(40) - expect(builder.read()).toBe("t will remain until another append. ") - expect(builder.cursor).toBe(100) - - builder.append("Fin. ") - expect(builder.content).toBe("Beginning content that w ... l remain until another append. Fin. ") - expect(builder.bytesProcessed).toBe(105) - expect(builder.bytesRemoved).toBe(45) - expect(builder.read()).toBe("Fin. ") - expect(builder.cursor).toBe(105) - - builder.append("Foo bar baz. ") - expect(builder.content).toBe("Beginning content that w ... l another append. Fin. Foo bar baz. ") - expect(builder.bytesProcessed).toBe(118) - expect(builder.bytesRemoved).toBe(58) - expect(builder.read()).toBe("Foo bar baz. ") - expect(builder.cursor).toBe(118) - - builder.append("Lorem ipsum dolor sit amet, libris convenire vix ei, ea cum aperiam liberavisse. ") - expect(builder.content).toBe("Beginning content that w ... vix ei, ea cum aperiam liberavisse. ") - expect(builder.bytesProcessed).toBe(199) - expect(builder.bytesRemoved).toBe(139) - expect(builder.read()).toBe("vix ei, ea cum aperiam liberavisse. ") - expect(builder.cursor).toBe(199) - }) - }) - - describe("readLine", () => { - it("handles truncated output", () => { - const builder = new OutputBuilder({ - maxSize: 60, - preserveStartPercent: 40, - preserveEndPercent: 60, - truncationMessage: " ... ", - }) - - builder.append("Lorem ipsum dolor sit amet.\nLibris convenire vix ei.") - expect(builder.content).toBe("Lorem ipsum dolor sit amet.\nLibris convenire vix ei.") - expect(builder.readLine()).toBe("Lorem ipsum dolor sit amet.\n") - expect(builder.readLine()).toBe("Libris convenire vix ei.") - - builder.append("Est aliqua quis aliqua.\nAliquip culpa id cillum enim.") - expect(builder.content).toBe("Lorem ipsum dolor sit am ... liqua.\nAliquip culpa id cillum enim.") - expect(builder.readLine()).toBe("liqua.\n") - expect(builder.readLine()).toBe("Aliquip culpa id cillum enim.") - }) - }) -}) diff --git a/src/integrations/terminal/__tests__/TerminalProcess.test.ts b/src/integrations/terminal/__tests__/TerminalProcess.test.ts index c90c517e3e..44cae92580 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.test.ts @@ -2,7 +2,7 @@ import * as vscode from "vscode" -import { TerminalProcess } from "../TerminalProcess" +import { TerminalProcess, mergePromise } from "../TerminalProcess" import { TerminalInfo, TerminalRegistry } from "../TerminalRegistry" // Mock vscode.window.createTerminal @@ -34,7 +34,7 @@ describe("TerminalProcess", () => { let mockStream: AsyncIterableIterator beforeEach(() => { - terminalProcess = new TerminalProcess(100 * 1024) + terminalProcess = new TerminalProcess() // Create properly typed mock terminal mockTerminal = { @@ -173,4 +173,32 @@ describe("TerminalProcess", () => { expect(terminalProcess["isListening"]).toBe(false) }) }) + + describe("getUnretrievedOutput", () => { + it("returns and clears unretrieved output", () => { + terminalProcess["fullOutput"] = `\x1b]633;C\x07previous\nnew output\x1b]633;D\x07` + terminalProcess["lastRetrievedIndex"] = 17 // After "previous\n" + + const unretrieved = terminalProcess.getUnretrievedOutput() + expect(unretrieved).toBe("new output") + + expect(terminalProcess["lastRetrievedIndex"]).toBe(terminalProcess["fullOutput"].length - "previous".length) + }) + }) + + describe("mergePromise", () => { + it("merges promise methods with terminal process", async () => { + const process = new TerminalProcess() + const promise = Promise.resolve() + + const merged = mergePromise(process, promise) + + expect(merged).toHaveProperty("then") + expect(merged).toHaveProperty("catch") + expect(merged).toHaveProperty("finally") + expect(merged instanceof TerminalProcess).toBe(true) + + await expect(merged).resolves.toBeUndefined() + }) + }) }) diff --git a/src/integrations/terminal/__tests__/mergePromise.test.ts b/src/integrations/terminal/__tests__/mergePromise.test.ts deleted file mode 100644 index 1b2b60d179..0000000000 --- a/src/integrations/terminal/__tests__/mergePromise.test.ts +++ /dev/null @@ -1,20 +0,0 @@ -// npx jest src/integrations/terminal/__tests__/mergePromise.test.ts - -import { TerminalProcess } from "../TerminalProcess" -import { mergePromise } from "../mergePromise" - -describe("mergePromise", () => { - it("merges promise methods with terminal process", async () => { - const process = new TerminalProcess(100 * 1024) - const promise = Promise.resolve() - - const merged = mergePromise(process, promise) - - expect(merged).toHaveProperty("then") - expect(merged).toHaveProperty("catch") - expect(merged).toHaveProperty("finally") - expect(merged instanceof TerminalProcess).toBe(true) - - await expect(merged).resolves.toBeUndefined() - }) -}) diff --git a/src/integrations/terminal/getLatestTerminalOutput.ts b/src/integrations/terminal/get-latest-output.ts similarity index 100% rename from src/integrations/terminal/getLatestTerminalOutput.ts rename to src/integrations/terminal/get-latest-output.ts diff --git a/src/integrations/terminal/mergePromise.ts b/src/integrations/terminal/mergePromise.ts deleted file mode 100644 index d1a1f45329..0000000000 --- a/src/integrations/terminal/mergePromise.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { TerminalProcess } from "./TerminalProcess" - -export type TerminalProcessResultPromise = TerminalProcess & Promise - -// Similar to execa's ResultPromise, this lets us create a mixin of both a -// TerminalProcess and a Promise: -// https://github.com/sindresorhus/execa/blob/main/lib/methods/promise.js -export function mergePromise(process: TerminalProcess, promise: Promise): TerminalProcessResultPromise { - const nativePromisePrototype = (async () => {})().constructor.prototype - - const descriptors = ["then", "catch", "finally"].map( - (property) => [property, Reflect.getOwnPropertyDescriptor(nativePromisePrototype, property)] as const, - ) - - for (const [property, descriptor] of descriptors) { - if (descriptor) { - const value = descriptor.value.bind(promise) - Reflect.defineProperty(process, property, { ...descriptor, value }) - } - } - - return process as TerminalProcessResultPromise -} diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 33c485adff..b42b1a502e 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -133,7 +133,7 @@ export interface ExtensionState { fuzzyMatchThreshold?: number preferredLanguage: string writeDelayMs: number - terminalOutputLimit?: number + terminalOutputLineLimit?: number mcpEnabled: boolean enableMcpServerCreation: boolean enableCustomModeCreation?: boolean diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index dc2ddd6c77..71bd516c29 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -70,7 +70,7 @@ export interface WebviewMessage { | "enhancedPrompt" | "draggedImages" | "deleteMessage" - | "terminalOutputLimit" + | "terminalOutputLineLimit" | "mcpEnabled" | "enableMcpServerCreation" | "enableCustomModeCreation" diff --git a/src/shared/globalState.ts b/src/shared/globalState.ts index 40973b3350..d03f5f20a4 100644 --- a/src/shared/globalState.ts +++ b/src/shared/globalState.ts @@ -71,7 +71,7 @@ export const GLOBAL_STATE_KEYS = [ "fuzzyMatchThreshold", "preferredLanguage", // Language setting for Cline's communication "writeDelayMs", - "terminalOutputLimit", + "terminalOutputLineLimit", "mcpEnabled", "enableMcpServerCreation", "alwaysApproveResubmit", diff --git a/src/shared/terminal.ts b/src/shared/terminal.ts deleted file mode 100644 index 4747f8727f..0000000000 --- a/src/shared/terminal.ts +++ /dev/null @@ -1 +0,0 @@ -export const TERMINAL_OUTPUT_LIMIT = 100 * 1024 diff --git a/src/utils/git.ts b/src/utils/git.ts index 16d6240ac1..640af7fd29 100644 --- a/src/utils/git.ts +++ b/src/utils/git.ts @@ -1,9 +1,9 @@ import { exec } from "child_process" import { promisify } from "util" - -import { OutputBuilder } from "../integrations/terminal/OutputBuilder" +import { truncateOutput } from "../integrations/misc/extract-text" const execAsync = promisify(exec) +const GIT_OUTPUT_LINE_LIMIT = 500 export interface GitCommit { hash: string @@ -122,9 +122,8 @@ export async function getCommitInfo(hash: string, cwd: string): Promise "\nFull Changes:", ].join("\n") - const builder = new OutputBuilder() - builder.append(summary + "\n\n" + diff.trim()) - return builder.content + const output = summary + "\n\n" + diff.trim() + return truncateOutput(output, GIT_OUTPUT_LINE_LIMIT) } catch (error) { console.error("Error getting commit info:", error) return `Failed to get commit info: ${error instanceof Error ? error.message : String(error)}` @@ -151,10 +150,9 @@ export async function getWorkingState(cwd: string): Promise { // Get all changes (both staged and unstaged) compared to HEAD const { stdout: diff } = await execAsync("git diff HEAD", { cwd }) - - const builder = new OutputBuilder() - builder.append(`Working directory changes:\n\n${status}\n\n${diff}`.trim()) - return builder.content + const lineLimit = GIT_OUTPUT_LINE_LIMIT + const output = `Working directory changes:\n\n${status}\n\n${diff}`.trim() + return truncateOutput(output, lineLimit) } catch (error) { console.error("Error getting working state:", error) return `Failed to get working state: ${error instanceof Error ? error.message : String(error)}` diff --git a/webview-ui/src/components/settings/AdvancedSettings.tsx b/webview-ui/src/components/settings/AdvancedSettings.tsx index f20e9a7c33..c04f28f88e 100644 --- a/webview-ui/src/components/settings/AdvancedSettings.tsx +++ b/webview-ui/src/components/settings/AdvancedSettings.tsx @@ -3,7 +3,6 @@ import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react" import { Cog } from "lucide-react" import { EXPERIMENT_IDS, ExperimentId } from "../../../../src/shared/experiments" -import { TERMINAL_OUTPUT_LIMIT } from "../../../../src/shared/terminal" import { cn } from "@/lib/utils" @@ -14,14 +13,14 @@ import { Section } from "./Section" type AdvancedSettingsProps = HTMLAttributes & { rateLimitSeconds: number - terminalOutputLimit?: number + terminalOutputLineLimit?: number maxOpenTabsContext: number diffEnabled?: boolean fuzzyMatchThreshold?: number showRooIgnoredFiles?: boolean setCachedStateField: SetCachedStateField< | "rateLimitSeconds" - | "terminalOutputLimit" + | "terminalOutputLineLimit" | "maxOpenTabsContext" | "diffEnabled" | "fuzzyMatchThreshold" @@ -32,7 +31,7 @@ type AdvancedSettingsProps = HTMLAttributes & { } export const AdvancedSettings = ({ rateLimitSeconds, - terminalOutputLimit = TERMINAL_OUTPUT_LIMIT, + terminalOutputLineLimit, maxOpenTabsContext, diffEnabled, fuzzyMatchThreshold, @@ -78,20 +77,21 @@ export const AdvancedSettings = ({
setCachedStateField("terminalOutputLimit", parseInt(e.target.value))} + min="100" + max="5000" + step="100" + value={terminalOutputLineLimit ?? 500} + onChange={(e) => + setCachedStateField("terminalOutputLineLimit", parseInt(e.target.value)) + } className="h-2 focus:outline-0 w-4/5 accent-vscode-button-background" /> - {Math.floor(terminalOutputLimit / 1024)} KB + {terminalOutputLineLimit ?? 500}

- Maximum amount of terminal output (in kilobytes) to send to the LLM when executing commands. If - the output exceeds this limit, it will be removed from the middle so that the start and end of - the output are preserved. + Maximum number of lines to include in terminal output when executing commands. When exceeded + lines will be removed from the middle, saving tokens.

diff --git a/webview-ui/src/components/settings/ExperimentalSettings.tsx b/webview-ui/src/components/settings/ExperimentalSettings.tsx index d39c52e9c4..bbdfe47e89 100644 --- a/webview-ui/src/components/settings/ExperimentalSettings.tsx +++ b/webview-ui/src/components/settings/ExperimentalSettings.tsx @@ -12,7 +12,7 @@ import { ExperimentalFeature } from "./ExperimentalFeature" type ExperimentalSettingsProps = HTMLAttributes & { setCachedStateField: SetCachedStateField< - "rateLimitSeconds" | "terminalOutputLimit" | "maxOpenTabsContext" | "diffEnabled" | "fuzzyMatchThreshold" + "rateLimitSeconds" | "terminalOutputLineLimit" | "maxOpenTabsContext" | "diffEnabled" | "fuzzyMatchThreshold" > experiments: Record setExperimentEnabled: SetExperimentEnabled diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index b2e3cb26c4..d49ed2d03a 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -11,10 +11,9 @@ import { AlertTriangle, } from "lucide-react" -import { ApiConfiguration } from "../../../../src/shared/api" import { ExperimentId } from "../../../../src/shared/experiments" -import { TERMINAL_OUTPUT_LIMIT } from "../../../../src/shared/terminal" import { TelemetrySetting } from "../../../../src/shared/TelemetrySetting" +import { ApiConfiguration } from "../../../../src/shared/api" import { vscode } from "@/utils/vscode" import { ExtensionStateContextType, useExtensionState } from "@/context/ExtensionStateContext" @@ -92,7 +91,7 @@ const SettingsView = forwardRef(({ onDone }, soundEnabled, soundVolume, telemetrySetting, - terminalOutputLimit, + terminalOutputLineLimit, writeDelayMs, showRooIgnoredFiles, remoteBrowserEnabled, @@ -189,7 +188,7 @@ const SettingsView = forwardRef(({ onDone }, vscode.postMessage({ type: "fuzzyMatchThreshold", value: fuzzyMatchThreshold ?? 1.0 }) vscode.postMessage({ type: "writeDelayMs", value: writeDelayMs }) vscode.postMessage({ type: "screenshotQuality", value: screenshotQuality ?? 75 }) - vscode.postMessage({ type: "terminalOutputLimit", value: terminalOutputLimit ?? TERMINAL_OUTPUT_LIMIT }) + vscode.postMessage({ type: "terminalOutputLineLimit", value: terminalOutputLineLimit ?? 500 }) vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled }) vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit }) vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds }) @@ -405,7 +404,7 @@ const SettingsView = forwardRef(({ onDone },
void screenshotQuality?: number setScreenshotQuality: (value: number) => void - terminalOutputLimit?: number - setTerminalOutputLimit: (value: number) => void + terminalOutputLineLimit?: number + setTerminalOutputLineLimit: (value: number) => void mcpEnabled: boolean setMcpEnabled: (value: boolean) => void enableMcpServerCreation: boolean @@ -126,7 +123,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode writeDelayMs: 1000, browserViewportSize: "900x600", screenshotQuality: 75, - terminalOutputLimit: TERMINAL_OUTPUT_LIMIT, + terminalOutputLineLimit: 500, mcpEnabled: true, enableMcpServerCreation: true, alwaysApproveResubmit: false, @@ -266,7 +263,8 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode setPreferredLanguage: (value) => setState((prevState) => ({ ...prevState, preferredLanguage: value })), setWriteDelayMs: (value) => setState((prevState) => ({ ...prevState, writeDelayMs: value })), setScreenshotQuality: (value) => setState((prevState) => ({ ...prevState, screenshotQuality: value })), - setTerminalOutputLimit: (value) => setState((prevState) => ({ ...prevState, terminalOutputLimit: value })), + setTerminalOutputLineLimit: (value) => + setState((prevState) => ({ ...prevState, terminalOutputLineLimit: value })), setMcpEnabled: (value) => setState((prevState) => ({ ...prevState, mcpEnabled: value })), setEnableMcpServerCreation: (value) => setState((prevState) => ({ ...prevState, enableMcpServerCreation: value })),