From b46f6adb78ded5d20529bb42000dc23beb79f4f8 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 28 Mar 2025 11:47:47 -0400 Subject: [PATCH] Extract code for read_file from Cline (#2059) --- src/core/Cline.ts | 150 +----- .../read-file-maxReadFileLine.test.ts | 444 ++++++++---------- src/core/tools/readFileTool.ts | 168 +++++++ src/core/tools/types.ts | 3 + 4 files changed, 374 insertions(+), 391 deletions(-) create mode 100644 src/core/tools/readFileTool.ts diff --git a/src/core/Cline.ts b/src/core/Cline.ts index ade6c37244..7618a640eb 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -30,6 +30,7 @@ import { } from "../integrations/misc/extract-text" import { countFileLines } from "../integrations/misc/line-counter" import { fetchInstructionsTool } from "./tools/fetchInstructionsTool" +import { readFileTool } from "./tools/readFileTool" import { ExitCodeDetails } from "../integrations/terminal/TerminalProcess" import { Terminal } from "../integrations/terminal/Terminal" import { TerminalRegistry } from "../integrations/terminal/TerminalRegistry" @@ -82,9 +83,7 @@ import { insertGroups } from "./diff/insert-groups" import { telemetryService } from "../services/telemetry/TelemetryService" import { validateToolUse, isToolAllowedForMode, ToolName } from "./mode-validator" import { parseXml } from "../utils/xml" -import { readLines } from "../integrations/misc/read-lines" import { getWorkspacePath } from "../utils/path" -import { isBinaryFile } from "isbinaryfile" export type ToolResponse = string | Array type UserContent = Array @@ -2256,151 +2255,8 @@ export class Cline extends EventEmitter { } case "read_file": { - const relPath: string | undefined = block.params.path - const startLineStr: string | undefined = block.params.start_line - const endLineStr: string | undefined = block.params.end_line - - // Get the full path and determine if it's outside the workspace - const fullPath = relPath ? path.resolve(this.cwd, removeClosingTag("path", relPath)) : "" - const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) - - const sharedMessageProps: ClineSayTool = { - tool: "readFile", - path: getReadablePath(this.cwd, removeClosingTag("path", relPath)), - isOutsideWorkspace, - } - try { - if (block.partial) { - const partialMessage = JSON.stringify({ - ...sharedMessageProps, - content: undefined, - } satisfies ClineSayTool) - await this.ask("tool", partialMessage, block.partial).catch(() => {}) - break - } else { - if (!relPath) { - this.consecutiveMistakeCount++ - pushToolResult(await this.sayAndCreateMissingParamError("read_file", "path")) - break - } - - // Check if we're doing a line range read - let isRangeRead = false - let startLine: number | undefined = undefined - let endLine: number | undefined = undefined - - // Check if we have either range parameter - if (startLineStr || endLineStr) { - isRangeRead = true - } - - // Parse start_line if provided - if (startLineStr) { - startLine = parseInt(startLineStr) - if (isNaN(startLine)) { - // Invalid start_line - this.consecutiveMistakeCount++ - await this.say("error", `Failed to parse start_line: ${startLineStr}`) - pushToolResult(formatResponse.toolError("Invalid start_line value")) - break - } - startLine -= 1 // Convert to 0-based index - } - - // Parse end_line if provided - if (endLineStr) { - endLine = parseInt(endLineStr) - - if (isNaN(endLine)) { - // Invalid end_line - this.consecutiveMistakeCount++ - await this.say("error", `Failed to parse end_line: ${endLineStr}`) - pushToolResult(formatResponse.toolError("Invalid end_line value")) - break - } - - // Convert to 0-based index - endLine -= 1 - } - - const accessAllowed = this.rooIgnoreController?.validateAccess(relPath) - if (!accessAllowed) { - await this.say("rooignore_error", relPath) - pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath))) - - break - } - - this.consecutiveMistakeCount = 0 - const absolutePath = path.resolve(this.cwd, relPath) - const completeMessage = JSON.stringify({ - ...sharedMessageProps, - content: absolutePath, - } satisfies ClineSayTool) - - const didApprove = await askApproval("tool", completeMessage) - if (!didApprove) { - break - } - - // Get the maxReadFileLine setting - const { maxReadFileLine = 500 } = (await this.providerRef.deref()?.getState()) ?? {} - - // Count total lines in the file - let totalLines = 0 - try { - totalLines = await countFileLines(absolutePath) - } catch (error) { - console.error(`Error counting lines in file ${absolutePath}:`, error) - } - - // now execute the tool like normal - let content: string - let isFileTruncated = false - let sourceCodeDef = "" - - const isBinary = await isBinaryFile(absolutePath).catch(() => false) - - if (isRangeRead) { - if (startLine === undefined) { - content = addLineNumbers(await readLines(absolutePath, endLine, startLine)) - } else { - content = addLineNumbers( - await readLines(absolutePath, endLine, startLine), - startLine + 1, - ) - } - } else if (!isBinary && maxReadFileLine >= 0 && totalLines > maxReadFileLine) { - // If file is too large, only read the first maxReadFileLine lines - isFileTruncated = true - - const res = await Promise.all([ - maxReadFileLine > 0 ? readLines(absolutePath, maxReadFileLine - 1, 0) : "", - parseSourceCodeDefinitionsForFile(absolutePath, this.rooIgnoreController), - ]) - - content = res[0].length > 0 ? addLineNumbers(res[0]) : "" - const result = res[1] - if (result) { - sourceCodeDef = `\n\n${result}` - } - } else { - // Read entire file - content = await extractTextFromFile(absolutePath) - } - - // Add truncation notice if applicable - if (isFileTruncated) { - content += `\n\n[Showing only ${maxReadFileLine} of ${totalLines} total lines. Use start_line and end_line if you need to read more]${sourceCodeDef}` - } - - pushToolResult(content) - break - } - } catch (error) { - await handleError("reading file", error) - break - } + readFileTool(this, block, askApproval, handleError, pushToolResult, removeClosingTag) + break } case "fetch_instructions": { diff --git a/src/core/__tests__/read-file-maxReadFileLine.test.ts b/src/core/__tests__/read-file-maxReadFileLine.test.ts index dbcacf39a2..619ae3a605 100644 --- a/src/core/__tests__/read-file-maxReadFileLine.test.ts +++ b/src/core/__tests__/read-file-maxReadFileLine.test.ts @@ -1,5 +1,3 @@ -const DEBUG = false - import * as path from "path" import { countFileLines } from "../../integrations/misc/line-counter" import { readLines } from "../../integrations/misc/read-lines" @@ -8,7 +6,6 @@ import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" import { isBinaryFile } from "isbinaryfile" import { ReadFileToolUse } from "../assistant-message" import { Cline } from "../Cline" -import { ClineProvider } from "../webview/ClineProvider" // Mock dependencies jest.mock("../../integrations/misc/line-counter") @@ -21,7 +18,7 @@ jest.mock("../ignore/RooIgnoreController", () => ({ initialize() { return Promise.resolve() } - validateAccess(filePath: string) { + validateAccess() { return true } }, @@ -45,281 +42,240 @@ jest.mock("path", () => { }) describe("read_file tool with maxReadFileLine setting", () => { - // Mock original implementation first to use in tests - const originalCountFileLines = jest.requireActual("../../integrations/misc/line-counter").countFileLines - const originalReadLines = jest.requireActual("../../integrations/misc/read-lines").readLines - const originalExtractTextFromFile = jest.requireActual("../../integrations/misc/extract-text").extractTextFromFile - const originalAddLineNumbers = jest.requireActual("../../integrations/misc/extract-text").addLineNumbers - const originalParseSourceCodeDefinitionsForFile = - jest.requireActual("../../services/tree-sitter").parseSourceCodeDefinitionsForFile - const originalIsBinaryFile = jest.requireActual("isbinaryfile").isBinaryFile - - let cline: Cline - let mockProvider: any + // Test data const testFilePath = "test/file.txt" - const absoluteFilePath = "/home/ewheeler/src/roo/roo-main/test/file.txt" + const absoluteFilePath = "/test/file.txt" const fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5" const numberedFileContent = "1 | Line 1\n2 | Line 2\n3 | Line 3\n4 | Line 4\n5 | Line 5" const sourceCodeDef = "\n\n# file.txt\n1--5 | Content" + // Mocked functions with correct types + const mockedCountFileLines = countFileLines as jest.MockedFunction + const mockedReadLines = readLines as jest.MockedFunction + const mockedExtractTextFromFile = extractTextFromFile as jest.MockedFunction + const mockedAddLineNumbers = addLineNumbers as jest.MockedFunction + const mockedParseSourceCodeDefinitionsForFile = parseSourceCodeDefinitionsForFile as jest.MockedFunction< + typeof parseSourceCodeDefinitionsForFile + > + const mockedIsBinaryFile = isBinaryFile as jest.MockedFunction + const mockedPathResolve = path.resolve as jest.MockedFunction + + // Mock instances + const mockCline: any = {} + let mockProvider: any + let toolResult: string | undefined + beforeEach(() => { - jest.resetAllMocks() + jest.clearAllMocks() - // Reset mocks to simulate original behavior - ;(countFileLines as jest.Mock).mockImplementation(originalCountFileLines) - ;(readLines as jest.Mock).mockImplementation(originalReadLines) - ;(extractTextFromFile as jest.Mock).mockImplementation(originalExtractTextFromFile) - ;(parseSourceCodeDefinitionsForFile as jest.Mock).mockImplementation(originalParseSourceCodeDefinitionsForFile) - ;(isBinaryFile as jest.Mock).mockImplementation(originalIsBinaryFile) + // Setup path resolution + mockedPathResolve.mockReturnValue(absoluteFilePath) - // Default mock implementations - ;(countFileLines as jest.Mock).mockResolvedValue(5) - ;(readLines as jest.Mock).mockResolvedValue(fileContent) - ;(extractTextFromFile as jest.Mock).mockResolvedValue(numberedFileContent) - // Use the real addLineNumbers function - ;(addLineNumbers as jest.Mock).mockImplementation(originalAddLineNumbers) - ;(parseSourceCodeDefinitionsForFile as jest.Mock).mockResolvedValue(sourceCodeDef) - ;(isBinaryFile as jest.Mock).mockResolvedValue(false) + // Setup mocks for file operations + mockedIsBinaryFile.mockResolvedValue(false) + mockedAddLineNumbers.mockImplementation((content: string, startLine = 1) => { + return content + .split("\n") + .map((line, i) => `${i + startLine} | ${line}`) + .join("\n") + }) - // Add spy to debug the readLines calls - const readLinesSpy = jest.spyOn(require("../../integrations/misc/read-lines"), "readLines") - - // Mock path.resolve to return a predictable path - ;(path.resolve as jest.Mock).mockReturnValue(absoluteFilePath) - - // Create mock provider + // Setup mock provider mockProvider = { getState: jest.fn(), deref: jest.fn().mockReturnThis(), } - // Create a Cline instance with the necessary configuration - cline = new Cline({ - provider: mockProvider, - apiConfiguration: { apiProvider: "anthropic" } as any, - task: "Test read_file tool", // Required to satisfy constructor check - startTask: false, // Prevent actual task initialization - }) + // Setup Cline instance with mock methods + mockCline.cwd = "/" + mockCline.task = "Test" + mockCline.providerRef = mockProvider + mockCline.rooIgnoreController = { + validateAccess: jest.fn().mockReturnValue(true), + } + mockCline.say = jest.fn().mockResolvedValue(undefined) + mockCline.ask = jest.fn().mockResolvedValue(true) + mockCline.presentAssistantMessage = jest.fn() - // Set up the read_file tool use - const readFileToolUse: ReadFileToolUse = { + // Reset tool result + toolResult = undefined + }) + + /** + * Helper function to execute the read file tool with different maxReadFileLine settings + */ + async function executeReadFileTool(maxReadFileLine: number, totalLines = 5): Promise { + // Configure mocks based on test scenario + mockProvider.getState.mockResolvedValue({ maxReadFileLine }) + mockedCountFileLines.mockResolvedValue(totalLines) + + // Create a tool use object + const toolUse: ReadFileToolUse = { type: "tool_use", name: "read_file", - params: { - path: testFilePath, - }, + params: { path: testFilePath }, partial: false, } - // Set up the Cline instance for testing - const clineAny = cline as any + // Import the tool implementation dynamically to avoid hoisting issues + const { readFileTool } = require("../tools/readFileTool") - // Set up the required properties for the test - clineAny.assistantMessageContent = [readFileToolUse] - clineAny.currentStreamingContentIndex = 0 - clineAny.userMessageContent = [] - clineAny.presentAssistantMessageLocked = false - clineAny.didCompleteReadingStream = true - clineAny.didRejectTool = false - clineAny.didAlreadyUseTool = false + // Execute the tool + await readFileTool( + mockCline, + toolUse, + mockCline.ask, + jest.fn(), + (result: string) => { + toolResult = result + }, + (param: string, value: string) => value, + ) - // Mock methods that would be called during presentAssistantMessage - clineAny.say = jest.fn().mockResolvedValue(undefined) - clineAny.ask = jest.fn().mockImplementation((type, message) => { - return Promise.resolve({ response: "yesButtonClicked" }) + return toolResult + } + + describe("when maxReadFileLine is negative", () => { + it("should read the entire file using extractTextFromFile", async () => { + // Setup + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(-1) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedReadLines).not.toHaveBeenCalled() + expect(mockedParseSourceCodeDefinitionsForFile).not.toHaveBeenCalled() + expect(result).toBe(numberedFileContent) }) }) - // Helper function to get user message content - const getUserMessageContent = (clineInstance: Cline) => { - const clineAny = clineInstance as any - return clineAny.userMessageContent - } + describe("when maxReadFileLine is 0", () => { + it("should return an empty content with source code definitions", async () => { + // Setup - for maxReadFileLine = 0, the implementation won't call readLines + mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(sourceCodeDef) - // Helper function to validate response lines - const validateResponseLines = ( - responseLines: string[], - options: { - expectedLineCount: number - shouldContainLines?: number[] - shouldNotContainLines?: number[] - }, - ) => { - if (options.shouldContainLines) { - const contentLines = responseLines.filter((line) => line.includes("Line ")) - expect(contentLines.length).toBe(options.expectedLineCount) - options.shouldContainLines.forEach((lineNum) => { - expect(contentLines[lineNum - 1]).toContain(`Line ${lineNum}`) - }) - } + // Execute + const result = await executeReadFileTool(0) - if (options.shouldNotContainLines) { - options.shouldNotContainLines.forEach((lineNum) => { - expect(responseLines.some((line) => line.includes(`Line ${lineNum}`))).toBe(false) - }) - } - } + // Verify + expect(mockedExtractTextFromFile).not.toHaveBeenCalled() + expect(mockedReadLines).not.toHaveBeenCalled() // Per implementation line 141 + expect(mockedParseSourceCodeDefinitionsForFile).toHaveBeenCalledWith( + absoluteFilePath, + mockCline.rooIgnoreController, + ) + expect(result).toContain("[Showing only 0 of 5 total lines") + expect(result).toContain(sourceCodeDef) + }) + }) - interface TestExpectations { - extractTextCalled: boolean - readLinesCalled: boolean - sourceCodeDefCalled: boolean - readLinesParams?: [string, number, number] - responseValidation: { - expectedLineCount: number - shouldContainLines?: number[] - shouldNotContainLines?: number[] - } - expectedContent?: string - truncationMessage?: string - includeSourceCodeDef?: boolean - } + describe("when maxReadFileLine is less than file length", () => { + it("should read only maxReadFileLine lines and add source code definitions", async () => { + // Setup + const content = "Line 1\nLine 2\nLine 3" + mockedReadLines.mockResolvedValue(content) + mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(sourceCodeDef) - interface TestCase { - name: string - maxReadFileLine: number - setup?: () => void - expectations: TestExpectations - } + // Execute + const result = await executeReadFileTool(3) - // Test cases - const testCases: TestCase[] = [ - { - name: "read entire file when maxReadFileLine is -1", - maxReadFileLine: -1, - expectations: { - extractTextCalled: true, - readLinesCalled: false, - sourceCodeDefCalled: false, - responseValidation: { - expectedLineCount: 5, - shouldContainLines: [1, 2, 3, 4, 5], + // Verify - check behavior but not specific implementation details + expect(mockedExtractTextFromFile).not.toHaveBeenCalled() + expect(mockedReadLines).toHaveBeenCalled() + expect(mockedParseSourceCodeDefinitionsForFile).toHaveBeenCalledWith( + absoluteFilePath, + mockCline.rooIgnoreController, + ) + expect(result).toContain("1 | Line 1") + expect(result).toContain("2 | Line 2") + expect(result).toContain("3 | Line 3") + expect(result).toContain("[Showing only 3 of 5 total lines") + expect(result).toContain(sourceCodeDef) + }) + }) + + describe("when maxReadFileLine equals or exceeds file length", () => { + it("should use extractTextFromFile when maxReadFileLine > totalLines", async () => { + // Setup + mockedCountFileLines.mockResolvedValue(5) // File shorter than maxReadFileLine + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(10, 5) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(result).toBe(numberedFileContent) + }) + + it("should read with extractTextFromFile when file has few lines", async () => { + // Setup + mockedCountFileLines.mockResolvedValue(3) // File shorter than maxReadFileLine + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(5, 3) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedReadLines).not.toHaveBeenCalled() + expect(result).toBe(numberedFileContent) + }) + }) + + describe("when file is binary", () => { + it("should always use extractTextFromFile regardless of maxReadFileLine", async () => { + // Setup + mockedIsBinaryFile.mockResolvedValue(true) + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(3) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedReadLines).not.toHaveBeenCalled() + expect(result).toBe(numberedFileContent) + }) + }) + + describe("with range parameters", () => { + it("should honor start_line and end_line when provided", async () => { + // Setup + const rangeToolUse: ReadFileToolUse = { + type: "tool_use", + name: "read_file", + params: { + path: testFilePath, + start_line: "2", + end_line: "4", }, - expectedContent: numberedFileContent, - }, - }, - { - name: "read entire file when maxReadFileLine >= file length", - maxReadFileLine: 10, - expectations: { - extractTextCalled: true, - readLinesCalled: false, - sourceCodeDefCalled: false, - responseValidation: { - expectedLineCount: 5, - shouldContainLines: [1, 2, 3, 4, 5], - }, - expectedContent: numberedFileContent, - }, - }, - { - name: "read zero lines and only provide line declaration definitions when maxReadFileLine is 0", - maxReadFileLine: 0, - expectations: { - extractTextCalled: false, - readLinesCalled: false, - sourceCodeDefCalled: true, - responseValidation: { - expectedLineCount: 0, - }, - truncationMessage: `[Showing only 0 of 5 total lines. Use start_line and end_line if you need to read more]`, - includeSourceCodeDef: true, - }, - }, - { - name: "read maxReadFileLine lines and provide line declaration definitions when maxReadFileLine < file length", - maxReadFileLine: 3, - setup: () => { - jest.clearAllMocks() - ;(countFileLines as jest.Mock).mockResolvedValue(5) - ;(readLines as jest.Mock).mockImplementation((path, endLine, startLine = 0) => { - const lines = fileContent.split("\n") - const actualEndLine = endLine !== undefined ? Math.min(endLine, lines.length - 1) : lines.length - 1 - const actualStartLine = startLine !== undefined ? Math.min(startLine, lines.length - 1) : 0 - const requestedLines = lines.slice(actualStartLine, actualEndLine + 1) - return Promise.resolve(requestedLines.join("\n")) - }) - }, - expectations: { - extractTextCalled: false, - readLinesCalled: true, - sourceCodeDefCalled: true, - readLinesParams: [absoluteFilePath, 2, 0], - responseValidation: { - expectedLineCount: 3, - shouldContainLines: [1, 2, 3], - shouldNotContainLines: [4, 5], - }, - truncationMessage: `[Showing only 3 of 5 total lines. Use start_line and end_line if you need to read more]`, - includeSourceCodeDef: true, - }, - }, - ] - - test.each(testCases)("should $name", async (testCase) => { - // Setup - if (testCase.setup) { - testCase.setup() - } - mockProvider.getState.mockResolvedValue({ maxReadFileLine: testCase.maxReadFileLine }) - - // Execute - await cline.presentAssistantMessage() - - // Verify mock calls - if (testCase.expectations.extractTextCalled) { - expect(extractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - } else { - expect(extractTextFromFile).not.toHaveBeenCalled() - } - - if (testCase.expectations.readLinesCalled) { - const params = testCase.expectations.readLinesParams - if (!params) { - throw new Error("readLinesParams must be defined when readLinesCalled is true") + partial: false, } - expect(readLines).toHaveBeenCalledWith(...params) - } else { - expect(readLines).not.toHaveBeenCalled() - } - if (testCase.expectations.sourceCodeDefCalled) { - expect(parseSourceCodeDefinitionsForFile).toHaveBeenCalled() - } else { - expect(parseSourceCodeDefinitionsForFile).not.toHaveBeenCalled() - } + mockedReadLines.mockResolvedValue("Line 2\nLine 3\nLine 4") - // Verify response content - const userMessageContent = getUserMessageContent(cline) + // Import the tool implementation dynamically + const { readFileTool } = require("../tools/readFileTool") - if (DEBUG) { - console.log(`\n=== Test: ${testCase.name} ===`) - console.log(`maxReadFileLine: ${testCase.maxReadFileLine}`) - console.log("Response content:", JSON.stringify(userMessageContent, null, 2)) - } - const responseLines = userMessageContent[1].text.split("\n") + // Execute the tool + let rangeResult: string | undefined + await readFileTool( + mockCline, + rangeToolUse, + mockCline.ask, + jest.fn(), + (result: string) => { + rangeResult = result + }, + (param: string, value: string) => value, + ) - if (DEBUG) { - console.log(`Number of lines in response: ${responseLines.length}`) - } - - expect(userMessageContent.length).toBe(2) - expect(userMessageContent[0].text).toBe(`[read_file for '${testFilePath}'] Result:`) - - if (testCase.expectations.expectedContent) { - expect(userMessageContent[1].text).toBe(testCase.expectations.expectedContent) - } - - if (testCase.expectations.responseValidation) { - validateResponseLines(responseLines, testCase.expectations.responseValidation) - } - - if (testCase.expectations.truncationMessage) { - expect(userMessageContent[1].text).toContain(testCase.expectations.truncationMessage) - } - - if (testCase.expectations.includeSourceCodeDef) { - expect(userMessageContent[1].text).toContain(sourceCodeDef) - } + // Verify + expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 3, 1) // end_line - 1, start_line - 1 + expect(mockedAddLineNumbers).toHaveBeenCalledWith(expect.any(String), 2) // start with proper line numbers + }) }) }) diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts new file mode 100644 index 0000000000..6616a5fcd1 --- /dev/null +++ b/src/core/tools/readFileTool.ts @@ -0,0 +1,168 @@ +import path from "path" +import { Cline } from "../Cline" +import { ClineSayTool } from "../../shared/ExtensionMessage" +import { ToolUse } from "../assistant-message" +import { formatResponse } from "../prompts/responses" +import { AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "./types" +import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getReadablePath } from "../../utils/path" +import { countFileLines } from "../../integrations/misc/line-counter" +import { readLines } from "../../integrations/misc/read-lines" +import { extractTextFromFile, addLineNumbers } from "../../integrations/misc/extract-text" +import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" +import { isBinaryFile } from "isbinaryfile" + +export async function readFileTool( + cline: Cline, + block: ToolUse, + askApproval: AskApproval, + handleError: HandleError, + pushToolResult: PushToolResult, + removeClosingTag: RemoveClosingTag, +) { + switch (true) { + default: + const relPath: string | undefined = block.params.path + const startLineStr: string | undefined = block.params.start_line + const endLineStr: string | undefined = block.params.end_line + + // Get the full path and determine if it's outside the workspace + const fullPath = relPath ? path.resolve(cline.cwd, removeClosingTag("path", relPath)) : "" + const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) + + const sharedMessageProps: ClineSayTool = { + tool: "readFile", + path: getReadablePath(cline.cwd, removeClosingTag("path", relPath)), + isOutsideWorkspace, + } + try { + if (block.partial) { + const partialMessage = JSON.stringify({ + ...sharedMessageProps, + content: undefined, + } satisfies ClineSayTool) + await cline.ask("tool", partialMessage, block.partial).catch(() => {}) + break + } else { + if (!relPath) { + cline.consecutiveMistakeCount++ + pushToolResult(await cline.sayAndCreateMissingParamError("read_file", "path")) + break + } + + // Check if we're doing a line range read + let isRangeRead = false + let startLine: number | undefined = undefined + let endLine: number | undefined = undefined + + // Check if we have either range parameter + if (startLineStr || endLineStr) { + isRangeRead = true + } + + // Parse start_line if provided + if (startLineStr) { + startLine = parseInt(startLineStr) + if (isNaN(startLine)) { + // Invalid start_line + cline.consecutiveMistakeCount++ + await cline.say("error", `Failed to parse start_line: ${startLineStr}`) + pushToolResult(formatResponse.toolError("Invalid start_line value")) + break + } + startLine -= 1 // Convert to 0-based index + } + + // Parse end_line if provided + if (endLineStr) { + endLine = parseInt(endLineStr) + + if (isNaN(endLine)) { + // Invalid end_line + cline.consecutiveMistakeCount++ + await cline.say("error", `Failed to parse end_line: ${endLineStr}`) + pushToolResult(formatResponse.toolError("Invalid end_line value")) + break + } + + // Convert to 0-based index + endLine -= 1 + } + + const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) + if (!accessAllowed) { + await cline.say("rooignore_error", relPath) + pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath))) + + break + } + + cline.consecutiveMistakeCount = 0 + const absolutePath = path.resolve(cline.cwd, relPath) + const completeMessage = JSON.stringify({ + ...sharedMessageProps, + content: absolutePath, + } satisfies ClineSayTool) + + const didApprove = await askApproval("tool", completeMessage) + if (!didApprove) { + break + } + + // Get the maxReadFileLine setting + const { maxReadFileLine = 500 } = (await cline.providerRef.deref()?.getState()) ?? {} + + // Count total lines in the file + let totalLines = 0 + try { + totalLines = await countFileLines(absolutePath) + } catch (error) { + console.error(`Error counting lines in file ${absolutePath}:`, error) + } + + // now execute the tool like normal + let content: string + let isFileTruncated = false + let sourceCodeDef = "" + + const isBinary = await isBinaryFile(absolutePath).catch(() => false) + + if (isRangeRead) { + if (startLine === undefined) { + content = addLineNumbers(await readLines(absolutePath, endLine, startLine)) + } else { + content = addLineNumbers(await readLines(absolutePath, endLine, startLine), startLine + 1) + } + } else if (!isBinary && maxReadFileLine >= 0 && totalLines > maxReadFileLine) { + // If file is too large, only read the first maxReadFileLine lines + isFileTruncated = true + + const res = await Promise.all([ + maxReadFileLine > 0 ? readLines(absolutePath, maxReadFileLine - 1, 0) : "", + parseSourceCodeDefinitionsForFile(absolutePath, cline.rooIgnoreController), + ]) + + content = res[0].length > 0 ? addLineNumbers(res[0]) : "" + const result = res[1] + if (result) { + sourceCodeDef = `\n\n${result}` + } + } else { + // Read entire file + content = await extractTextFromFile(absolutePath) + } + + // Add truncation notice if applicable + if (isFileTruncated) { + content += `\n\n[Showing only ${maxReadFileLine} of ${totalLines} total lines. Use start_line and end_line if you need to read more]${sourceCodeDef}` + } + + pushToolResult(content) + break + } + } catch (error) { + await handleError("reading file", error) + break + } + } +} diff --git a/src/core/tools/types.ts b/src/core/tools/types.ts index d03ddd016e..87d7d1ed36 100644 --- a/src/core/tools/types.ts +++ b/src/core/tools/types.ts @@ -1,4 +1,5 @@ import { ClineAsk, ToolProgressStatus } from "../../schemas" +import { ToolParamName } from "../assistant-message" import { ToolResponse } from "../Cline" export type AskApproval = ( @@ -10,3 +11,5 @@ export type AskApproval = ( export type HandleError = (action: string, error: Error) => void export type PushToolResult = (content: ToolResponse) => void + +export type RemoveClosingTag = (tag: ToolParamName, content?: string) => string