From d0773e9bb549035ad135b322e80bbbb38ea6b9d7 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 25 Nov 2025 10:30:30 +0000 Subject: [PATCH] fix: respect line_ranges parameter in read_file tool with maxReadFileLine - Add transformNativeFileEntries method to convert snake_case line_ranges to camelCase lineRanges - Transform native protocol format (line_ranges as string arrays) to internal format (lineRanges as LineRange objects) - Add comprehensive tests for native protocol line_ranges handling Fixes #9566 --- src/core/tools/ReadFileTool.ts | 41 +++- src/core/tools/__tests__/readFileTool.spec.ts | 211 ++++++++++++++++++ 2 files changed, 251 insertions(+), 1 deletion(-) diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 9dff9502bd..ba058b7dd8 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/src/core/tools/ReadFileTool.ts @@ -107,7 +107,8 @@ export class ReadFileTool extends BaseTool<"read_file"> { async execute(params: { files: FileEntry[] }, task: Task, callbacks: ToolCallbacks): Promise { const { handleError, pushToolResult, toolProtocol } = callbacks - const fileEntries = params.files + // Transform native protocol format if necessary + const fileEntries = this.transformNativeFileEntries(params.files) const modelInfo = task.api.getModel().info const protocol = resolveToolProtocol(task.apiConfiguration, modelInfo) const useNative = isNativeProtocol(protocol) @@ -706,6 +707,44 @@ export class ReadFileTool extends BaseTool<"read_file"> { return `[${blockName} with missing path/args/files]` } + /** + * Transform native protocol file entries to the expected format. + * Native protocol uses snake_case `line_ranges` as string arrays, + * but our internal format uses camelCase `lineRanges` as LineRange objects. + */ + private transformNativeFileEntries(files: any[]): FileEntry[] { + return files.map((file) => { + const entry: FileEntry = { + path: file.path, + } + + // Transform line_ranges (snake_case, string[]) to lineRanges (camelCase, LineRange[]) + if (file.line_ranges && Array.isArray(file.line_ranges)) { + entry.lineRanges = [] + for (const rangeStr of file.line_ranges) { + const match = String(rangeStr).match(/^(\d+)-(\d+)$/) + if (match) { + const [, startStr, endStr] = match + const start = parseInt(startStr, 10) + const end = parseInt(endStr, 10) + if (!isNaN(start) && !isNaN(end) && start > 0 && end > 0) { + entry.lineRanges.push({ start, end }) + } + } + } + // Only keep lineRanges if we successfully parsed at least one range + if (entry.lineRanges.length === 0) { + delete entry.lineRanges + } + } else if (file.lineRanges && Array.isArray(file.lineRanges)) { + // Already in the correct format (camelCase with LineRange objects) + entry.lineRanges = file.lineRanges + } + + return entry + }) + } + override async handlePartial(task: Task, block: ToolUse<"read_file">): Promise { const argsXmlTag = block.params.args const legacyPath = block.params.path diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index 11f533e54f..698ba83b07 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -1726,3 +1726,214 @@ describe("read_file tool with image support", () => { }) }) }) + +describe("read_file tool with native protocol line_ranges", () => { + // Test that the native protocol format (snake_case line_ranges as strings) works correctly + const testFilePath = "test/large-file.txt" + const absoluteFilePath = "/test/large-file.txt" + + const mockedCountFileLines = vi.mocked(countFileLines) + const mockedReadLines = vi.mocked(readLines) + const mockedIsBinaryFile = vi.mocked(isBinaryFile) + const mockedPathResolve = vi.mocked(path.resolve) + + let mockCline: any + let mockProvider: any + let toolResult: ToolResponse | undefined + + beforeEach(() => { + // Clear specific mocks + mockedCountFileLines.mockClear() + mockedReadLines.mockClear() + mockedIsBinaryFile.mockClear() + mockedPathResolve.mockClear() + addLineNumbersMock.mockClear() + + // Use shared mock setup function + const mocks = createMockCline() + mockCline = mocks.mockCline + mockProvider = mocks.mockProvider + + // Set up native protocol support + mockCline.api = { + getModel: vi.fn().mockReturnValue({ + info: { + supportsImages: false, + contextWindow: 200000, + maxTokens: 4096, + supportsPromptCache: false, + supportsNativeTools: true, // Enable native tools support + }, + }), + } + + mockedPathResolve.mockReturnValue(absoluteFilePath) + mockedIsBinaryFile.mockResolvedValue(false) + + // Mock addLineNumbers to add line numbers to content + addLineNumbersMock.mockImplementation((content, startLine = 1) => { + if (!content) return "" + const lines = content.split("\n") + return lines.map((line: string, i: number) => `${startLine + i} | ${line}`).join("\n") + }) + + toolResult = undefined + }) + + it("should respect line_ranges in native protocol format even with maxReadFileLine set", async () => { + // Setup - file with 472 lines, maxReadFileLine set to 250 + mockedCountFileLines.mockResolvedValue(472) + mockProvider.getState.mockResolvedValue({ maxReadFileLine: 250 }) + + // Mock the specific lines we want to read (251-350) + const requestedContent = Array.from({ length: 100 }, (_, i) => `Line ${251 + i}`).join("\n") + mockedReadLines.mockResolvedValue(requestedContent) + + // Mock addLineNumbers from extract-text module + const { addLineNumbers } = await import("../../../integrations/misc/extract-text") + vi.mocked(addLineNumbers).mockImplementation((content, startLine = 1) => { + if (!content) return "" + const lines = content.split("\n") + return lines.map((line: string, i: number) => `${startLine + i} | ${line}`).join("\n") + }) + + // Create a tool use with native protocol format (snake_case line_ranges as string array) + const toolUse: ReadFileToolUse = { + type: "tool_use", + name: "read_file", + params: {}, // Native protocol doesn't use params + partial: false, + nativeArgs: { + files: [ + { + path: testFilePath, + line_ranges: ["251-350"], // Snake_case, string array format + } as any, // Use any to bypass TypeScript for native protocol format + ], + } as any, + } + + // Execute the tool + await readFileTool.handle(mockCline, toolUse, { + askApproval: mockCline.ask, + handleError: vi.fn(), + pushToolResult: (result: ToolResponse) => { + toolResult = result + }, + removeClosingTag: (_: ToolParamName, content?: string) => content ?? "", + toolProtocol: "native", + }) + + // Verify that readLines was called with the correct range + expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 349, 250) // 0-indexed: lines 251-350 + + // Verify the result contains the requested lines, not the first 250 lines + expect(toolResult).toBeDefined() + // For native protocol, the result format is different + expect(toolResult).toContain("251-350") + expect(toolResult).toContain("251 |") // Check for line number prefix + expect(toolResult).not.toContain("Showing only 250 of 472 total lines") + }) + + it("should handle multiple line_ranges in native protocol format", async () => { + // Setup - file with 500 lines + mockedCountFileLines.mockResolvedValue(500) + mockProvider.getState.mockResolvedValue({ maxReadFileLine: 100 }) + + // Create a tool use with multiple ranges + const toolUse: ReadFileToolUse = { + type: "tool_use", + name: "read_file", + params: {}, + partial: false, + nativeArgs: { + files: [ + { + path: testFilePath, + line_ranges: ["10-20", "300-310", "450-460"], // Multiple ranges + } as any, // Use any to bypass TypeScript for native protocol format + ], + } as any, + } + + // Mock the readLines to return different content for each range + mockedReadLines + .mockResolvedValueOnce("Lines 10-20 content") + .mockResolvedValueOnce("Lines 300-310 content") + .mockResolvedValueOnce("Lines 450-460 content") + + // Mock addLineNumbers from extract-text module + const { addLineNumbers } = await import("../../../integrations/misc/extract-text") + vi.mocked(addLineNumbers).mockImplementation((content, startLine = 1) => { + if (!content) return "" + return `${startLine} | ${content}` + }) + + // Execute the tool + await readFileTool.handle(mockCline, toolUse, { + askApproval: mockCline.ask, + handleError: vi.fn(), + pushToolResult: (result: ToolResponse) => { + toolResult = result + }, + removeClosingTag: (_: ToolParamName, content?: string) => content ?? "", + toolProtocol: "native", + }) + + // Verify that readLines was called for each range + expect(mockedReadLines).toHaveBeenCalledTimes(3) + expect(mockedReadLines).toHaveBeenNthCalledWith(1, absoluteFilePath, 19, 9) // lines 10-20 + expect(mockedReadLines).toHaveBeenNthCalledWith(2, absoluteFilePath, 309, 299) // lines 300-310 + expect(mockedReadLines).toHaveBeenNthCalledWith(3, absoluteFilePath, 459, 449) // lines 450-460 + + // Verify the result contains all ranges (native protocol format) + expect(toolResult).toContain("10-20") + expect(toolResult).toContain("300-310") + expect(toolResult).toContain("450-460") + }) + + it("should handle mixed camelCase and snake_case formats", async () => { + // Setup + mockedCountFileLines.mockResolvedValue(100) + mockProvider.getState.mockResolvedValue({ maxReadFileLine: 50 }) + mockedReadLines.mockResolvedValue("Lines 60-70 content") + + // Mock addLineNumbers from extract-text module + const { addLineNumbers } = await import("../../../integrations/misc/extract-text") + vi.mocked(addLineNumbers).mockImplementation((content, startLine = 1) => { + if (!content) return "" + return `${startLine} | ${content}` + }) + + // Test with already camelCase format (should still work) + const toolUse: ReadFileToolUse = { + type: "tool_use", + name: "read_file", + params: {}, + partial: false, + nativeArgs: { + files: [ + { + path: testFilePath, + lineRanges: [{ start: 60, end: 70 }], // Already in camelCase with LineRange objects + }, + ], + }, + } + + // Execute the tool + await readFileTool.handle(mockCline, toolUse, { + askApproval: mockCline.ask, + handleError: vi.fn(), + pushToolResult: (result: ToolResponse) => { + toolResult = result + }, + removeClosingTag: (_: ToolParamName, content?: string) => content ?? "", + toolProtocol: "native", + }) + + // Verify the camelCase format still works + expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 69, 59) // lines 60-70 + expect(toolResult).toContain("60-70") + }) +})