Skip to content

fix(grep): stream ripgrep output to prevent memory exhaustion #5432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Hona wants to merge 2 commits into from
Closed

fix(grep): stream ripgrep output to prevent memory exhaustion #5432

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52e8688
fix(grep): stream ripgrep output to prevent memory exhaustion
Hona Dec 12, 2025
5a08e68
clean
Hona Dec 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 77 additions & 32 deletions packages/opencode/src/tool/grep.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import DESCRIPTION from "./grep.txt"
import { Instance } from "../project/instance"

const MAX_LINE_LENGTH = 2000
const MATCH_LIMIT = 100

export const GrepTool = Tool.define("grep", {
description: DESCRIPTION,
Expand Down Expand Up @@ -33,51 +34,95 @@ export const GrepTool = Tool.define("grep", {
stderr: "pipe",
})

const output = await new Response(proc.stdout).text()
const reader = proc.stdout.getReader()
const decoder = new TextDecoder()
let buffer = ""
const matches: Array<{
path: string
modTime: number
lineNum: number
lineText: string
}> = []
let hitCollectLimit = false

try {
while (true) {
const { done, value } = await reader.read()
if (done) break

buffer += decoder.decode(value, { stream: true })
const lines = buffer.split("\n")
buffer = lines.pop() || ""

for (const line of lines) {
if (!line) continue
if (matches.length >= MATCH_LIMIT) {
hitCollectLimit = true
break
}

const [filePath, lineNumStr, ...lineTextParts] = line.split("|")
if (!filePath || !lineNumStr || lineTextParts.length === 0) continue

const lineNum = parseInt(lineNumStr, 10)
const lineText = lineTextParts.join("|")

const file = Bun.file(filePath)
const stats = await file.stat().catch(() => null)
if (!stats) continue

Copilot AI Dec 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling file.stat() for every single match result can be a performance bottleneck, especially when processing many results. Each stat call is an I/O operation that waits for file system access. Since ripgrep already provides the file path, consider whether the modification time is truly necessary for sorting, or if there's a way to batch or optimize these stat calls.

Copilot uses AI. Check for mistakes.

matches.push({
path: filePath,
modTime: stats.mtime.getTime(),
lineNum,
lineText,
})
}
Comment on lines +56 to +71

Copilot AI Dec 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The streaming implementation processes matches asynchronously with file.stat() calls inside the read loop. If file.stat() is slow, this could cause backpressure issues where the stdout buffer fills up because the reader isn't consuming chunks fast enough. Consider processing file stats after collecting all line data, or implement a queue-based approach to handle I/O more efficiently.

Copilot uses AI. Check for mistakes.

if (hitCollectLimit) break
}

if (!hitCollectLimit && buffer) {
const [filePath, lineNumStr, ...lineTextParts] = buffer.split("|")
if (filePath && lineNumStr && lineTextParts.length > 0) {
const lineNum = parseInt(lineNumStr, 10)
const lineText = lineTextParts.join("|")
const file = Bun.file(filePath)
const stats = await file.stat().catch(() => null)
if (stats) {
matches.push({
path: filePath,
modTime: stats.mtime.getTime(),
lineNum,
lineText,
})
}
}
}

Copilot AI Dec 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handling of the last buffered line (lines 85-101) has duplication with the main loop logic (lines 64-79). This duplicated parsing and file stat logic reduces maintainability. Consider extracting the line processing into a separate function to avoid code duplication.

Copilot uses AI. Check for mistakes.
} finally {
if (hitCollectLimit) proc.kill()
reader.releaseLock()
}

const errorOutput = await new Response(proc.stderr).text()

Copilot AI Dec 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the collection limit is hit and the process is killed, the stderr might not be fully captured yet since proc.stderr is being read after the kill. This could lead to incomplete error messages. Consider reading stderr before killing the process, or at minimum be aware that error output may be incomplete when the limit is hit.

Copilot uses AI. Check for mistakes.
const exitCode = await proc.exited

if (exitCode === 1) {
if (exitCode === 1 && matches.length === 0) {

Copilot AI Dec 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the process is killed due to hitting the collection limit (line 103), the exit code will likely be non-zero (e.g., killed by signal). The error handling on line 118 accounts for this with !hitCollectLimit, but there's a potential edge case: if ripgrep legitimately fails with exit code 1 but we already collected some matches, we'll now return those matches instead of reporting the error. The condition on line 110 should verify that exit code 1 specifically means "no matches found" rather than another error condition.

Copilot uses AI. Check for mistakes.
return {
title: params.pattern,
metadata: { matches: 0, truncated: false },
output: "No files found",
}
}

if (exitCode !== 0) {
if (exitCode !== 0 && exitCode !== 1 && !hitCollectLimit) {
throw new Error(`ripgrep failed: ${errorOutput}`)
}

const lines = output.trim().split("\n")
const matches = []

for (const line of lines) {
if (!line) continue

const [filePath, lineNumStr, ...lineTextParts] = line.split("|")
if (!filePath || !lineNumStr || lineTextParts.length === 0) continue

const lineNum = parseInt(lineNumStr, 10)
const lineText = lineTextParts.join("|")

const file = Bun.file(filePath)
const stats = await file.stat().catch(() => null)
if (!stats) continue

matches.push({
path: filePath,
modTime: stats.mtime.getTime(),
lineNum,
lineText,
})
}

matches.sort((a, b) => b.modTime - a.modTime)

const limit = 100
const truncated = matches.length > limit
const finalMatches = truncated ? matches.slice(0, limit) : matches
const truncated = matches.length > MATCH_LIMIT
const finalMatches = truncated ? matches.slice(0, MATCH_LIMIT) : matches

Copilot AI Dec 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable truncated is computed based on whether matches.length exceeds MATCH_LIMIT, but since the collection loop stops at MATCH_LIMIT items, matches.length will never actually be greater than MATCH_LIMIT. This means truncated will always be false, making the variable redundant. The truncation logic should rely solely on hitCollectLimit to determine if results were truncated.

Copilot uses AI. Check for mistakes.

if (finalMatches.length === 0) {
return {
Expand All @@ -103,7 +148,7 @@ export const GrepTool = Tool.define("grep", {
outputLines.push(` Line ${match.lineNum}: ${truncatedLineText}`)
}

if (truncated) {
if (truncated || hitCollectLimit) {
outputLines.push("")
outputLines.push("(Results are truncated. Consider using a more specific path or pattern.)")
}
Expand All @@ -112,7 +157,7 @@ export const GrepTool = Tool.define("grep", {
title: params.pattern,
metadata: {
matches: finalMatches.length,
truncated,
truncated: truncated || hitCollectLimit,
},
output: outputLines.join("\n"),
}
Expand Down