fix: respect maxReadFileLine setting for file mentions to prevent context exhaustion (#6073)

Co-authored-by: Roo Code <roomote@roocode.com>
Co-authored-by: Daniel Riccio <ricciodaniel98@gmail.com>
This commit is contained in:
roomote[bot] 2025-07-25 16:45:45 -04:00 committed by GitHub
parent d62a260576
commit f45d9be709
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 629 additions and 4 deletions

View file

@ -0,0 +1,353 @@
// npx vitest core/mentions/__tests__/processUserContentMentions.spec.ts
import { describe, it, expect, vi, beforeEach } from "vitest"
import { processUserContentMentions } from "../processUserContentMentions"
import { parseMentions } from "../index"
import { UrlContentFetcher } from "../../../services/browser/UrlContentFetcher"
import { FileContextTracker } from "../../context-tracking/FileContextTracker"
// Mock the parseMentions function
vi.mock("../index", () => ({
parseMentions: vi.fn(),
}))
describe("processUserContentMentions", () => {
let mockUrlContentFetcher: UrlContentFetcher
let mockFileContextTracker: FileContextTracker
let mockRooIgnoreController: any
beforeEach(() => {
vi.clearAllMocks()
mockUrlContentFetcher = {} as UrlContentFetcher
mockFileContextTracker = {} as FileContextTracker
mockRooIgnoreController = {}
// Default mock implementation
vi.mocked(parseMentions).mockImplementation(async (text) => `parsed: ${text}`)
})
describe("maxReadFileLine parameter", () => {
it("should pass maxReadFileLine to parseMentions when provided", async () => {
const userContent = [
{
type: "text" as const,
text: "<task>Read file with limit</task>",
},
]
await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
rooIgnoreController: mockRooIgnoreController,
maxReadFileLine: 100,
})
expect(parseMentions).toHaveBeenCalledWith(
"<task>Read file with limit</task>",
"/test",
mockUrlContentFetcher,
mockFileContextTracker,
mockRooIgnoreController,
true,
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
100,
)
})
it("should pass undefined maxReadFileLine when not provided", async () => {
const userContent = [
{
type: "text" as const,
text: "<task>Read file without limit</task>",
},
]
await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
rooIgnoreController: mockRooIgnoreController,
})
expect(parseMentions).toHaveBeenCalledWith(
"<task>Read file without limit</task>",
"/test",
mockUrlContentFetcher,
mockFileContextTracker,
mockRooIgnoreController,
true,
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
undefined,
)
})
it("should handle UNLIMITED_LINES constant correctly", async () => {
const userContent = [
{
type: "text" as const,
text: "<task>Read unlimited lines</task>",
},
]
await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
rooIgnoreController: mockRooIgnoreController,
maxReadFileLine: -1,
})
expect(parseMentions).toHaveBeenCalledWith(
"<task>Read unlimited lines</task>",
"/test",
mockUrlContentFetcher,
mockFileContextTracker,
mockRooIgnoreController,
true,
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
-1,
)
})
})
describe("content processing", () => {
it("should process text blocks with <task> tags", async () => {
const userContent = [
{
type: "text" as const,
text: "<task>Do something</task>",
},
]
const result = await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
})
expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
type: "text",
text: "parsed: <task>Do something</task>",
})
})
it("should process text blocks with <feedback> tags", async () => {
const userContent = [
{
type: "text" as const,
text: "<feedback>Fix this issue</feedback>",
},
]
const result = await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
})
expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
type: "text",
text: "parsed: <feedback>Fix this issue</feedback>",
})
})
it("should not process text blocks without task or feedback tags", async () => {
const userContent = [
{
type: "text" as const,
text: "Regular text without special tags",
},
]
const result = await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
})
expect(parseMentions).not.toHaveBeenCalled()
expect(result[0]).toEqual(userContent[0])
})
it("should process tool_result blocks with string content", async () => {
const userContent = [
{
type: "tool_result" as const,
tool_use_id: "123",
content: "<feedback>Tool feedback</feedback>",
},
]
const result = await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
})
expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
type: "tool_result",
tool_use_id: "123",
content: "parsed: <feedback>Tool feedback</feedback>",
})
})
it("should process tool_result blocks with array content", async () => {
const userContent = [
{
type: "tool_result" as const,
tool_use_id: "123",
content: [
{
type: "text" as const,
text: "<task>Array task</task>",
},
{
type: "text" as const,
text: "Regular text",
},
],
},
]
const result = await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
})
expect(parseMentions).toHaveBeenCalledTimes(1)
expect(result[0]).toEqual({
type: "tool_result",
tool_use_id: "123",
content: [
{
type: "text",
text: "parsed: <task>Array task</task>",
},
{
type: "text",
text: "Regular text",
},
],
})
})
it("should handle mixed content types", async () => {
const userContent = [
{
type: "text" as const,
text: "<task>First task</task>",
},
{
type: "image" as const,
source: {
type: "base64" as const,
media_type: "image/png" as const,
data: "base64data",
},
},
{
type: "tool_result" as const,
tool_use_id: "456",
content: "<feedback>Feedback</feedback>",
},
]
const result = await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
maxReadFileLine: 50,
})
expect(parseMentions).toHaveBeenCalledTimes(2)
expect(result).toHaveLength(3)
expect(result[0]).toEqual({
type: "text",
text: "parsed: <task>First task</task>",
})
expect(result[1]).toEqual(userContent[1]) // Image block unchanged
expect(result[2]).toEqual({
type: "tool_result",
tool_use_id: "456",
content: "parsed: <feedback>Feedback</feedback>",
})
})
})
describe("showRooIgnoredFiles parameter", () => {
it("should default showRooIgnoredFiles to true", async () => {
const userContent = [
{
type: "text" as const,
text: "<task>Test default</task>",
},
]
await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
})
expect(parseMentions).toHaveBeenCalledWith(
"<task>Test default</task>",
"/test",
mockUrlContentFetcher,
mockFileContextTracker,
undefined,
true, // showRooIgnoredFiles should default to true
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
undefined,
)
})
it("should respect showRooIgnoredFiles when explicitly set to false", async () => {
const userContent = [
{
type: "text" as const,
text: "<task>Test explicit false</task>",
},
]
await processUserContentMentions({
userContent,
cwd: "/test",
urlContentFetcher: mockUrlContentFetcher,
fileContextTracker: mockFileContextTracker,
showRooIgnoredFiles: false,
})
expect(parseMentions).toHaveBeenCalledWith(
"<task>Test explicit false</task>",
"/test",
mockUrlContentFetcher,
mockFileContextTracker,
undefined,
false,
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
undefined,
)
})
})
})

View file

@ -82,6 +82,7 @@ export async function parseMentions(
showRooIgnoredFiles: boolean = true,
includeDiagnosticMessages: boolean = true,
maxDiagnosticMessages: number = 50,
maxReadFileLine?: number,
): Promise<string> {
const mentions: Set<string> = new Set()
let parsedText = text.replace(mentionRegexGlobal, (match, mention) => {
@ -149,7 +150,13 @@ export async function parseMentions(
} else if (mention.startsWith("/")) {
const mentionPath = mention.slice(1)
try {
const content = await getFileOrFolderContent(mentionPath, cwd, rooIgnoreController, showRooIgnoredFiles)
const content = await getFileOrFolderContent(
mentionPath,
cwd,
rooIgnoreController,
showRooIgnoredFiles,
maxReadFileLine,
)
if (mention.endsWith("/")) {
parsedText += `\n\n<folder_content path="${mentionPath}">\n${content}\n</folder_content>`
} else {
@ -212,6 +219,7 @@ async function getFileOrFolderContent(
cwd: string,
rooIgnoreController?: any,
showRooIgnoredFiles: boolean = true,
maxReadFileLine?: number,
): Promise<string> {
const unescapedPath = unescapeSpaces(mentionPath)
const absPath = path.resolve(cwd, unescapedPath)
@ -224,7 +232,7 @@ async function getFileOrFolderContent(
return `(File ${mentionPath} is ignored by .rooignore)`
}
try {
const content = await extractTextFromFile(absPath)
const content = await extractTextFromFile(absPath, maxReadFileLine)
return content
} catch (error) {
return `(Failed to read contents of ${mentionPath}): ${error.message}`
@ -264,7 +272,7 @@ async function getFileOrFolderContent(
if (isBinary) {
return undefined
}
const content = await extractTextFromFile(absoluteFilePath)
const content = await extractTextFromFile(absoluteFilePath, maxReadFileLine)
return `<file_content path="${filePath.toPosix()}">\n${content}\n</file_content>`
} catch (error) {
return undefined

View file

@ -15,6 +15,7 @@ export async function processUserContentMentions({
showRooIgnoredFiles = true,
includeDiagnosticMessages = true,
maxDiagnosticMessages = 50,
maxReadFileLine,
}: {
userContent: Anthropic.Messages.ContentBlockParam[]
cwd: string
@ -24,6 +25,7 @@ export async function processUserContentMentions({
showRooIgnoredFiles?: boolean
includeDiagnosticMessages?: boolean
maxDiagnosticMessages?: number
maxReadFileLine?: number
}) {
// Process userContent array, which contains various block types:
// TextBlockParam, ImageBlockParam, ToolUseBlockParam, and ToolResultBlockParam.
@ -52,6 +54,7 @@ export async function processUserContentMentions({
showRooIgnoredFiles,
includeDiagnosticMessages,
maxDiagnosticMessages,
maxReadFileLine,
),
}
}
@ -71,6 +74,7 @@ export async function processUserContentMentions({
showRooIgnoredFiles,
includeDiagnosticMessages,
maxDiagnosticMessages,
maxReadFileLine,
),
}
}
@ -91,6 +95,7 @@ export async function processUserContentMentions({
showRooIgnoredFiles,
includeDiagnosticMessages,
maxDiagnosticMessages,
maxReadFileLine,
),
}
}

View file

@ -1230,6 +1230,7 @@ export class Task extends EventEmitter<ClineEvents> {
showRooIgnoredFiles = true,
includeDiagnosticMessages = true,
maxDiagnosticMessages = 50,
maxReadFileLine = -1,
} = (await this.providerRef.deref()?.getState()) ?? {}
const parsedUserContent = await processUserContentMentions({
@ -1241,6 +1242,7 @@ export class Task extends EventEmitter<ClineEvents> {
showRooIgnoredFiles,
includeDiagnosticMessages,
maxDiagnosticMessages,
maxReadFileLine,
})
const environmentDetails = await getEnvironmentDetails(this, includeFileDetails)

View file

@ -0,0 +1,221 @@
// npx vitest run integrations/misc/__tests__/extract-text-large-files.spec.ts
import { describe, it, expect, vi, beforeEach, Mock } from "vitest"
import * as fs from "fs/promises"
import { extractTextFromFile } from "../extract-text"
import { countFileLines } from "../line-counter"
import { readLines } from "../read-lines"
import { isBinaryFile } from "isbinaryfile"
// Mock all dependencies
vi.mock("fs/promises")
vi.mock("../line-counter")
vi.mock("../read-lines")
vi.mock("isbinaryfile")
describe("extractTextFromFile - Large File Handling", () => {
// Type the mocks
const mockedFs = vi.mocked(fs)
const mockedCountFileLines = vi.mocked(countFileLines)
const mockedReadLines = vi.mocked(readLines)
const mockedIsBinaryFile = vi.mocked(isBinaryFile)
beforeEach(() => {
vi.clearAllMocks()
// Set default mock behavior
mockedFs.access.mockResolvedValue(undefined)
mockedIsBinaryFile.mockResolvedValue(false)
})
it("should truncate files that exceed maxReadFileLine limit", async () => {
const largeFileContent = Array(150)
.fill(null)
.map((_, i) => `Line ${i + 1}: This is a test line with some content`)
.join("\n")
mockedCountFileLines.mockResolvedValue(150)
mockedReadLines.mockResolvedValue(
Array(100)
.fill(null)
.map((_, i) => `Line ${i + 1}: This is a test line with some content`)
.join("\n"),
)
const result = await extractTextFromFile("/test/large-file.ts", 100)
// Should only include first 100 lines with line numbers
expect(result).toContain(" 1 | Line 1: This is a test line with some content")
expect(result).toContain("100 | Line 100: This is a test line with some content")
expect(result).not.toContain("101 | Line 101: This is a test line with some content")
// Should include truncation message
expect(result).toContain(
"[File truncated: showing 100 of 150 total lines. The file is too large and may exhaust the context window if read in full.]",
)
})
it("should not truncate files within the maxReadFileLine limit", async () => {
const smallFileContent = Array(50)
.fill(null)
.map((_, i) => `Line ${i + 1}: This is a test line`)
.join("\n")
mockedCountFileLines.mockResolvedValue(50)
mockedFs.readFile.mockResolvedValue(smallFileContent as any)
const result = await extractTextFromFile("/test/small-file.ts", 100)
// Should include all lines with line numbers
expect(result).toContain(" 1 | Line 1: This is a test line")
expect(result).toContain("50 | Line 50: This is a test line")
// Should not include truncation message
expect(result).not.toContain("[File truncated:")
})
it("should handle files with exactly maxReadFileLine lines", async () => {
const exactFileContent = Array(100)
.fill(null)
.map((_, i) => `Line ${i + 1}`)
.join("\n")
mockedCountFileLines.mockResolvedValue(100)
mockedFs.readFile.mockResolvedValue(exactFileContent as any)
const result = await extractTextFromFile("/test/exact-file.ts", 100)
// Should include all lines with line numbers
expect(result).toContain(" 1 | Line 1")
expect(result).toContain("100 | Line 100")
// Should not include truncation message
expect(result).not.toContain("[File truncated:")
})
it("should handle undefined maxReadFileLine by not truncating", async () => {
const largeFileContent = Array(200)
.fill(null)
.map((_, i) => `Line ${i + 1}`)
.join("\n")
mockedFs.readFile.mockResolvedValue(largeFileContent as any)
const result = await extractTextFromFile("/test/large-file.ts", undefined)
// Should include all lines with line numbers when maxReadFileLine is undefined
expect(result).toContain(" 1 | Line 1")
expect(result).toContain("200 | Line 200")
// Should not include truncation message
expect(result).not.toContain("[File truncated:")
})
it("should handle empty files", async () => {
mockedFs.readFile.mockResolvedValue("" as any)
const result = await extractTextFromFile("/test/empty-file.ts", 100)
expect(result).toBe("")
expect(result).not.toContain("[File truncated:")
})
it("should handle files with only newlines", async () => {
const newlineOnlyContent = "\n\n\n\n\n"
mockedCountFileLines.mockResolvedValue(6) // 5 newlines = 6 lines
mockedReadLines.mockResolvedValue("\n\n")
const result = await extractTextFromFile("/test/newline-file.ts", 3)
// Should truncate at line 3
expect(result).toContain("[File truncated: showing 3 of 6 total lines")
})
it("should handle very large files efficiently", async () => {
// Simulate a 10,000 line file
mockedCountFileLines.mockResolvedValue(10000)
mockedReadLines.mockResolvedValue(
Array(500)
.fill(null)
.map((_, i) => `Line ${i + 1}: Some content here`)
.join("\n"),
)
const result = await extractTextFromFile("/test/very-large-file.ts", 500)
// Should only include first 500 lines with line numbers
expect(result).toContain(" 1 | Line 1: Some content here")
expect(result).toContain("500 | Line 500: Some content here")
expect(result).not.toContain("501 | Line 501: Some content here")
// Should show truncation message
expect(result).toContain("[File truncated: showing 500 of 10000 total lines")
})
it("should handle maxReadFileLine of 0 by throwing an error", async () => {
const fileContent = "Line 1\nLine 2\nLine 3"
mockedFs.readFile.mockResolvedValue(fileContent as any)
// maxReadFileLine of 0 should throw an error
await expect(extractTextFromFile("/test/file.ts", 0)).rejects.toThrow(
"Invalid maxReadFileLine: 0. Must be a positive integer or -1 for unlimited.",
)
})
it("should handle negative maxReadFileLine by treating as undefined", async () => {
const fileContent = "Line 1\nLine 2\nLine 3"
mockedFs.readFile.mockResolvedValue(fileContent as any)
const result = await extractTextFromFile("/test/file.ts", -1)
// Should include all content with line numbers when negative
expect(result).toContain("1 | Line 1")
expect(result).toContain("2 | Line 2")
expect(result).toContain("3 | Line 3")
expect(result).not.toContain("[File truncated:")
})
it("should preserve file content structure when truncating", async () => {
const structuredContent = [
"function example() {",
" const x = 1;",
" const y = 2;",
" return x + y;",
"}",
"",
"// More code below",
].join("\n")
mockedCountFileLines.mockResolvedValue(7)
mockedReadLines.mockResolvedValue(["function example() {", " const x = 1;", " const y = 2;"].join("\n"))
const result = await extractTextFromFile("/test/structured.ts", 3)
// Should preserve the first 3 lines with line numbers
expect(result).toContain("1 | function example() {")
expect(result).toContain("2 | const x = 1;")
expect(result).toContain("3 | const y = 2;")
expect(result).not.toContain("4 | return x + y;")
// Should include truncation info
expect(result).toContain("[File truncated: showing 3 of 7 total lines")
})
it("should handle binary files by throwing an error", async () => {
mockedIsBinaryFile.mockResolvedValue(true)
await expect(extractTextFromFile("/test/binary.bin", 100)).rejects.toThrow(
"Cannot read text for file type: .bin",
)
})
it("should handle file not found errors", async () => {
mockedFs.access.mockRejectedValue(new Error("ENOENT"))
await expect(extractTextFromFile("/test/nonexistent.ts", 100)).rejects.toThrow(
"File not found: /test/nonexistent.ts",
)
})
})

View file

@ -5,6 +5,8 @@ import mammoth from "mammoth"
import fs from "fs/promises"
import { isBinaryFile } from "isbinaryfile"
import { extractTextFromXLSX } from "./extract-text-from-xlsx"
import { countFileLines } from "./line-counter"
import { readLines } from "./read-lines"
async function extractTextFromPDF(filePath: string): Promise<string> {
const dataBuffer = await fs.readFile(filePath)
@ -48,7 +50,27 @@ export function getSupportedBinaryFormats(): string[] {
return Object.keys(SUPPORTED_BINARY_FORMATS)
}
export async function extractTextFromFile(filePath: string): Promise<string> {
/**
* Extracts text content from a file, with support for various formats including PDF, DOCX, XLSX, and plain text.
* For large text files, can limit the number of lines read to prevent context exhaustion.
*
* @param filePath - Path to the file to extract text from
* @param maxReadFileLine - Maximum number of lines to read from text files.
* Use UNLIMITED_LINES (-1) or undefined for no limit.
* Must be a positive integer or UNLIMITED_LINES.
* @returns Promise resolving to the extracted text content with line numbers
* @throws {Error} If file not found, unsupported format, or invalid parameters
*/
export async function extractTextFromFile(filePath: string, maxReadFileLine?: number): Promise<string> {
// Validate maxReadFileLine parameter
if (maxReadFileLine !== undefined && maxReadFileLine !== -1) {
if (!Number.isInteger(maxReadFileLine) || maxReadFileLine < 1) {
throw new Error(
`Invalid maxReadFileLine: ${maxReadFileLine}. Must be a positive integer or -1 for unlimited.`,
)
}
}
try {
await fs.access(filePath)
} catch (error) {
@ -67,6 +89,20 @@ export async function extractTextFromFile(filePath: string): Promise<string> {
const isBinary = await isBinaryFile(filePath).catch(() => false)
if (!isBinary) {
// Check if we need to apply line limit
if (maxReadFileLine !== undefined && maxReadFileLine !== -1) {
const totalLines = await countFileLines(filePath)
if (totalLines > maxReadFileLine) {
// Read only up to maxReadFileLine (endLine is 0-based and inclusive)
const content = await readLines(filePath, maxReadFileLine - 1, 0)
const numberedContent = addLineNumbers(content)
return (
numberedContent +
`\n\n[File truncated: showing ${maxReadFileLine} of ${totalLines} total lines. The file is too large and may exhaust the context window if read in full.]`
)
}
}
// Read the entire file if no limit or file is within limit
return addLineNumbers(await fs.readFile(filePath, "utf8"))
} else {
throw new Error(`Cannot read text for file type: ${fileExtension}`)