diff --git a/src/core/Cline.ts b/src/core/Cline.ts index c1200162307..38fc61f48a6 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -25,7 +25,6 @@ import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers, - truncateOutput, } from "../integrations/misc/extract-text" import { TerminalManager, ExitCodeDetails } from "../integrations/terminal/TerminalManager" import { UrlContentFetcher } from "../services/browser/UrlContentFetcher" @@ -55,11 +54,12 @@ import { HistoryItem } from "../shared/HistoryItem" import { ClineAskResponse } from "../shared/WebviewMessage" import { GlobalFileNames } from "../shared/globalFileNames" import { defaultModeSlug, getModeBySlug, getFullModeDetails } from "../shared/modes" +import { EXPERIMENT_IDS, experiments as Experiments, ExperimentId } from "../shared/experiments" import { calculateApiCost } from "../utils/cost" import { fileExistsAtPath } from "../utils/fs" import { arePathsEqual, getReadablePath } from "../utils/path" import { parseMentions } from "./mentions" -import { RooIgnoreController, LOCK_TEXT_SYMBOL } from "./ignore/RooIgnoreController" +import { RooIgnoreController } from "./ignore/RooIgnoreController" import { AssistantMessageContent, parseAssistantMessage, ToolParamName, ToolUseName } from "./assistant-message" import { formatResponse } from "./prompts/responses" import { SYSTEM_PROMPT } from "./prompts/system" @@ -70,7 +70,7 @@ import { BrowserSession } from "../services/browser/BrowserSession" import { McpHub } from "../services/mcp/McpHub" import crypto from "crypto" import { insertGroups } from "./diff/insert-groups" -import { EXPERIMENT_IDS, experiments as Experiments, ExperimentId } from "../shared/experiments" +import { OutputBuilder } from "../integrations/terminal/OutputBuilder" const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution @@ -912,30 +912,43 @@ 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) - 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) + // 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) let userFeedback: { text?: string; images?: string[] } | undefined let didContinue = false - const sendCommandOutput = async (line: string): Promise => { + + const sendCommandOutput = async (line: string) => { 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 lines: string[] = [] + let completed = false + let exitDetails: ExitCodeDetails | undefined + + let builder = new OutputBuilder({ maxSize: terminalOutputLimit }) + let output: string | undefined = undefined + process.on("line", (line) => { - lines.push(line) + builder.append(line) + if (!didContinue) { sendCommandOutput(line) } else { @@ -943,13 +956,8 @@ export class Cline { } }) - 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") - } + process.once("completed", (buffer?: string) => { + output = buffer completed = true }) @@ -965,19 +973,17 @@ 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 { terminalOutputLineLimit } = (await this.providerRef.deref()?.getState()) ?? {} - const output = truncateOutput(lines.join("\n"), terminalOutputLineLimit) - const result = output.trim() + const result = output || builder.content if (userFeedback) { await this.say("user_feedback", userFeedback.text, userFeedback.images) + return [ true, formatResponse.toolResult( @@ -991,9 +997,11 @@ 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" } @@ -1001,15 +1009,16 @@ 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 { @@ -3495,7 +3504,7 @@ export class Cline { terminalDetails += "\n\n# Actively Running Terminals" for (const busyTerminal of busyTerminals) { terminalDetails += `\n## Original command: \`${busyTerminal.lastCommand}\`` - const newOutput = this.terminalManager.getUnretrievedOutput(busyTerminal.id) + const newOutput = this.terminalManager.readLine(busyTerminal.id) if (newOutput) { terminalDetails += `\n### New Output\n${newOutput}` } else { @@ -3507,7 +3516,7 @@ export class Cline { if (inactiveTerminals.length > 0) { const inactiveTerminalOutputs = new Map() for (const inactiveTerminal of inactiveTerminals) { - const newOutput = this.terminalManager.getUnretrievedOutput(inactiveTerminal.id) + const newOutput = this.terminalManager.readLine(inactiveTerminal.id) if (newOutput) { inactiveTerminalOutputs.set(inactiveTerminal.id, newOutput) } diff --git a/src/core/__tests__/contextProxy.test.ts b/src/core/__tests__/contextProxy.test.ts index 0ac98bbc8ce..e44f3e45b3c 100644 --- a/src/core/__tests__/contextProxy.test.ts +++ b/src/core/__tests__/contextProxy.test.ts @@ -255,4 +255,77 @@ describe("ContextProxy", () => { expect(proxy.getGlobalState("unknownKey")).toBe("some-value") }) }) + + describe("resetAllState", () => { + it("should clear all in-memory caches", async () => { + // Setup initial state in caches + await proxy.setValues({ + apiModelId: "gpt-4", // global state + openAiApiKey: "test-api-key", // secret + unknownKey: "some-value", // unknown + }) + + // Verify initial state + expect(proxy.getGlobalState("apiModelId")).toBe("gpt-4") + expect(proxy.getSecret("openAiApiKey")).toBe("test-api-key") + expect(proxy.getGlobalState("unknownKey")).toBe("some-value") + + // Reset all state + await proxy.resetAllState() + + // Caches should be reinitialized with values from the context + // Since our mock globalState.get returns undefined by default, + // the cache should now contain undefined values + expect(proxy.getGlobalState("apiModelId")).toBeUndefined() + expect(proxy.getGlobalState("unknownKey")).toBeUndefined() + }) + + it("should update all global state keys to undefined", async () => { + // Setup initial state + await proxy.updateGlobalState("apiModelId", "gpt-4") + await proxy.updateGlobalState("apiProvider", "openai") + + // Reset all state + await proxy.resetAllState() + + // Should have called update with undefined for each key + for (const key of GLOBAL_STATE_KEYS) { + expect(mockGlobalState.update).toHaveBeenCalledWith(key, undefined) + } + + // Total calls should include initial setup + reset operations + const expectedUpdateCalls = 2 + GLOBAL_STATE_KEYS.length + expect(mockGlobalState.update).toHaveBeenCalledTimes(expectedUpdateCalls) + }) + + it("should delete all secrets", async () => { + // Setup initial secrets + await proxy.storeSecret("apiKey", "test-api-key") + await proxy.storeSecret("openAiApiKey", "test-openai-key") + + // Reset all state + await proxy.resetAllState() + + // Should have called delete for each key + for (const key of SECRET_KEYS) { + expect(mockSecrets.delete).toHaveBeenCalledWith(key) + } + + // Total calls should equal the number of secret keys + expect(mockSecrets.delete).toHaveBeenCalledTimes(SECRET_KEYS.length) + }) + + it("should reinitialize caches after reset", async () => { + // Spy on initialization methods + const initStateCache = jest.spyOn(proxy as any, "initializeStateCache") + const initSecretCache = jest.spyOn(proxy as any, "initializeSecretCache") + + // Reset all state + await proxy.resetAllState() + + // Should reinitialize caches + expect(initStateCache).toHaveBeenCalledTimes(1) + expect(initSecretCache).toHaveBeenCalledTimes(1) + }) + }) }) diff --git a/src/core/contextProxy.ts b/src/core/contextProxy.ts index f27cc7f65c8..52197d99f3a 100644 --- a/src/core/contextProxy.ts +++ b/src/core/contextProxy.ts @@ -129,4 +129,29 @@ export class ContextProxy { return Promise.all(promises) } + + /** + * Resets all global state, secrets, and in-memory caches. + * This clears all data from both the in-memory caches and the VSCode storage. + * @returns A promise that resolves when all reset operations are complete + */ + async resetAllState(): Promise { + // Clear in-memory caches + this.stateCache.clear() + this.secretCache.clear() + + // Reset all global state values to undefined + const stateResetPromises = GLOBAL_STATE_KEYS.map((key) => + this.originalContext.globalState.update(key, undefined), + ) + + // Delete all secrets + const secretResetPromises = SECRET_KEYS.map((key) => this.originalContext.secrets.delete(key)) + + // Wait for all reset operations to complete + await Promise.all([...stateResetPromises, ...secretResetPromises]) + + this.initializeStateCache() + this.initializeSecretCache() + } } diff --git a/src/core/mentions/index.ts b/src/core/mentions/index.ts index 5853a4abdd2..d74ee0db97f 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, formatGitSuggestion, type MentionSuggestion } from "../../shared/context-mentions" +import { mentionRegexGlobal } 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/get-latest-output" +import { getLatestTerminalOutput } from "../../integrations/terminal/getLatestTerminalOutput" export async function openMention(mention?: string): Promise { if (!mention) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6883eb4fdfa..c661d705e97 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -10,17 +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 { CustomSupportPrompts, supportPrompt } from "../../shared/support-prompt" +import { 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 { ApiConfigMeta, ExtensionMessage } from "../../shared/ExtensionMessage" import { checkoutDiffPayloadSchema, checkoutRestorePayloadSchema, WebviewMessage } from "../../shared/WebviewMessage" -import { Mode, CustomModePrompts, PromptComponent, defaultModeSlug, ModeConfig } from "../../shared/modes" +import { Mode, 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" @@ -1215,7 +1215,6 @@ 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() @@ -1249,8 +1248,8 @@ export class ClineProvider implements vscode.WebviewViewProvider { await this.updateGlobalState("writeDelayMs", message.value) await this.postStateToWebview() break - case "terminalOutputLineLimit": - await this.updateGlobalState("terminalOutputLineLimit", message.value) + case "terminalOutputLimit": + await this.updateGlobalState("terminalOutputLimit", message.value) await this.postStateToWebview() break case "mode": @@ -2134,7 +2133,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { screenshotQuality, preferredLanguage, writeDelayMs, - terminalOutputLineLimit, + terminalOutputLimit, fuzzyMatchThreshold, mcpEnabled, enableMcpServerCreation, @@ -2186,7 +2185,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { screenshotQuality: screenshotQuality ?? 75, preferredLanguage: preferredLanguage ?? "English", writeDelayMs: writeDelayMs ?? 1000, - terminalOutputLineLimit: terminalOutputLineLimit ?? 500, + terminalOutputLimit: terminalOutputLimit ?? TERMINAL_OUTPUT_LIMIT, fuzzyMatchThreshold: fuzzyMatchThreshold ?? 1.0, mcpEnabled: mcpEnabled ?? true, enableMcpServerCreation: enableMcpServerCreation ?? true, @@ -2334,7 +2333,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { screenshotQuality: stateValues.screenshotQuality ?? 75, fuzzyMatchThreshold: stateValues.fuzzyMatchThreshold ?? 1.0, writeDelayMs: stateValues.writeDelayMs ?? 1000, - terminalOutputLineLimit: stateValues.terminalOutputLineLimit ?? 500, + terminalOutputLimit: stateValues.terminalOutputLimit ?? TERMINAL_OUTPUT_LIMIT, mode: stateValues.mode ?? defaultModeSlug, preferredLanguage: stateValues.preferredLanguage ?? @@ -2433,14 +2432,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { return } - for (const key of this.context.globalState.keys()) { - await this.contextProxy.updateGlobalState(key, undefined) - } - - for (const key of SECRET_KEYS) { - await this.storeSecret(key, undefined) - } - + await this.contextProxy.resetAllState() await this.configManager.resetAllConfigs() await this.customModesManager.resetCustomModes() await this.removeClineFromStack() diff --git a/src/integrations/misc/__tests__/extract-text.test.ts b/src/integrations/misc/__tests__/extract-text.test.ts index 7e084d010c9..a8aacd9f342 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, truncateOutput } from "../extract-text" +import { addLineNumbers, everyLineHasLineNumbers, stripLineNumbers } from "../extract-text" describe("addLineNumbers", () => { it("should add line numbers starting from 1 by default", () => { @@ -101,67 +101,3 @@ 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 03545707065..2ce90d7e382 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -89,37 +89,3 @@ 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 new file mode 100644 index 00000000000..4d7d922dff3 --- /dev/null +++ b/src/integrations/terminal/OutputBuilder.ts @@ -0,0 +1,183 @@ +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 a55f7867d40..6dd0a57c46f 100644 --- a/src/integrations/terminal/TerminalManager.ts +++ b/src/integrations/terminal/TerminalManager.ts @@ -1,8 +1,11 @@ import pWaitFor from "p-wait-for" import * as vscode from "vscode" + +import { TERMINAL_OUTPUT_LIMIT } from "../../shared/terminal" import { arePathsEqual } from "../../utils/path" -import { mergePromise, TerminalProcess, TerminalProcessResultPromise } from "./TerminalProcess" +import { TerminalProcess } from "./TerminalProcess" import { TerminalInfo, TerminalRegistry } from "./TerminalRegistry" +import { mergePromise, TerminalProcessResultPromise } from "./mergePromise" /* TerminalManager: @@ -15,8 +18,6 @@ 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 @@ -30,7 +31,6 @@ 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.getUnretrievedOutput(terminalId); +const unretrievedOutput = terminalManager.readLine(terminalId); console.log('Unretrieved output:', unretrievedOutput); Resources: @@ -259,10 +259,14 @@ export class TerminalManager { } } - runCommand(terminalInfo: TerminalInfo, command: string): TerminalProcessResultPromise { + runCommand( + terminalInfo: TerminalInfo, + command: string, + terminalOutputLimit = TERMINAL_OUTPUT_LIMIT, + ): TerminalProcessResultPromise { terminalInfo.busy = true terminalInfo.lastCommand = command - const process = new TerminalProcess() + const process = new TerminalProcess(terminalOutputLimit) this.processes.set(terminalInfo.id, process) process.once("completed", () => { @@ -347,12 +351,13 @@ export class TerminalManager { .map((t) => ({ id: t.id, lastCommand: t.lastCommand })) } - getUnretrievedOutput(terminalId: number): string { + readLine(terminalId: number): string { if (!this.terminalIds.has(terminalId)) { return "" } + const process = this.processes.get(terminalId) - return process ? process.getUnretrievedOutput() : "" + return process ? process.readLine() : "" } isProcessHot(terminalId: number): boolean { diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 99ef215e784..047465e3fea 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -5,6 +5,34 @@ 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] @@ -21,20 +49,28 @@ 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 { - waitForShellIntegration: boolean = true - private isListening: boolean = true + public waitForShellIntegration = true + private _isHot = false + + private isListening = true private terminalInfo: TerminalInfo | undefined - private lastEmitTime_ms: number = 0 - private fullOutput: string = "" - private lastRetrievedIndex: number = 0 - isHot: boolean = false + private lastEmitTime_ms = 0 + private outputBuilder?: OutputBuilder 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 @@ -66,7 +102,7 @@ export class TerminalProcess extends EventEmitter { }) }) - // getUnretrievedOutput needs to know if streamClosed, so store this for later + // readLine needs to know if streamClosed, so store this for later this.terminalInfo = terminalInfo // Execute command @@ -89,61 +125,58 @@ export class TerminalProcess extends EventEmitter { * - OSC 633 ; E ; [; ] ST - Explicitly set command line with optional nonce */ - // Process stream data + 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? + */ + 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.fullOutput = "" // Reset fullOutput when command actually starts + this.outputBuilder.reset() // Reset output when command actually starts. } else { continue } } // Command output started, accumulate data without filtering. - // notice to future programmers: do not add escape sequence - // filtering here: fullOutput cannot change in length (see getUnretrievedOutput), + // Notice to future programmers: do not add escape sequence + // filtering here: output cannot change in length (see `readLine`), // and chunks may not be complete so you cannot rely on detecting or removing escape sequences mid-stream. - this.fullOutput += data + this.outputBuilder.append(data) // For non-immediately returning commands we want to show loading 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 + // 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. const now = Date.now() - if (this.isListening && (now - this.lastEmitTime_ms > 100 || this.lastEmitTime_ms === 0)) { - this.emitRemainingBufferIfListening() + const timeSinceLastEmit = now - this.lastEmitTime_ms + + if (this.isListening && timeSinceLastEmit > EMIT_INTERVAL) { + this.flushLine() this.lastEmitTime_ms = now } - // 2. Set isHot depending on the command. + // 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 = - compilingMarkers.some((marker) => data.toLowerCase().includes(marker.toLowerCase())) && - !markerNullifiers.some((nullifier) => data.toLowerCase().includes(nullifier.toLowerCase())) + COMPILE_MARKERS.some((marker) => data.toLowerCase().includes(marker.toLowerCase())) && + !COMPILE_MARKER_NULLIFIERS.some((nullifier) => data.toLowerCase().includes(nullifier.toLowerCase())) + this.hotTimer = setTimeout( () => { this.isHot = false @@ -152,18 +185,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 - const exitDetails = await shellExecutionComplete + // Wait for shell execution to complete and handle exit details. + await shellExecutionComplete this.isHot = false if (commandOutputStarted) { - // Emit any remaining output before completing - this.emitRemainingBufferIfListening() + // Emit any remaining output before completing. + this.flushAll() } else { console.error( "[Terminal Process] VSCE output start escape sequence (]633;C or ]133;C) not received! VSCE Bug? preOutput: " + @@ -171,62 +204,77 @@ export class TerminalProcess extends EventEmitter { ) } - // 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 + // 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.fullOutput) + const match = this.matchBeforeVsceEndMarkers(this.outputBuilder.content) + if (match !== undefined) { - this.fullOutput = match + this.outputBuilder.reset(match) } - // 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") + // 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.fullOutput)) + this.emit("completed", this.removeEscapeSequences(this.outputBuilder.content)) 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 after a delay + // For terminals without shell integration, we can't know when the command completes. + // So we'll just emit the continue event. 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 } } - private emitRemainingBufferIfListening() { - if (this.isListening) { - const remainingBuffer = this.getUnretrievedOutput() - if (remainingBuffer !== "") { - this.emit("line", remainingBuffer) - } - } + public readLine() { + return this.processOutput(this.outputBuilder?.readLine() || "") } - continue() { - this.emitRemainingBufferIfListening() + public read() { + return this.processOutput(this.outputBuilder?.read() || "") + } + + public continue() { + this.flushAll() this.isListening = false this.removeAllListeners("line") this.emit("continue") } - // 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) + private flushLine() { + if (!this.isListening) { + return + } + + const line = this.readLine() + + if (line) { + this.emit("line", line) + } + } + + private flushAll() { + if (!this.isListening) { + return + } + + const buffer = this.read() - // Check for VSCE command end markers + if (buffer) { + this.emit("line", buffer) + } + } + + private processOutput(outputToProcess: string) { + // Check for VSCE command end markers. const index633 = outputToProcess.indexOf("\x1b]633;D") const index133 = outputToProcess.indexOf("\x1b]133;D") let endIndex = -1 @@ -239,32 +287,7 @@ export class TerminalProcess extends EventEmitter { endIndex = index133 } - // 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) + return this.removeEscapeSequences(endIndex >= 0 ? outputToProcess.slice(0, endIndex) : outputToProcess) } private stringIndexMatch( @@ -282,18 +305,20 @@ 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 @@ -307,6 +332,7 @@ export class TerminalProcess extends EventEmitter { endIndex = data.length } else { endIndex = data.indexOf(suffix, contentStart) + if (endIndex === -1) { return undefined } @@ -323,7 +349,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): string { + private removeEscapeSequences(str: string) { return stripAnsi(str.replace(/\x1b\]633;[^\x07]+\x07/gs, "").replace(/\x1b\]133;[^\x07]+\x07/gs, "")) } @@ -396,20 +422,3 @@ 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 new file mode 100644 index 00000000000..acb228053a0 --- /dev/null +++ b/src/integrations/terminal/__tests__/OutputBuilder.test.ts @@ -0,0 +1,272 @@ +// 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 44cae92580f..c90c517e3ec 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, mergePromise } from "../TerminalProcess" +import { TerminalProcess } from "../TerminalProcess" import { TerminalInfo, TerminalRegistry } from "../TerminalRegistry" // Mock vscode.window.createTerminal @@ -34,7 +34,7 @@ describe("TerminalProcess", () => { let mockStream: AsyncIterableIterator beforeEach(() => { - terminalProcess = new TerminalProcess() + terminalProcess = new TerminalProcess(100 * 1024) // Create properly typed mock terminal mockTerminal = { @@ -173,32 +173,4 @@ 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 new file mode 100644 index 00000000000..1b2b60d179c --- /dev/null +++ b/src/integrations/terminal/__tests__/mergePromise.test.ts @@ -0,0 +1,20 @@ +// 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/get-latest-output.ts b/src/integrations/terminal/getLatestTerminalOutput.ts similarity index 100% rename from src/integrations/terminal/get-latest-output.ts rename to src/integrations/terminal/getLatestTerminalOutput.ts diff --git a/src/integrations/terminal/mergePromise.ts b/src/integrations/terminal/mergePromise.ts new file mode 100644 index 00000000000..d1a1f453295 --- /dev/null +++ b/src/integrations/terminal/mergePromise.ts @@ -0,0 +1,23 @@ +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 9a10e04a08f..795832ae58b 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -124,7 +124,7 @@ export interface ExtensionState { fuzzyMatchThreshold?: number preferredLanguage: string writeDelayMs: number - terminalOutputLineLimit?: number + terminalOutputLimit?: number mcpEnabled: boolean enableMcpServerCreation: boolean mode: Mode diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 9592b559e26..58219abb307 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -68,7 +68,7 @@ export interface WebviewMessage { | "enhancedPrompt" | "draggedImages" | "deleteMessage" - | "terminalOutputLineLimit" + | "terminalOutputLimit" | "mcpEnabled" | "enableMcpServerCreation" | "searchCommits" diff --git a/src/shared/globalState.ts b/src/shared/globalState.ts index fd7bd1adb9f..2df20242a12 100644 --- a/src/shared/globalState.ts +++ b/src/shared/globalState.ts @@ -66,7 +66,7 @@ export const GLOBAL_STATE_KEYS = [ "fuzzyMatchThreshold", "preferredLanguage", // Language setting for Cline's communication "writeDelayMs", - "terminalOutputLineLimit", + "terminalOutputLimit", "mcpEnabled", "enableMcpServerCreation", "alwaysApproveResubmit", diff --git a/src/shared/terminal.ts b/src/shared/terminal.ts new file mode 100644 index 00000000000..4747f8727ff --- /dev/null +++ b/src/shared/terminal.ts @@ -0,0 +1 @@ +export const TERMINAL_OUTPUT_LIMIT = 100 * 1024 diff --git a/src/utils/git.ts b/src/utils/git.ts index 640af7fd29d..16d6240ac17 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 { truncateOutput } from "../integrations/misc/extract-text" + +import { OutputBuilder } from "../integrations/terminal/OutputBuilder" const execAsync = promisify(exec) -const GIT_OUTPUT_LINE_LIMIT = 500 export interface GitCommit { hash: string @@ -122,8 +122,9 @@ export async function getCommitInfo(hash: string, cwd: string): Promise "\nFull Changes:", ].join("\n") - const output = summary + "\n\n" + diff.trim() - return truncateOutput(output, GIT_OUTPUT_LINE_LIMIT) + const builder = new OutputBuilder() + builder.append(summary + "\n\n" + diff.trim()) + return builder.content } catch (error) { console.error("Error getting commit info:", error) return `Failed to get commit info: ${error instanceof Error ? error.message : String(error)}` @@ -150,9 +151,10 @@ 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 lineLimit = GIT_OUTPUT_LINE_LIMIT - const output = `Working directory changes:\n\n${status}\n\n${diff}`.trim() - return truncateOutput(output, lineLimit) + + const builder = new OutputBuilder() + builder.append(`Working directory changes:\n\n${status}\n\n${diff}`.trim()) + return builder.content } 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 dd5fd44f883..ec78699c42d 100644 --- a/webview-ui/src/components/settings/AdvancedSettings.tsx +++ b/webview-ui/src/components/settings/AdvancedSettings.tsx @@ -3,6 +3,7 @@ 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" @@ -13,12 +14,12 @@ import { Section } from "./Section" type AdvancedSettingsProps = HTMLAttributes & { rateLimitSeconds: number - terminalOutputLineLimit?: number + terminalOutputLimit?: number maxOpenTabsContext: number diffEnabled?: boolean fuzzyMatchThreshold?: number setCachedStateField: SetCachedStateField< - "rateLimitSeconds" | "terminalOutputLineLimit" | "maxOpenTabsContext" | "diffEnabled" | "fuzzyMatchThreshold" + "rateLimitSeconds" | "terminalOutputLimit" | "maxOpenTabsContext" | "diffEnabled" | "fuzzyMatchThreshold" > experiments: Record setExperimentEnabled: SetExperimentEnabled @@ -26,7 +27,7 @@ type AdvancedSettingsProps = HTMLAttributes & { export const AdvancedSettings = ({ rateLimitSeconds, - terminalOutputLineLimit, + terminalOutputLimit = TERMINAL_OUTPUT_LIMIT, maxOpenTabsContext, diffEnabled, fuzzyMatchThreshold, @@ -71,21 +72,20 @@ export const AdvancedSettings = ({
- setCachedStateField("terminalOutputLineLimit", parseInt(e.target.value)) - } + min={1024} + max={1024 * 1024} + step={1024} + value={terminalOutputLimit} + onChange={(e) => setCachedStateField("terminalOutputLimit", parseInt(e.target.value))} className="h-2 focus:outline-0 w-4/5 accent-vscode-button-background" /> - {terminalOutputLineLimit ?? 500} + {Math.floor(terminalOutputLimit / 1024)} KB

- Maximum number of lines to include in terminal output when executing commands. When exceeded - lines will be removed from the middle, saving tokens. + 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.

diff --git a/webview-ui/src/components/settings/ExperimentalSettings.tsx b/webview-ui/src/components/settings/ExperimentalSettings.tsx index bbdfe47e893..d39c52e9c4f 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" | "terminalOutputLineLimit" | "maxOpenTabsContext" | "diffEnabled" | "fuzzyMatchThreshold" + "rateLimitSeconds" | "terminalOutputLimit" | "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 7b38455b80b..f06364a7362 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -2,8 +2,9 @@ import { forwardRef, memo, useCallback, useEffect, useImperativeHandle, useMemo, import { Button as VSCodeButton } from "vscrui" import { CheckCheck, SquareMousePointer, Webhook, GitBranch, Bell, Cog, FlaskConical } from "lucide-react" -import { ExperimentId } from "../../../../src/shared/experiments" import { ApiConfiguration } from "../../../../src/shared/api" +import { ExperimentId } from "../../../../src/shared/experiments" +import { TERMINAL_OUTPUT_LIMIT } from "../../../../src/shared/terminal" import { vscode } from "@/utils/vscode" import { ExtensionStateContextType, useExtensionState } from "@/context/ExtensionStateContext" @@ -77,7 +78,7 @@ const SettingsView = forwardRef(({ onDone }, screenshotQuality, soundEnabled, soundVolume, - terminalOutputLineLimit, + terminalOutputLimit, writeDelayMs, } = cachedState @@ -157,7 +158,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: "terminalOutputLineLimit", value: terminalOutputLineLimit ?? 500 }) + vscode.postMessage({ type: "terminalOutputLimit", value: terminalOutputLimit ?? TERMINAL_OUTPUT_LIMIT }) vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled }) vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit }) vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds }) @@ -379,7 +380,7 @@ const SettingsView = forwardRef(({ onDone },
void screenshotQuality?: number setScreenshotQuality: (value: number) => void - terminalOutputLineLimit?: number - setTerminalOutputLineLimit: (value: number) => void + terminalOutputLimit?: number + setTerminalOutputLimit: (value: number) => void mcpEnabled: boolean setMcpEnabled: (value: boolean) => void enableMcpServerCreation: boolean @@ -113,7 +116,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode writeDelayMs: 1000, browserViewportSize: "900x600", screenshotQuality: 75, - terminalOutputLineLimit: 500, + terminalOutputLimit: TERMINAL_OUTPUT_LIMIT, mcpEnabled: true, enableMcpServerCreation: true, alwaysApproveResubmit: false, @@ -250,8 +253,7 @@ 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 })), - setTerminalOutputLineLimit: (value) => - setState((prevState) => ({ ...prevState, terminalOutputLineLimit: value })), + setTerminalOutputLimit: (value) => setState((prevState) => ({ ...prevState, terminalOutputLimit: value })), setMcpEnabled: (value) => setState((prevState) => ({ ...prevState, mcpEnabled: value })), setEnableMcpServerCreation: (value) => setState((prevState) => ({ ...prevState, enableMcpServerCreation: value })),