Skip to content

Commit 7980cd3

Browse files
fix: enforce maxConcurrentFileReads limit in read_file tool (#10363)
Co-authored-by: Roo Code <[email protected]>
1 parent ba1dd3c commit 7980cd3

File tree

2 files changed

+197
-0
lines changed

2 files changed

+197
-0
lines changed

src/core/tools/ReadFileTool.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,18 @@ export class ReadFileTool extends BaseTool<"read_file"> {
123123
return
124124
}
125125

126+
// Enforce maxConcurrentFileReads limit
127+
const { maxConcurrentFileReads = 5 } = (await task.providerRef.deref()?.getState()) ?? {}
128+
if (fileEntries.length > maxConcurrentFileReads) {
129+
task.consecutiveMistakeCount++
130+
task.recordToolError("read_file")
131+
const errorMsg = `Too many files requested. You attempted to read ${fileEntries.length} files, but the concurrent file reads limit is ${maxConcurrentFileReads}. Please read files in batches of ${maxConcurrentFileReads} or fewer.`
132+
await task.say("error", errorMsg)
133+
const errorResult = useNative ? `Error: ${errorMsg}` : `<files><error>${errorMsg}</error></files>`
134+
pushToolResult(errorResult)
135+
return
136+
}
137+
126138
const supportsImages = modelInfo.supportsImages ?? false
127139

128140
const fileResults: FileResult[] = fileEntries.map((entry) => ({

src/core/tools/__tests__/readFileTool.spec.ts

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1771,3 +1771,188 @@ describe("read_file tool with image support", () => {
17711771
})
17721772
})
17731773
})
1774+
1775+
describe("read_file tool concurrent file reads limit", () => {
1776+
const mockedCountFileLines = vi.mocked(countFileLines)
1777+
const mockedIsBinaryFile = vi.mocked(isBinaryFile)
1778+
const mockedPathResolve = vi.mocked(path.resolve)
1779+
1780+
let mockCline: any
1781+
let mockProvider: any
1782+
let toolResult: ToolResponse | undefined
1783+
1784+
beforeEach(() => {
1785+
// Clear specific mocks
1786+
mockedCountFileLines.mockClear()
1787+
mockedIsBinaryFile.mockClear()
1788+
mockedPathResolve.mockClear()
1789+
addLineNumbersMock.mockClear()
1790+
toolResultMock.mockClear()
1791+
1792+
// Use shared mock setup function
1793+
const mocks = createMockCline()
1794+
mockCline = mocks.mockCline
1795+
mockProvider = mocks.mockProvider
1796+
1797+
// Disable image support for these tests
1798+
setImageSupport(mockCline, false)
1799+
1800+
mockedPathResolve.mockImplementation((cwd, relPath) => `/${relPath}`)
1801+
mockedIsBinaryFile.mockResolvedValue(false)
1802+
mockedCountFileLines.mockResolvedValue(10)
1803+
1804+
toolResult = undefined
1805+
})
1806+
1807+
async function executeReadFileToolWithLimit(
1808+
fileCount: number,
1809+
maxConcurrentFileReads: number,
1810+
): Promise<ToolResponse | undefined> {
1811+
// Setup provider state with the specified limit
1812+
mockProvider.getState.mockResolvedValue({
1813+
maxReadFileLine: -1,
1814+
maxConcurrentFileReads,
1815+
maxImageFileSize: 20,
1816+
maxTotalImageSize: 20,
1817+
})
1818+
1819+
// Create args with the specified number of files
1820+
const files = Array.from({ length: fileCount }, (_, i) => `<file><path>file${i + 1}.txt</path></file>`)
1821+
const argsContent = files.join("")
1822+
1823+
const toolUse: ReadFileToolUse = {
1824+
type: "tool_use",
1825+
name: "read_file",
1826+
params: { args: argsContent },
1827+
partial: false,
1828+
}
1829+
1830+
// Configure mocks for successful file reads
1831+
mockReadFileWithTokenBudget.mockResolvedValue({
1832+
content: "test content",
1833+
tokenCount: 10,
1834+
lineCount: 1,
1835+
complete: true,
1836+
})
1837+
1838+
await readFileTool.handle(mockCline, toolUse, {
1839+
askApproval: mockCline.ask,
1840+
handleError: vi.fn(),
1841+
pushToolResult: (result: ToolResponse) => {
1842+
toolResult = result
1843+
},
1844+
removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
1845+
toolProtocol: "xml",
1846+
})
1847+
1848+
return toolResult
1849+
}
1850+
1851+
it("should reject when file count exceeds maxConcurrentFileReads", async () => {
1852+
// Try to read 6 files when limit is 5
1853+
const result = await executeReadFileToolWithLimit(6, 5)
1854+
1855+
// Verify error result
1856+
expect(result).toContain("Error: Too many files requested")
1857+
expect(result).toContain("You attempted to read 6 files")
1858+
expect(result).toContain("but the concurrent file reads limit is 5")
1859+
expect(result).toContain("Please read files in batches of 5 or fewer")
1860+
1861+
// Verify error tracking
1862+
expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Too many files requested"))
1863+
})
1864+
1865+
it("should allow reading files when count equals maxConcurrentFileReads", async () => {
1866+
// Try to read exactly 5 files when limit is 5
1867+
const result = await executeReadFileToolWithLimit(5, 5)
1868+
1869+
// Should not contain error
1870+
expect(result).not.toContain("Error: Too many files requested")
1871+
1872+
// Should contain file results
1873+
expect(typeof result === "string" ? result : JSON.stringify(result)).toContain("file1.txt")
1874+
})
1875+
1876+
it("should allow reading files when count is below maxConcurrentFileReads", async () => {
1877+
// Try to read 3 files when limit is 5
1878+
const result = await executeReadFileToolWithLimit(3, 5)
1879+
1880+
// Should not contain error
1881+
expect(result).not.toContain("Error: Too many files requested")
1882+
1883+
// Should contain file results
1884+
expect(typeof result === "string" ? result : JSON.stringify(result)).toContain("file1.txt")
1885+
})
1886+
1887+
it("should respect custom maxConcurrentFileReads value of 1", async () => {
1888+
// Try to read 2 files when limit is 1
1889+
const result = await executeReadFileToolWithLimit(2, 1)
1890+
1891+
// Verify error result with limit of 1
1892+
expect(result).toContain("Error: Too many files requested")
1893+
expect(result).toContain("You attempted to read 2 files")
1894+
expect(result).toContain("but the concurrent file reads limit is 1")
1895+
})
1896+
1897+
it("should allow single file read when maxConcurrentFileReads is 1", async () => {
1898+
// Try to read 1 file when limit is 1
1899+
const result = await executeReadFileToolWithLimit(1, 1)
1900+
1901+
// Should not contain error
1902+
expect(result).not.toContain("Error: Too many files requested")
1903+
1904+
// Should contain file result
1905+
expect(typeof result === "string" ? result : JSON.stringify(result)).toContain("file1.txt")
1906+
})
1907+
1908+
it("should respect higher maxConcurrentFileReads value", async () => {
1909+
// Try to read 15 files when limit is 10
1910+
const result = await executeReadFileToolWithLimit(15, 10)
1911+
1912+
// Verify error result
1913+
expect(result).toContain("Error: Too many files requested")
1914+
expect(result).toContain("You attempted to read 15 files")
1915+
expect(result).toContain("but the concurrent file reads limit is 10")
1916+
})
1917+
1918+
it("should use default value of 5 when maxConcurrentFileReads is not set", async () => {
1919+
// Setup provider state without maxConcurrentFileReads
1920+
mockProvider.getState.mockResolvedValue({
1921+
maxReadFileLine: -1,
1922+
maxImageFileSize: 20,
1923+
maxTotalImageSize: 20,
1924+
})
1925+
1926+
// Create args with 6 files
1927+
const files = Array.from({ length: 6 }, (_, i) => `<file><path>file${i + 1}.txt</path></file>`)
1928+
const argsContent = files.join("")
1929+
1930+
const toolUse: ReadFileToolUse = {
1931+
type: "tool_use",
1932+
name: "read_file",
1933+
params: { args: argsContent },
1934+
partial: false,
1935+
}
1936+
1937+
mockReadFileWithTokenBudget.mockResolvedValue({
1938+
content: "test content",
1939+
tokenCount: 10,
1940+
lineCount: 1,
1941+
complete: true,
1942+
})
1943+
1944+
await readFileTool.handle(mockCline, toolUse, {
1945+
askApproval: mockCline.ask,
1946+
handleError: vi.fn(),
1947+
pushToolResult: (result: ToolResponse) => {
1948+
toolResult = result
1949+
},
1950+
removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
1951+
toolProtocol: "xml",
1952+
})
1953+
1954+
// Should use default limit of 5 and reject 6 files
1955+
expect(toolResult).toContain("Error: Too many files requested")
1956+
expect(toolResult).toContain("but the concurrent file reads limit is 5")
1957+
})
1958+
})

0 commit comments

Comments
 (0)