feat: lossless terminal output with on-demand retrieval #10944
Conversation
…utput Implements a new tool that allows the LLM to retrieve full command output when execute_command produces output exceeding the preview threshold. Key components: - ReadCommandOutputTool: Reads persisted output with search/pagination - OutputInterceptor: Intercepts and persists large command outputs to disk - Terminal settings UI: Configuration for output interception behavior - Type definitions for output interception settings The tool supports: - Reading full output beyond the truncated preview - Search/filtering with regex patterns (like grep) - Pagination through large outputs using offset/limit Includes comprehensive tests for ReadCommandOutputTool and OutputInterceptor.
dosubot
bot
added
size:XXL
Re-review complete. No new issues found since
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
src/core/tools/ExecuteCommandTool.ts
Outdated
| // Use default preview size for now (terminalOutputPreviewSize setting will be added in Phase 5) | ||
| const terminalOutputPreviewSize = DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE | ||
| const providerState = await provider?.getState() | ||
| const terminalCompressProgressBar = providerState?.terminalCompressProgressBar ?? true |
There was a problem hiding this comment.
terminalOutputPreviewSize is exposed in settings/types, but execute_command currently always uses DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE, so user configuration has no effect.
| // Use default preview size for now (terminalOutputPreviewSize setting will be added in Phase 5) | |
| const terminalOutputPreviewSize = DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE | |
| const providerState = await provider?.getState() | |
| const terminalCompressProgressBar = providerState?.terminalCompressProgressBar ?? true | |
| const providerState = await provider?.getState() | |
| const terminalOutputPreviewSize = providerState?.terminalOutputPreviewSize ?? DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE | |
| const terminalCompressProgressBar = providerState?.terminalCompressProgressBar ?? true |
Fix it with Roo Code or mention @roomote and request a fix.
| description: LIMIT_DESCRIPTION, | ||
| }, | ||
| }, | ||
| required: ["artifact_id", "search", "offset", "limit"], |
There was a problem hiding this comment.
The native tool schema marks search/offset/limit as required, but the implementation treats them as optional. With strict=true, tool calls that omit those fields will fail schema validation before reaching the tool.
| required: ["artifact_id", "search", "offset", "limit"], | |
| required: ["artifact_id"], |
Fix it with Roo Code or mention @roomote and request a fix.
| // Calculate line numbers based on offset | ||
| let startLineNumber = 1 | ||
| if (offset > 0) { | ||
| // Count newlines before offset to determine starting line number | ||
| const prefixBuffer = Buffer.alloc(offset) | ||
| await fileHandle.read(prefixBuffer, 0, offset, 0) | ||
| const prefix = prefixBuffer.toString("utf8") | ||
| startLineNumber = (prefix.match(/\n/g) || []).length + 1 | ||
| } |
There was a problem hiding this comment.
readArtifact() allocates a Buffer of size offset to count newlines, which can be huge for large outputs and defeats the goal of keeping memory usage bounded. Consider streaming/iterating to compute the start line number, or dropping line numbers in paginated reads (or only adding them when offset is small).
Fix it with Roo Code or mention @roomote and request a fix.
hannesrudolph
changed the title
Flagged a few correctness and contract issues to address before merge.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
src/core/tools/ExecuteCommandTool.ts
Outdated
| const taskDir = await getTaskDirectoryPath(globalStoragePath, task.taskId) | ||
| const storageDir = path.join(taskDir, "command-output") | ||
| // Use default preview size for now (terminalOutputPreviewSize setting will be added in Phase 5) | ||
| const terminalOutputPreviewSize = DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE |
There was a problem hiding this comment.
execute_command currently hardcodes previewSize to DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE, so user-configured terminalOutputPreviewSize has no effect. Pull this from provider state when available so the dropdown actually controls spill threshold.
| const terminalOutputPreviewSize = DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE | |
| const providerState = await provider?.getState() | |
| const terminalOutputPreviewSize = | |
| providerState?.terminalOutputPreviewSize ?? DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE | |
| const terminalCompressProgressBar = providerState?.terminalCompressProgressBar ?? true |
Fix it with Roo Code or mention @roomote and request a fix.
| description: LIMIT_DESCRIPTION, | ||
| }, | ||
| }, | ||
| required: ["artifact_id", "search", "offset", "limit"], |
There was a problem hiding this comment.
With strict=true, marking search/offset/limit as required means tool calls that omit those fields will fail schema validation before reaching the implementation (which treats them as optional). Make only artifact_id required.
| required: ["artifact_id", "search", "offset", "limit"], | |
| required: ["artifact_id"], |
Fix it with Roo Code or mention @roomote and request a fix.
| // Calculate line numbers based on offset | ||
| let startLineNumber = 1 | ||
| if (offset > 0) { | ||
| // Count newlines before offset to determine starting line number | ||
| const prefixBuffer = Buffer.alloc(offset) | ||
| await fileHandle.read(prefixBuffer, 0, offset, 0) | ||
| const prefix = prefixBuffer.toString("utf8") | ||
| startLineNumber = (prefix.match(/\n/g) || []).length + 1 | ||
| } |
There was a problem hiding this comment.
read_command_output calculates startLineNumber by allocating Buffer(offset) and reading the entire prefix, which can be huge for large offsets and defeats the bounded-memory goal. Consider dropping line numbers when offset is large, or count newlines in fixed-size chunks up to offset.
Fix it with Roo Code or mention @roomote and request a fix.
- Read terminalOutputPreviewSize from providerState instead of hardcoded default - Fix native tool schema to only require artifact_id (optional params no longer required) - Fix Buffer allocation for line numbers using chunked 64KB reads to avoid memory blowup
| let accumulatedOutput = "" | ||
| const callbacks: RooTerminalCallbacks = { | ||
| onLine: async (lines: string, process: RooTerminalProcess) => { | ||
| accumulatedOutput += lines |
There was a problem hiding this comment.
accumulatedOutput grows without bound as output streams in, so a long-running command can still drive memory usage arbitrarily high even though the UI only needs a truncated view. Capping the in-memory accumulator (for UI status updates) to terminalOutputCharacterLimit keeps memory bounded while OutputInterceptor still preserves the full output on disk.
| accumulatedOutput += lines | |
| accumulatedOutput += lines | |
| if (accumulatedOutput.length > terminalOutputCharacterLimit) { | |
| accumulatedOutput = accumulatedOutput.slice(-terminalOutputCharacterLimit) | |
| } |
Fix it with Roo Code or mention @roomote and request a fix.
Prevent unbounded memory growth during long-running commands by trimming the accumulated output buffer. The full output is preserved by the OutputInterceptor; this buffer is only used for UI display.
- Remove unused barrel file (src/integrations/terminal/index.ts) to fix knip check - Fix Windows path test in OutputInterceptor.test.ts by using path.normalize() - Add missing translations for terminal.outputPreviewSize settings to all 17 locales
Summary
Refactors terminal command output handling to preserve full output losslessly. Instead of truncating large outputs and losing information, Roo now saves complete output to disk and provides the LLM with a preview plus the ability to retrieve the full content on demand.
Closes #10941
Problem
Users experienced several pain points with the previous terminal output handling:
npm install,cargo build, test suites, or other commands that produce significant output, important error messages at the end could get truncated awaySolution
This PR implements a "persisted output" pattern:
read_command_outputtool lets the LLM fetch the complete output or search for specific patterns when neededUser-Facing Changes
Settings
Before: Two confusing sliders
After: One simple dropdown
The new setting controls how much output Roo sees directly in the preview. Full output is always preserved and accessible.
How It Works
npm test)read_command_outputto view the full output or search for specific errorsBenefits
Technical Changes
OutputInterceptorclass: Buffers output and spills to disk when threshold exceededread_command_outputtool: Allows reading full output or searching for patternsExecuteCommandTool: Uses the new interceptor patternTesting
OutputInterceptor(buffering, spilling, cleanup)ReadCommandOutputTool(read, search, error cases)