diff --git a/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap b/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap index 40ee385f58..d4519ad5d7 100644 --- a/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap +++ b/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap @@ -315,22 +315,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -762,22 +772,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -1209,22 +1229,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -1709,22 +1739,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -2208,22 +2248,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -2727,22 +2777,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -3265,22 +3325,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -3712,22 +3782,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -4250,22 +4330,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -4703,22 +4793,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -5038,22 +5138,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + @@ -5567,22 +5677,32 @@ Example: Requesting to switch to code mode ## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. +Description: Create a new task with a specified starting mode and initial message. This tool instructs the system to create a new Cline instance in the given mode with the provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + diff --git a/src/core/prompts/tools/new-task.ts b/src/core/prompts/tools/new-task.ts index 7301b7b422..40462e8b64 100644 --- a/src/core/prompts/tools/new-task.ts +++ b/src/core/prompts/tools/new-task.ts @@ -2,22 +2,31 @@ import { ToolArgs } from "./types" export function getNewTaskDescription(_args: ToolArgs): string { return `## new_task -Description: This will let you create a new task instance in the chosen mode using your provided message. +Description: This will let you create a new task instance in the chosen mode using your provided message and attached files. Parameters: - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). - message: (required) The initial user message or instructions for this new task. +- files: (optional) A list of relevant files to include in the new task. Use a parent tag containing one or more tags, each with a relative workspace path, optionally followed by \`:startLine:endLine\` to specify a range (e.g., \`path/to/file.ts:10:50\`) if needed). Usage: your-mode-slug-here Your initial instructions here + +path/without/range.js +path/with/range.py:25:100 + Example: code Implement a new feature for the application. + +src/somefile.ts +src/anotherfile.ts:10:50 + ` } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index c9f2b4a100..034ec3f490 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -34,7 +34,7 @@ import { ClineApiReqCancelReason, ClineApiReqInfo } from "../../shared/Extension import { getApiMetrics } from "../../shared/getApiMetrics" import { ClineAskResponse } from "../../shared/WebviewMessage" import { defaultModeSlug } from "../../shared/modes" -import { DiffStrategy } from "../../shared/tools" +import { DiffStrategy, AttachedFileSpec } from "../../shared/tools" // services import { UrlContentFetcher } from "../../services/browser/UrlContentFetcher" @@ -80,6 +80,7 @@ import { processUserContentMentions } from "../mentions/processUserContentMentio import { ApiMessage } from "../task-persistence/apiMessages" import { getMessagesSinceLastSummary, summarizeConversation } from "../condense" import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning" +import { processFileForReading, formatProcessedFileResultToString } from "../../shared/fileReadUtils" export type ClineEvents = { message: [{ action: "created" | "updated"; message: ClineMessage }] @@ -104,6 +105,7 @@ export type TaskOptions = { consecutiveMistakeLimit?: number task?: string images?: string[] + attachedFiles?: AttachedFileSpec[] historyItem?: HistoryItem experiments?: Record startTask?: boolean @@ -121,6 +123,7 @@ export class Task extends EventEmitter { readonly parentTask: Task | undefined = undefined readonly taskNumber: number readonly workspacePath: string + public readonly attachedFiles: AttachedFileSpec[] = [] providerRef: WeakRef private readonly globalStoragePath: string @@ -197,6 +200,7 @@ export class Task extends EventEmitter { consecutiveMistakeLimit = 3, task, images, + attachedFiles, historyItem, startTask = true, rootTask, @@ -241,6 +245,7 @@ export class Task extends EventEmitter { this.rootTask = rootTask this.parentTask = parentTask this.taskNumber = taskNumber + this.attachedFiles = attachedFiles || [] if (historyItem) { telemetryService.captureTaskRestarted(this.taskId) @@ -656,6 +661,42 @@ export class Task extends EventEmitter { // Start / Abort / Resume + /** + * Formats the content of attached files for inclusion in the task prompt + * @param workspaceRoot The root directory of the workspace + * @param maxReadFileLine Maximum number of lines to read from each file + * @returns Formatted string containing all attached files content + */ + private async _formatAttachedFilesContent(workspaceRoot: string, maxReadFileLine: number): Promise { + const attachedFilesStrings: string[] = [] + + for (const fileSpec of this.attachedFiles) { + // Handle both string and AttachedFileSpec types + const relativePath = typeof fileSpec === "string" ? fileSpec : fileSpec.path + if (!relativePath) continue // Skip empty paths + + const requestedStartLine = typeof fileSpec === "string" ? undefined : fileSpec.startLine + const requestedEndLine = typeof fileSpec === "string" ? undefined : fileSpec.endLine + const absolutePath = path.join(workspaceRoot, relativePath) + + // Process the file using shared utilities + const result = await processFileForReading( + absolutePath, + relativePath, + maxReadFileLine, + requestedStartLine, + requestedEndLine, + this.rooIgnoreController, + ) + + // Format the result to string + const fileString = formatProcessedFileResultToString(result) + attachedFilesStrings.push(fileString) + } + + return attachedFilesStrings.join("\n") + } + private async startTask(task?: string, images?: string[]): Promise { // `conversationHistory` (for API) and `clineMessages` (for webview) // need to be in sync. @@ -667,6 +708,10 @@ export class Task extends EventEmitter { this.apiConversationHistory = [] await this.providerRef.deref()?.postStateToWebview() + const providerState = await this.providerRef.deref()?.getState() + const workspaceRoot = this.cwd + const maxReadFileLine = providerState?.maxReadFileLine ?? 500 // Default to 500 + await this.say("text", task, images) this.isInitialized = true @@ -674,13 +719,20 @@ export class Task extends EventEmitter { console.log(`[subtasks] task ${this.taskId}.${this.instanceId} starting`) - await this.initiateTaskLoop([ - { - type: "text", - text: `\n${task}\n`, - }, - ...imageBlocks, - ]) + const taskBlock = `\n${task ?? ""}\n` + + const attachedFilesContent = await this._formatAttachedFilesContent(workspaceRoot, maxReadFileLine) + + const attachedFilesBlock = + this.attachedFiles.length > 0 ? `\n${attachedFilesContent}` : "" + + const blocks: Anthropic.ContentBlockParam[] = [{ type: "text", text: taskBlock }, ...imageBlocks] + + if (attachedFilesBlock) { + blocks.push({ type: "text", text: attachedFilesBlock }) + } + + await this.initiateTaskLoop(blocks) } public async resumePausedTask(lastMessage: string) { diff --git a/src/core/tools/__tests__/readFileTool.test.ts b/src/core/tools/__tests__/readFileTool.test.ts index f0b3600a26..0717574a33 100644 --- a/src/core/tools/__tests__/readFileTool.test.ts +++ b/src/core/tools/__tests__/readFileTool.test.ts @@ -1,15 +1,8 @@ -// npx jest src/core/tools/__tests__/readFileTool.test.ts - -import * as path from "path" - -import { countFileLines } from "../../../integrations/misc/line-counter" -import { readLines } from "../../../integrations/misc/read-lines" -import { extractTextFromFile } from "../../../integrations/misc/extract-text" -import { parseSourceCodeDefinitionsForFile } from "../../../services/tree-sitter" -import { isBinaryFile } from "isbinaryfile" -import { ReadFileToolUse, ToolParamName, ToolResponse } from "../../../shared/tools" +import { processFileForReading, formatProcessedFileResultToString } from "../../../shared/fileReadUtils" +import { ToolUse } from "../../../shared/tools" import { readFileTool } from "../readFileTool" +jest.mock("../../../shared/fileReadUtils") jest.mock("path", () => { const originalPath = jest.requireActual("path") return { @@ -18,935 +11,273 @@ jest.mock("path", () => { } }) -jest.mock("fs/promises", () => ({ - mkdir: jest.fn().mockResolvedValue(undefined), - writeFile: jest.fn().mockResolvedValue(undefined), - readFile: jest.fn().mockResolvedValue("{}"), -})) - -jest.mock("isbinaryfile") - -jest.mock("../../../integrations/misc/line-counter") -jest.mock("../../../integrations/misc/read-lines") - -let mockInputContent = "" - -jest.mock("../../../integrations/misc/extract-text", () => { - const actual = jest.requireActual("../../../integrations/misc/extract-text") - // Create a spy on the actual addLineNumbers function. - const addLineNumbersSpy = jest.spyOn(actual, "addLineNumbers") - - return { - ...actual, - // Expose the spy so tests can access it. - __addLineNumbersSpy: addLineNumbersSpy, - extractTextFromFile: jest.fn().mockImplementation((_filePath) => { - // Use the actual addLineNumbers function. - const content = mockInputContent - return Promise.resolve(actual.addLineNumbers(content)) - }), - } -}) - -const addLineNumbersSpy = jest.requireMock("../../../integrations/misc/extract-text").__addLineNumbersSpy - -jest.mock("../../../services/tree-sitter") - -jest.mock("../../ignore/RooIgnoreController", () => ({ - RooIgnoreController: class { - initialize() { - return Promise.resolve() - } - validateAccess() { - return true - } - }, -})) - -jest.mock("../../../utils/fs", () => ({ - fileExistsAtPath: jest.fn().mockReturnValue(true), -})) - -describe("read_file tool with maxReadFileLine setting", () => { - // Test data - const testFilePath = "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\n" - const sourceCodeDef = "\n\n# file.txt\n1--5 | Content" - const expectedFullFileXml = `${testFilePath}\n\n${numberedFileContent}\n` - - // Mocked functions with correct types - const mockedCountFileLines = countFileLines as jest.MockedFunction - const mockedReadLines = readLines as jest.MockedFunction - const mockedExtractTextFromFile = extractTextFromFile as jest.MockedFunction - const mockedParseSourceCodeDefinitionsForFile = parseSourceCodeDefinitionsForFile as jest.MockedFunction< - typeof parseSourceCodeDefinitionsForFile - > - - const mockedIsBinaryFile = isBinaryFile as jest.MockedFunction - const mockedPathResolve = path.resolve as jest.MockedFunction - - const mockCline: any = {} +describe("readFileTool tests", () => { + let mockPushToolResult: jest.Mock + let mockAskApproval: jest.Mock + let mockRemoveClosingTag: jest.Mock + let mockHandleError: jest.Mock + let mockCline: any let mockProvider: any - let toolResult: ToolResponse | undefined beforeEach(() => { jest.clearAllMocks() - mockedPathResolve.mockReturnValue(absoluteFilePath) - mockedIsBinaryFile.mockResolvedValue(false) - - mockInputContent = fileContent - - // Setup the extractTextFromFile mock implementation with the current - // mockInputContent. - mockedExtractTextFromFile.mockImplementation((_filePath) => { - const actual = jest.requireActual("../../../integrations/misc/extract-text") - return Promise.resolve(actual.addLineNumbers(mockInputContent)) - }) - - // No need to setup the extractTextFromFile mock implementation here - // as it's already defined at the module level. - + // Mock for providerRef.deref().getState() mockProvider = { - getState: jest.fn(), + getState: jest.fn().mockResolvedValue({ maxReadFileLine: 500 }), // Default maxReadFileLine deref: jest.fn().mockReturnThis(), } - 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() - - mockCline.fileContextTracker = { - trackFileContext: jest.fn().mockResolvedValue(undefined), - } - - mockCline.recordToolUsage = jest.fn().mockReturnValue(undefined) - mockCline.recordToolError = jest.fn().mockReturnValue(undefined) - - toolResult = undefined - }) - - /** - * Helper function to execute the read file tool with different maxReadFileLine settings - */ - async function executeReadFileTool( - params: Partial = {}, - options: { - maxReadFileLine?: number - totalLines?: number - skipAddLineNumbersCheck?: boolean // Flag to skip addLineNumbers check - } = {}, - ): Promise { - // Configure mocks based on test scenario - const maxReadFileLine = options.maxReadFileLine ?? 500 - const totalLines = options.totalLines ?? 5 - - mockProvider.getState.mockResolvedValue({ maxReadFileLine }) - mockedCountFileLines.mockResolvedValue(totalLines) - - // Reset the spy before each test - addLineNumbersSpy.mockClear() - - // Create a tool use object - const toolUse: ReadFileToolUse = { - type: "tool_use", - name: "read_file", - params: { path: testFilePath, ...params }, - partial: false, - } - - await readFileTool( - mockCline, - toolUse, - mockCline.ask, - jest.fn(), - (result: ToolResponse) => { - toolResult = result + mockPushToolResult = jest.fn() + mockAskApproval = jest.fn().mockResolvedValue(true) + mockRemoveClosingTag = jest.fn((_, content) => content) + mockHandleError = jest.fn() + + mockCline = { + cwd: "/test/workspace", + task: "TestTask", + providerRef: mockProvider, + rooIgnoreController: { + validateAccess: jest.fn().mockReturnValue(true), + }, + fileContextTracker: { + trackFileContext: jest.fn().mockResolvedValue(undefined), }, - (_: ToolParamName, content?: string) => content ?? "", - ) - - // Verify addLineNumbers was called appropriately - if (!options.skipAddLineNumbersCheck) { - expect(addLineNumbersSpy).toHaveBeenCalled() - } else { - expect(addLineNumbersSpy).not.toHaveBeenCalled() + say: jest.fn().mockResolvedValue(undefined), + ask: mockAskApproval, + recordToolError: jest.fn(), + sayAndCreateMissingParamError: jest.fn().mockResolvedValue("Missing required parameter"), + consecutiveMistakeCount: 0, } - return toolResult - } - - describe("when maxReadFileLine is negative", () => { - it("should read the entire file using extractTextFromFile", async () => { - // Setup - use default mockInputContent - mockInputContent = fileContent - - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine: -1 }) + // Reset individual spies + mockPushToolResult.mockClear() + mockAskApproval.mockClear().mockResolvedValue(true) + mockRemoveClosingTag.mockClear().mockImplementation((_, content) => content) + mockHandleError.mockClear() + ;(processFileForReading as jest.Mock).mockClear() + ;(formatProcessedFileResultToString as jest.Mock).mockClear() + }) - // Verify - expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - expect(mockedReadLines).not.toHaveBeenCalled() - expect(mockedParseSourceCodeDefinitionsForFile).not.toHaveBeenCalled() - expect(result).toBe(expectedFullFileXml) - }) + it("should have readFileTool defined", () => { + expect(readFileTool).toBeDefined() + }) - it("should ignore range parameters and read entire file when maxReadFileLine is -1", async () => { - // Setup - use default mockInputContent - mockInputContent = fileContent + describe("Parameter Validation and Error Handling", () => { + it("should handle missing path parameter", async () => { + const block: ToolUse = { + type: "tool_use" as const, + name: "read_file", + params: {}, + partial: false, + } - // Execute with range parameters - const result = await executeReadFileTool( - { - start_line: "2", - end_line: "4", - }, - { maxReadFileLine: -1 }, + await readFileTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, ) - // Verify that extractTextFromFile is still used (not readLines) - expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - expect(mockedReadLines).not.toHaveBeenCalled() - expect(mockedParseSourceCodeDefinitionsForFile).not.toHaveBeenCalled() - expect(result).toBe(expectedFullFileXml) - }) - - it("should not show line snippet in approval message when maxReadFileLine is -1", async () => { - // This test verifies the line snippet behavior for the approval message - // Setup - use default mockInputContent - mockInputContent = fileContent - - // Execute - we'll reuse executeReadFileTool to run the tool - await executeReadFileTool({}, { maxReadFileLine: -1 }) - - // Verify the empty line snippet for full read was passed to the approval message - // Look at the parameters passed to the 'ask' method in the approval message - const askCall = mockCline.ask.mock.calls[0] - const completeMessage = JSON.parse(askCall[1]) - - // Verify the reason (lineSnippet) is empty or undefined for full read - expect(completeMessage.reason).toBeFalsy() + expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("read_file", "path") + expect(mockPushToolResult).toHaveBeenCalledWith( + "Missing required parameter", + ) + expect(processFileForReading).not.toHaveBeenCalled() + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("read_file") }) - }) - 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) - - // Execute - skip addLineNumbers check as it's not called for maxReadFileLine=0 - const result = await executeReadFileTool( - {}, - { - maxReadFileLine: 0, - totalLines: 5, - skipAddLineNumbersCheck: true, + it("should handle invalid start_line parameter", async () => { + const block: ToolUse = { + type: "tool_use" as const, + name: "read_file", + params: { + path: "test/file.txt", + start_line: "not-a-number", }, - ) + partial: false, + } - // Verify - expect(mockedExtractTextFromFile).not.toHaveBeenCalled() - expect(mockedReadLines).not.toHaveBeenCalled() // Per implementation line 141 - expect(mockedParseSourceCodeDefinitionsForFile).toHaveBeenCalledWith( - absoluteFilePath, - mockCline.rooIgnoreController, + await readFileTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, ) - // Verify XML structure - expect(result).toContain(`${testFilePath}`) - expect(result).toContain("Showing only 0 of 5 total lines") - expect(result).toContain("") - expect(result).toContain("") - expect(result).toContain(sourceCodeDef.trim()) - expect(result).toContain("") - expect(result).not.toContain(" { - 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) - - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine: 3 }) - - // Verify - check behavior but not specific implementation details - expect(mockedExtractTextFromFile).not.toHaveBeenCalled() - expect(mockedReadLines).toHaveBeenCalled() - expect(mockedParseSourceCodeDefinitionsForFile).toHaveBeenCalledWith( - absoluteFilePath, - mockCline.rooIgnoreController, + expect(mockCline.say).toHaveBeenCalledWith("error", "Failed to parse start_line: not-a-number") + expect(mockPushToolResult).toHaveBeenCalledWith( + "test/file.txtInvalid start_line value", ) - - // Verify XML structure - expect(result).toContain(`${testFilePath}`) - expect(result).toContain('') - expect(result).toContain("1 | Line 1") - expect(result).toContain("2 | Line 2") - expect(result).toContain("3 | Line 3") - expect(result).toContain("") - expect(result).toContain("Showing only 3 of 5 total lines") - expect(result).toContain("") - expect(result).toContain("") - expect(result).toContain(sourceCodeDef.trim()) - expect(result).toContain("") - expect(result).toContain("") - expect(result).toContain(sourceCodeDef.trim()) - }) - }) - - describe("when maxReadFileLine equals or exceeds file length", () => { - it("should use extractTextFromFile when maxReadFileLine > totalLines", async () => { - // Setup - mockedCountFileLines.mockResolvedValue(5) // File shorter than maxReadFileLine - mockInputContent = fileContent - - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine: 10, totalLines: 5 }) - - // Verify - expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - expect(result).toBe(expectedFullFileXml) + expect(processFileForReading).not.toHaveBeenCalled() + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("read_file") }) - it("should read with extractTextFromFile when file has few lines", async () => { - // Setup - mockedCountFileLines.mockResolvedValue(3) // File shorter than maxReadFileLine - mockInputContent = fileContent - - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine: 5, totalLines: 3 }) - - // Verify - expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - expect(mockedReadLines).not.toHaveBeenCalled() - // Create a custom expected XML with lines="1-3" since totalLines is 3 - const expectedXml = `${testFilePath}\n\n${numberedFileContent}\n` - expect(result).toBe(expectedXml) - }) - }) - - describe("when file is binary", () => { - it("should always use extractTextFromFile regardless of maxReadFileLine", async () => { - // Setup - mockedIsBinaryFile.mockResolvedValue(true) - // For binary files, we're using a maxReadFileLine of 3 and totalLines is assumed to be 3 - mockedCountFileLines.mockResolvedValue(3) - - // For binary files, we need a special mock implementation that doesn't use addLineNumbers - // Save the original mock implementation - const originalMockImplementation = mockedExtractTextFromFile.getMockImplementation() - // Create a special mock implementation that doesn't call addLineNumbers - mockedExtractTextFromFile.mockImplementation(() => { - return Promise.resolve(numberedFileContent) - }) - - // Reset the spy to clear any previous calls - addLineNumbersSpy.mockClear() - - // Execute - skip addLineNumbers check as we're directly providing the numbered content - const result = await executeReadFileTool( - {}, - { - maxReadFileLine: 3, - totalLines: 3, - skipAddLineNumbersCheck: true, + it("should handle invalid end_line parameter", async () => { + const block: ToolUse = { + type: "tool_use" as const, + name: "read_file", + params: { + path: "test/file.txt", + end_line: "not-a-number", }, - ) - - // Restore the original mock implementation after the test - mockedExtractTextFromFile.mockImplementation(originalMockImplementation) - - // Verify - expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - expect(mockedReadLines).not.toHaveBeenCalled() - // Create a custom expected XML with lines="1-3" for binary files - const expectedXml = `${testFilePath}\n\n${numberedFileContent}\n` - expect(result).toBe(expectedXml) - }) - }) - - describe("with range parameters", () => { - it("should honor start_line and end_line when provided", async () => { - // Setup - mockedReadLines.mockResolvedValue("Line 2\nLine 3\nLine 4") - - // Execute using executeReadFileTool with range parameters - const rangeResult = await executeReadFileTool({ - start_line: "2", - end_line: "4", - }) - - // Verify - expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 3, 1) // end_line - 1, start_line - 1 - expect(addLineNumbersSpy).toHaveBeenCalledWith(expect.any(String), 2) // start with proper line numbers - - // Verify XML structure with lines attribute - expect(rangeResult).toContain(`${testFilePath}`) - expect(rangeResult).toContain(``) - expect(rangeResult).toContain("2 | Line 2") - expect(rangeResult).toContain("3 | Line 3") - expect(rangeResult).toContain("4 | Line 4") - expect(rangeResult).toContain("") - }) - }) -}) - -describe("read_file tool XML output structure", () => { - // Test data - const testFilePath = "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\n" - 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 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: ToolResponse | undefined - - beforeEach(() => { - jest.clearAllMocks() - - mockedPathResolve.mockReturnValue(absoluteFilePath) - mockedIsBinaryFile.mockResolvedValue(false) - - mockInputContent = fileContent - - mockProvider = { - getState: jest.fn().mockResolvedValue({ maxReadFileLine: 500 }), - deref: jest.fn().mockReturnThis(), - } - - 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() - mockCline.sayAndCreateMissingParamError = jest.fn().mockResolvedValue("Missing required parameter") - - mockCline.fileContextTracker = { - trackFileContext: jest.fn().mockResolvedValue(undefined), - } - - mockCline.recordToolUsage = jest.fn().mockReturnValue(undefined) - mockCline.recordToolError = jest.fn().mockReturnValue(undefined) - - toolResult = undefined - }) - - /** - * Helper function to execute the read file tool with custom parameters - */ - async function executeReadFileTool( - params: Partial = {}, - options: { - totalLines?: number - maxReadFileLine?: number - isBinary?: boolean - validateAccess?: boolean - skipAddLineNumbersCheck?: boolean // Flag to skip addLineNumbers check - } = {}, - ): Promise { - // Configure mocks based on test scenario - const totalLines = options.totalLines ?? 5 - const maxReadFileLine = options.maxReadFileLine ?? 500 - const isBinary = options.isBinary ?? false - const validateAccess = options.validateAccess ?? true - - mockProvider.getState.mockResolvedValue({ maxReadFileLine }) - mockedCountFileLines.mockResolvedValue(totalLines) - mockedIsBinaryFile.mockResolvedValue(isBinary) - mockCline.rooIgnoreController.validateAccess = jest.fn().mockReturnValue(validateAccess) - - // Create a tool use object - const toolUse: ReadFileToolUse = { - type: "tool_use", - name: "read_file", - params: { - path: testFilePath, - ...params, - }, - partial: false, - } - - // Reset the spy's call history before each test - addLineNumbersSpy.mockClear() - - // Execute the tool - await readFileTool( - mockCline, - toolUse, - mockCline.ask, - jest.fn(), - (result: ToolResponse) => { - toolResult = result - }, - (param: ToolParamName, content?: string) => content ?? "", - ) - // Verify addLineNumbers was called (unless explicitly skipped) - if (!options.skipAddLineNumbersCheck) { - expect(addLineNumbersSpy).toHaveBeenCalled() - } else { - // For cases where we expect addLineNumbers NOT to be called - expect(addLineNumbersSpy).not.toHaveBeenCalled() - } - - return toolResult - } - - describe("Basic XML Structure Tests", () => { - it("should produce XML output with no unnecessary indentation", async () => { - // Setup - use default mockInputContent (fileContent) - mockInputContent = fileContent - - // Execute - const result = await executeReadFileTool() + partial: false, + } - // Verify - expect(result).toBe( - `${testFilePath}\n\n${numberedFileContent}\n`, + await readFileTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, ) - }) - - it("should follow the correct XML structure format", async () => { - // Setup - use default mockInputContent (fileContent) - mockInputContent = fileContent - - // Execute - const result = await executeReadFileTool() - // Verify using regex to check structure - const xmlStructureRegex = new RegExp( - `^${testFilePath}\\n\\n.*\\n$`, - "s", + expect(mockCline.say).toHaveBeenCalledWith("error", "Failed to parse end_line: not-a-number") + expect(mockPushToolResult).toHaveBeenCalledWith( + "test/file.txtInvalid end_line value", ) - expect(result).toMatch(xmlStructureRegex) + expect(processFileForReading).not.toHaveBeenCalled() + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("read_file") }) }) - describe("Line Range Tests", () => { - it("should include lines attribute when start_line is specified", async () => { - // Setup - const startLine = 2 - mockedReadLines.mockResolvedValue( - fileContent - .split("\n") - .slice(startLine - 1) - .join("\n"), + describe("Main Execution, Approval Flow, and Line Snippets", () => { + const setupReadFileTest = async (params: Record, maxReadFileLine: number = 500) => { + mockProvider.getState.mockResolvedValue({ maxReadFileLine }) + ;(processFileForReading as jest.Mock).mockResolvedValue({ + type: "success", + path: "test/file.txt", + content: "file content", + lines: "1-1", + totalLines: 1, + isBinary: false, + }) + ;(formatProcessedFileResultToString as jest.Mock).mockReturnValue( + "test/file.txtfile content", ) - // Execute - const result = await executeReadFileTool({ start_line: startLine.toString() }) - - // Verify - expect(result).toContain(``) - }) - - it("should include lines attribute when end_line is specified", async () => { - // Setup - const endLine = 3 - mockedReadLines.mockResolvedValue(fileContent.split("\n").slice(0, endLine).join("\n")) - - // Execute - const result = await executeReadFileTool({ end_line: endLine.toString() }) - - // Verify - expect(result).toContain(``) - }) + const block: ToolUse = { + type: "tool_use" as const, + name: "read_file", + params, + partial: false, + } - it("should include lines attribute when both start_line and end_line are specified", async () => { - // Setup - const startLine = 2 - const endLine = 4 - mockedReadLines.mockResolvedValue( - fileContent - .split("\n") - .slice(startLine - 1, endLine) - .join("\n"), + await readFileTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, ) + } - // Execute - const result = await executeReadFileTool({ - start_line: startLine.toString(), - end_line: endLine.toString(), - }) - - // Verify - expect(result).toContain(``) - }) - - it("should include lines attribute even when no range is specified", async () => { - // Setup - use default mockInputContent (fileContent) - mockInputContent = fileContent + it("should handle full file read", async () => { + await setupReadFileTest({ path: "test/file.txt" }, -1) - // Execute - const result = await executeReadFileTool() + expect(mockAskApproval).toHaveBeenCalledWith("tool", expect.any(String)) + const approvalMessage = JSON.parse(mockAskApproval.mock.calls[0][1]) + expect(approvalMessage.reason).toBeFalsy() - // Verify - expect(result).toContain(`\n`) + expect(processFileForReading).toHaveBeenCalled() + expect(mockCline.fileContextTracker.trackFileContext).toHaveBeenCalledWith("test/file.txt", "read_tool") + expect(mockPushToolResult).toHaveBeenCalled() }) - it("should include content when maxReadFileLine=0 and range is specified", async () => { - // Setup - const maxReadFileLine = 0 - const startLine = 2 - const endLine = 4 - const totalLines = 10 - - mockedReadLines.mockResolvedValue( - fileContent - .split("\n") - .slice(startLine - 1, endLine) - .join("\n"), - ) - - // Execute - const result = await executeReadFileTool( - { - start_line: startLine.toString(), - end_line: endLine.toString(), - }, - { maxReadFileLine, totalLines }, - ) - - // Verify - // Should include content tag with line range - expect(result).toContain(``) - - // Should NOT include definitions (range reads never show definitions) - expect(result).not.toContain("") + it("should handle range specified read", async () => { + await setupReadFileTest({ + path: "test/file.txt", + start_line: "2", + end_line: "4", + }) - // Should NOT include truncation notice - expect(result).not.toContain(`Showing only ${maxReadFileLine} of ${totalLines} total lines`) + const approvalMessage = JSON.parse(mockAskApproval.mock.calls[0][1]) + expect(approvalMessage.reason).toBe("readFile.linesRange") }) - it("should include content when maxReadFileLine=0 and only start_line is specified", async () => { - // Setup - const maxReadFileLine = 0 - const startLine = 3 - const totalLines = 10 - - mockedReadLines.mockResolvedValue( - fileContent - .split("\n") - .slice(startLine - 1) - .join("\n"), - ) - - // Execute - const result = await executeReadFileTool( - { - start_line: startLine.toString(), - }, - { maxReadFileLine, totalLines }, - ) - - // Verify - // Should include content tag with line range - expect(result).toContain(``) - - // Should NOT include definitions (range reads never show definitions) - expect(result).not.toContain("") + it("should handle start_line only read", async () => { + await setupReadFileTest({ + path: "test/file.txt", + start_line: "2", + }) - // Should NOT include truncation notice - expect(result).not.toContain(`Showing only ${maxReadFileLine} of ${totalLines} total lines`) + const approvalMessage = JSON.parse(mockAskApproval.mock.calls[0][1]) + expect(approvalMessage.reason).toBe("readFile.linesFromToEnd") }) - it("should include content when maxReadFileLine=0 and only end_line is specified", async () => { - // Setup - const maxReadFileLine = 0 - const endLine = 3 - const totalLines = 10 - - mockedReadLines.mockResolvedValue(fileContent.split("\n").slice(0, endLine).join("\n")) - - // Execute - const result = await executeReadFileTool( - { - end_line: endLine.toString(), - }, - { maxReadFileLine, totalLines }, - ) - - // Verify - // Should include content tag with line range - expect(result).toContain(``) - - // Should NOT include definitions (range reads never show definitions) - expect(result).not.toContain("") + it("should handle end_line only read", async () => { + await setupReadFileTest({ + path: "test/file.txt", + end_line: "4", + }) - // Should NOT include truncation notice - expect(result).not.toContain(`Showing only ${maxReadFileLine} of ${totalLines} total lines`) + const approvalMessage = JSON.parse(mockAskApproval.mock.calls[0][1]) + expect(approvalMessage.reason).toBe("readFile.linesFromStartTo") }) - it("should include full range content when maxReadFileLine=5 and content has more than 5 lines", async () => { - // Setup - const maxReadFileLine = 5 - const startLine = 2 - const endLine = 8 - const totalLines = 10 - - // Create mock content with 7 lines (more than maxReadFileLine) - const rangeContent = Array(endLine - startLine + 1) - .fill("Range line content") - .join("\n") - - mockedReadLines.mockResolvedValue(rangeContent) - - // Execute - const result = await executeReadFileTool( - { - start_line: startLine.toString(), - end_line: endLine.toString(), - }, - { maxReadFileLine, totalLines }, - ) - - // Verify - // Should include content tag with the full requested range (not limited by maxReadFileLine) - expect(result).toContain(``) - - // Should NOT include definitions (range reads never show definitions) - expect(result).not.toContain("") - - // Should NOT include truncation notice - expect(result).not.toContain(`Showing only ${maxReadFileLine} of ${totalLines} total lines`) - - // Should contain all the requested lines, not just maxReadFileLine lines - expect(result).toBeDefined() - expect(typeof result).toBe("string") + it("should handle definitions only read", async () => { + await setupReadFileTest({ path: "test/file.txt" }, 0) - if (typeof result === "string") { - expect(result.split("\n").length).toBeGreaterThan(maxReadFileLine) - } + const approvalMessage = JSON.parse(mockAskApproval.mock.calls[0][1]) + expect(approvalMessage.reason).toBe("readFile.definitionsOnly") }) - }) - - describe("Notice and Definition Tags Tests", () => { - it("should include notice tag for truncated files", async () => { - // Setup - const maxReadFileLine = 3 - const totalLines = 10 - mockedReadLines.mockResolvedValue(fileContent.split("\n").slice(0, maxReadFileLine).join("\n")) - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine, totalLines }) + it("should handle max lines read", async () => { + await setupReadFileTest({ path: "test/file.txt" }, 100) - // Verify - expect(result).toContain(`Showing only ${maxReadFileLine} of ${totalLines} total lines`) + const approvalMessage = JSON.parse(mockAskApproval.mock.calls[0][1]) + expect(approvalMessage.reason).toBe("readFile.maxLines") }) - it("should include list_code_definition_names tag when source code definitions are available", async () => { - // Setup - const maxReadFileLine = 3 - const totalLines = 10 - mockedReadLines.mockResolvedValue(fileContent.split("\n").slice(0, maxReadFileLine).join("\n")) - mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(sourceCodeDef) - - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine, totalLines }) - - // Verify - // Use regex to match the tag content regardless of whitespace - expect(result).toMatch( - new RegExp( - `[\\s\\S]*${sourceCodeDef.trim()}[\\s\\S]*`, - ), - ) - }) + it("should handle file read denied", async () => { + mockAskApproval.mockResolvedValueOnce(false) + await setupReadFileTest({ path: "test/file.txt" }) - it("should only have definitions, no content when maxReadFileLine=0", async () => { - // Setup - const maxReadFileLine = 0 - const totalLines = 10 - // Mock content with exactly 10 lines to match totalLines - const rawContent = Array(10).fill("Line content").join("\n") - mockInputContent = rawContent - mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(sourceCodeDef) - - // Execute - skip addLineNumbers check as it's not called for maxReadFileLine=0 - const result = await executeReadFileTool({}, { maxReadFileLine, totalLines, skipAddLineNumbersCheck: true }) - - // Verify - expect(result).toContain(`Showing only 0 of ${totalLines} total lines`) - // Use regex to match the tag content regardless of whitespace - expect(result).toMatch( - new RegExp( - `[\\s\\S]*${sourceCodeDef.trim()}[\\s\\S]*`, - ), - ) - expect(result).not.toContain(` { - // Setup - const maxReadFileLine = 0 - const totalLines = 10 - // Mock that no source code definitions are available - mockedParseSourceCodeDefinitionsForFile.mockResolvedValue("") - // Mock content with exactly 10 lines to match totalLines - const rawContent = Array(10).fill("Line content").join("\n") - mockInputContent = rawContent - - // Execute - skip addLineNumbers check as it's not called for maxReadFileLine=0 - const result = await executeReadFileTool({}, { maxReadFileLine, totalLines, skipAddLineNumbersCheck: true }) - - // Verify - // Should include notice - expect(result).toContain( - `${testFilePath}\nShowing only 0 of ${totalLines} total lines. Use start_line and end_line if you need to read more\n`, - ) - // Should not include list_code_definition_names tag since there are no definitions - expect(result).not.toContain("") - // Should not include content tag for non-empty files with maxReadFileLine=0 - expect(result).not.toContain(" { - it("should include error tag for invalid path", async () => { - // Setup - missing path parameter - const toolUse: ReadFileToolUse = { - type: "tool_use", + it("should handle partial block processing", async () => { + const block: ToolUse = { + type: "tool_use" as const, name: "read_file", - params: {}, - partial: false, + params: { path: "test/file.txt" }, + partial: true, } - // Execute the tool await readFileTool( mockCline, - toolUse, - mockCline.ask, - jest.fn(), - (result: ToolResponse) => { - toolResult = result - }, - (param: ToolParamName, content?: string) => content ?? "", + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, ) - // Verify - expect(toolResult).toContain(``) - expect(toolResult).not.toContain(` { - // Execute - skip addLineNumbers check as it returns early with an error - const result = await executeReadFileTool({ start_line: "invalid" }, { skipAddLineNumbersCheck: true }) - - // Verify - expect(result).toContain(`${testFilePath}Invalid start_line value`) - expect(result).not.toContain(` { - // Execute - skip addLineNumbers check as it returns early with an error - const result = await executeReadFileTool({ end_line: "invalid" }, { skipAddLineNumbersCheck: true }) - - // Verify - expect(result).toContain(`${testFilePath}Invalid end_line value`) - expect(result).not.toContain(` { - // Execute - skip addLineNumbers check as it returns early with an error - const result = await executeReadFileTool({}, { validateAccess: false, skipAddLineNumbersCheck: true }) - - // Verify - expect(result).toContain(`${testFilePath}`) - expect(result).not.toContain(` { - it("should handle empty files correctly with maxReadFileLine=-1", async () => { - // Setup - use empty string - mockInputContent = "" - const maxReadFileLine = -1 - const totalLines = 0 - mockedCountFileLines.mockResolvedValue(totalLines) - - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine, totalLines }) - console.log(result) - - // Verify - // Empty files should include a content tag and notice - expect(result).toBe(`${testFilePath}\nFile is empty\n`) - // And make sure there's no error - expect(result).not.toContain(``) - }) - - it("should handle empty files correctly with maxReadFileLine=0", async () => { - // Setup - use empty string - mockInputContent = "" - const maxReadFileLine = 0 - const totalLines = 0 - mockedCountFileLines.mockResolvedValue(totalLines) - - // Execute - const result = await executeReadFileTool({}, { maxReadFileLine, totalLines }) - - // Verify - // Empty files should include a content tag and notice even with maxReadFileLine=0 - expect(result).toBe(`${testFilePath}\nFile is empty\n`) - }) - - it("should handle binary files correctly", async () => { - // Setup - // For binary content, we need to override the mock since we don't use addLineNumbers - mockedExtractTextFromFile.mockResolvedValue("Binary content") - - // Execute - skip addLineNumbers check as we're directly mocking extractTextFromFile - const result = await executeReadFileTool({}, { isBinary: true, skipAddLineNumbersCheck: true }) - - // Verify - expect(result).toBe( - `${testFilePath}\n\nBinary content\n`, - ) - expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(mockCline.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"tool":"readFile"'), true) + expect(processFileForReading).not.toHaveBeenCalled() + expect(mockPushToolResult).not.toHaveBeenCalled() }) - it("should handle file read errors correctly", async () => { - // Setup - const errorMessage = "File not found" - // For error cases, we need to override the mock to simulate a failure - mockedExtractTextFromFile.mockRejectedValue(new Error(errorMessage)) + it("should handle error during processFileForReading", async () => { + const error = new Error("Simulated read error") + ;(processFileForReading as jest.Mock).mockRejectedValueOnce(error) - // Execute - skip addLineNumbers check as it throws an error - const result = await executeReadFileTool({}, { skipAddLineNumbersCheck: true }) + await setupReadFileTest({ path: "test/file.txt" }) - // Verify - expect(result).toContain( - `${testFilePath}Error reading file: ${errorMessage}`, + expect(mockHandleError).toHaveBeenCalledWith("reading file", error) + expect(mockPushToolResult).toHaveBeenCalledWith( + "test/file.txtError reading file: Simulated read error", ) - expect(result).not.toContain(`(.*?)<\/file>/g + for (const match of filesParam.matchAll(fileRegex)) { + const fileString = match[1] + // Parse the file string to extract path and optional line range + // Format could be: path/to/file.js or path/to/file.js:10:20 + const rangeRegex = /^(.*?)(?::(\d+):(\d+))?$/ + const rangeMatch = fileString.match(rangeRegex) + + if (rangeMatch) { + const [, filePath, startLineStr, endLineStr] = rangeMatch + const fileSpec: AttachedFileSpec = { path: filePath } + + // Convert line numbers to numbers if they exist + if (startLineStr && endLineStr) { + fileSpec.startLine = parseInt(startLineStr, 10) + fileSpec.endLine = parseInt(endLineStr, 10) + } + + attachedFiles.push(fileSpec) + } + } + } + const toolMessage = JSON.stringify({ tool: "newTask", mode: targetMode.name, content: message, + files: attachedFiles, }) const didApprove = await askApproval("tool", toolMessage) @@ -78,7 +105,11 @@ export async function newTaskTool( // Delay to allow mode change to take effect before next tool is executed. await delay(500) - const newCline = await provider.initClineWithTask(message, undefined, cline) + const newCline = await provider.initClineWithTask(message, undefined, cline, { + attachedFiles, + enableDiff: true, + enableCheckpoints: true, + }) cline.emit("taskSpawned", newCline.taskId) pushToolResult(`Successfully created new task in ${targetMode.name} mode with message: ${message}`) diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts index 67fd4b5e96..b5280c52b9 100644 --- a/src/core/tools/readFileTool.ts +++ b/src/core/tools/readFileTool.ts @@ -1,18 +1,13 @@ import path from "path" -import { isBinaryFile } from "isbinaryfile" import { Task } from "../task/Task" import { ClineSayTool } from "../../shared/ExtensionMessage" -import { formatResponse } from "../prompts/responses" import { t } from "../../i18n" import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" 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 { processFileForReading, formatProcessedFileResultToString } from "../../shared/fileReadUtils" export async function readFileTool( cline: Task, @@ -52,56 +47,30 @@ export async function readFileTool( const { maxReadFileLine = -1 } = (await cline.providerRef.deref()?.getState()) ?? {} const isFullRead = maxReadFileLine === -1 - // 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 and we're not doing a full read - if (!isFullRead && (startLineStr || endLineStr)) { - isRangeRead = true - } - - // Parse start_line if provided + // Parse start_line if provided (keep as 1-based) + let requestedStartLine: number | undefined = undefined if (startLineStr) { - startLine = parseInt(startLineStr) - - if (isNaN(startLine)) { - // Invalid start_line + requestedStartLine = parseInt(startLineStr) + if (isNaN(requestedStartLine)) { cline.consecutiveMistakeCount++ cline.recordToolError("read_file") await cline.say("error", `Failed to parse start_line: ${startLineStr}`) pushToolResult(`${relPath}Invalid start_line value`) return } - - startLine -= 1 // Convert to 0-based index } - // Parse end_line if provided + // Parse end_line if provided (keep as 1-based) + let requestedEndLine: number | undefined = undefined if (endLineStr) { - endLine = parseInt(endLineStr) - - if (isNaN(endLine)) { - // Invalid end_line + requestedEndLine = parseInt(endLineStr) + if (isNaN(requestedEndLine)) { cline.consecutiveMistakeCount++ cline.recordToolError("read_file") await cline.say("error", `Failed to parse end_line: ${endLineStr}`) pushToolResult(`${relPath}Invalid end_line value`) return } - - // Convert to 0-based index - endLine -= 1 - } - - const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) - - if (!accessAllowed) { - await cline.say("rooignore_error", relPath) - const errorMsg = formatResponse.rooIgnoreError(relPath) - pushToolResult(`${relPath}${errorMsg}`) - return } // Create line snippet description for approval message @@ -109,12 +78,12 @@ export async function readFileTool( if (isFullRead) { // No snippet for full read - } else if (startLine !== undefined && endLine !== undefined) { - lineSnippet = t("tools:readFile.linesRange", { start: startLine + 1, end: endLine + 1 }) - } else if (startLine !== undefined) { - lineSnippet = t("tools:readFile.linesFromToEnd", { start: startLine + 1 }) - } else if (endLine !== undefined) { - lineSnippet = t("tools:readFile.linesFromStartTo", { end: endLine + 1 }) + } else if (requestedStartLine !== undefined && requestedEndLine !== undefined) { + lineSnippet = t("tools:readFile.linesRange", { start: requestedStartLine, end: requestedEndLine }) + } else if (requestedStartLine !== undefined) { + lineSnippet = t("tools:readFile.linesFromToEnd", { start: requestedStartLine }) + } else if (requestedEndLine !== undefined) { + lineSnippet = t("tools:readFile.linesFromStartTo", { end: requestedEndLine }) } else if (maxReadFileLine === 0) { lineSnippet = t("tools:readFile.definitionsOnly") } else if (maxReadFileLine > 0) { @@ -136,120 +105,23 @@ export async function readFileTool( return } - // 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) : "", - (async () => { - try { - return await parseSourceCodeDefinitionsForFile(absolutePath, cline.rooIgnoreController) - } catch (error) { - if (error instanceof Error && error.message.startsWith("Unsupported language:")) { - console.warn(`[read_file] Warning: ${error.message}`) - return undefined - } else { - console.error( - `[read_file] Unhandled error: ${error instanceof Error ? error.message : String(error)}`, - ) - return undefined - } - } - })(), - ]) - - content = res[0].length > 0 ? addLineNumbers(res[0]) : "" - const result = res[1] - - if (result) { - sourceCodeDef = `${result}` - } - } else { - // Read entire file - content = await extractTextFromFile(absolutePath) - } - - // Create variables to store XML components - let xmlInfo = "" - let contentTag = "" - - // Add truncation notice if applicable - if (isFileTruncated) { - xmlInfo += `Showing only ${maxReadFileLine} of ${totalLines} total lines. Use start_line and end_line if you need to read more\n` - - // Add source code definitions if available - if (sourceCodeDef) { - xmlInfo += `${sourceCodeDef}\n` - } - } - - // Empty files (zero lines) - if (content === "" && totalLines === 0) { - // Always add self-closing content tag and notice for empty files - contentTag = `` - xmlInfo += `File is empty\n` - } - // Range reads should always show content regardless of maxReadFileLine - else if (isRangeRead) { - // Create content tag with line range information - let lineRangeAttr = "" - const displayStartLine = startLine !== undefined ? startLine + 1 : 1 - const displayEndLine = endLine !== undefined ? endLine + 1 : totalLines - lineRangeAttr = ` lines="${displayStartLine}-${displayEndLine}"` - - // Maintain exact format expected by tests - contentTag = `\n${content}\n` - } - // maxReadFileLine=0 for non-range reads - else if (maxReadFileLine === 0) { - // Skip content tag for maxReadFileLine=0 (definitions only mode) - contentTag = "" - } - // Normal case: non-empty files with content (non-range reads) - else { - // For non-range reads, always show line range - let lines = totalLines - - if (maxReadFileLine >= 0 && totalLines > maxReadFileLine) { - lines = maxReadFileLine - } - - const lineRangeAttr = ` lines="1-${lines}"` - - // Maintain exact format expected by tests - contentTag = `\n${content}\n` - } + // Use shared utility functions for file processing + const result = await processFileForReading( + absolutePath, + relPath, + maxReadFileLine, + requestedStartLine, + requestedEndLine, + cline.rooIgnoreController, + ) // Track file read operation if (relPath) { await cline.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource) } - // Format the result into the required XML structure - const xmlResult = `${relPath}\n${contentTag}${xmlInfo}` + // Format and push the result + const xmlResult = formatProcessedFileResultToString(result) pushToolResult(xmlResult) } } catch (error) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 13121531a7..ddef93f971 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -27,6 +27,7 @@ import { t } from "../../i18n" import { setPanel } from "../../activate/registerCommands" import { Package } from "../../shared/package" import { requestyDefaultModelId, openRouterDefaultModelId, glamaDefaultModelId } from "../../shared/api" +import { AttachedFileSpec } from "../../shared/tools" import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" import { GlobalFileNames } from "../../shared/globalFileNames" @@ -483,8 +484,11 @@ export class ClineProvider Pick< TaskOptions, "enableDiff" | "enableCheckpoints" | "fuzzyMatchThreshold" | "consecutiveMistakeLimit" | "experiments" + | "attachedFiles" > - > = {}, + > & { + attachedFiles?: AttachedFileSpec[] + } = {}, ) { const { apiConfiguration, @@ -1301,6 +1305,7 @@ export class ClineProvider ? (taskHistory || []).find((item: HistoryItem) => item.id === this.getCurrentCline()?.taskId) : undefined, clineMessages: this.getCurrentCline()?.clineMessages || [], + attachedFiles: this.getCurrentCline()?.attachedFiles || [], taskHistory: (taskHistory || []) .filter((item: HistoryItem) => item.ts && item.task) .sort((a: HistoryItem, b: HistoryItem) => b.ts - a.ts), diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 5586e1327b..a77e937f6e 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -14,6 +14,7 @@ import { GitCommit } from "../utils/git" import { McpServer } from "./mcp" import { Mode } from "./modes" import { RouterModels } from "./api" +import { AttachedFileSpec } from "./tools" export interface LanguageModelChatSelector { vendor?: string @@ -30,6 +31,7 @@ export interface ExtensionMessage { | "action" | "state" | "selectedImages" + | "attachedFiles" | "theme" | "workspaceUpdated" | "invoke" @@ -184,6 +186,8 @@ export type ExtensionState = Pick< taskHistory: HistoryItem[] + attachedFiles?: AttachedFileSpec[] + writeDelayMs: number requestDelaySeconds: number @@ -246,6 +250,7 @@ export interface ClineSayTool { endLine?: number lineNumber?: number query?: string + files?: AttachedFileSpec[] } // Must keep in sync with system prompt. diff --git a/src/shared/__tests__/fileReadUtils.test.ts b/src/shared/__tests__/fileReadUtils.test.ts new file mode 100644 index 0000000000..eadaa295bd --- /dev/null +++ b/src/shared/__tests__/fileReadUtils.test.ts @@ -0,0 +1,495 @@ +import { isBinaryFile } from "isbinaryfile" +import * as path from "path" +import { processFileForReading } from "../fileReadUtils" +import { RooIgnoreController } from "../../core/ignore/RooIgnoreController" + +// Define default mock content source +const getGlobalDefaultMockLines = () => Array.from({ length: 5 }, (_, i) => `line ${i + 1}`) +const getGlobalDefaultMockContentString = () => getGlobalDefaultMockLines().join("\n") + +jest.mock("path", () => { + const originalPath = jest.requireActual("path") + return { + ...originalPath, + resolve: jest.fn().mockImplementation((...args) => args.join("/")), + } +}) + +jest.mock("node:fs", () => { + const originalFs = jest.requireActual("node:fs") + const streamMock = { + on: jest.fn().mockReturnThis(), + pipe: jest.fn().mockReturnThis(), + // @ts-ignore + [Symbol.asyncIterator]: async function* () { + for (const line of getGlobalDefaultMockLines()) { + yield line + } + }, + } + return { + ...originalFs, + createReadStream: jest.fn().mockReturnValue(streamMock), + promises: { + readFile: jest.fn().mockImplementation(async () => getGlobalDefaultMockContentString()), + }, + } +}) + +jest.mock("node:readline", () => ({ + createInterface: jest.fn().mockImplementation((_options) => ({ + [Symbol.asyncIterator]: jest.fn().mockImplementation(async function* () { + for (const line of getGlobalDefaultMockLines()) { + yield line + } + }), + close: jest.fn(), + })), +})) + +jest.mock("isbinaryfile") +jest.mock("../../services/tree-sitter", () => ({ + parseSourceCodeDefinitionsForFile: jest.fn(), +})) + +jest.mock("../../core/ignore/RooIgnoreController", () => ({ + RooIgnoreController: class { + // Mock only the method needed by fileReadUtils + validateAccess: jest.Mock = jest.fn().mockReturnValue(true) + }, +})) + +describe("processFileForReading", () => { + const testFilePath = "test/file.txt" + const absoluteFilePath = "/test/file.txt" + + const mockedIsBinaryFile = isBinaryFile as jest.MockedFunction + const mockedPathResolve = path.resolve as jest.MockedFunction + const mockRooIgnoreController = new RooIgnoreController("/mock/cwd") + + beforeEach(() => { + jest.clearAllMocks() + mockedPathResolve.mockReturnValue(absoluteFilePath) + mockedIsBinaryFile.mockResolvedValue(false) + }) + + it("should process a text file with line range", async () => { + const result = await processFileForReading(absoluteFilePath, testFilePath, 500, 2, 4, mockRooIgnoreController) + + expect(result).toEqual({ + relativePath: testFilePath, + contentWithLineNumbers: expect.any(String), + totalLinesInFile: expect.any(Number), + isBinary: false, + wasTruncated: false, + wasRangeRead: true, + actualStartLine: 2, + actualEndLine: 4, + }) + }) + + it("should handle binary files", async () => { + mockedIsBinaryFile.mockResolvedValue(true) + + const result = await processFileForReading(absoluteFilePath, testFilePath, 500, 1, 5, mockRooIgnoreController) + + expect(result).toEqual({ + relativePath: testFilePath, + notice: "File is binary. Content display may be limited.", + totalLinesInFile: expect.any(Number), + isBinary: true, + wasTruncated: false, + wasRangeRead: true, + }) + }) + + describe("processFileForReading with maxReadFileLine setting", () => { + it("should return full content when maxReadFileLine is negative (-1)", async () => { + // This test will now use the global default mocks (5 lines of "line X") + // fs.promises.readFile will provide the 5 lines, and countFileLines will also count 5 lines. + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + -1, // maxReadFileLine = -1 + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + contentWithLineNumbers: getGlobalDefaultMockLines() + .map((line, i) => `${i + 1} | ${line}`) + .join("\n"), + totalLinesInFile: 5, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + actualStartLine: 1, + actualEndLine: 5, + }) + }) + + it("should truncate content when maxReadFileLine is less than file length", async () => { + // This test uses the global default mocks (5 lines "line X"). + // processFileForReading will call countFileLines (sees 5 lines), + // then determine truncation, then call readLines which will read from the 5-line mock + // but limit to maxReadFileLine. + // NO local mock override for readline needed here for this specific scenario. + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 3, // maxReadFileLine = 3 + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + contentWithLineNumbers: "1 | line 1\n2 | line 2\n3 | line 3", + totalLinesInFile: 5, + isBinary: false, + wasTruncated: true, + wasRangeRead: false, + actualStartLine: 1, + actualEndLine: 3, + notice: "Showing only 3 of 5 total lines. Use start_line and end_line if you need to read more.", + }) + }) + + it("should return full content when maxReadFileLine equals file length", async () => { + // Uses global default mocks (5 lines "line X") + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 5, // maxReadFileLine = 5 + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + contentWithLineNumbers: getGlobalDefaultMockLines() + .map((line, i) => `${i + 1} | ${line}`) + .join("\n"), + totalLinesInFile: 5, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + actualStartLine: 1, + actualEndLine: 5, + }) + }) + + it("should return full content when maxReadFileLine exceeds file length", async () => { + // Uses global default mocks (5 lines "line X") + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 10, // maxReadFileLine = 10 + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + contentWithLineNumbers: "1 | line 1\n2 | line 2\n3 | line 3\n4 | line 4\n5 | line 5", + totalLinesInFile: 5, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + actualStartLine: 1, + actualEndLine: 5, + }) + }) + }) + + describe("processFileForReading with invalid range parameters", () => { + it("should return error for non-numeric start_line", async () => { + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 500, + "invalid" as any, + undefined, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + error: "Invalid start_line value", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + }) + }) + + it("should return error for non-numeric end_line", async () => { + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 500, + undefined, + "invalid" as any, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + error: "Invalid end_line value", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + }) + }) + + it("should return error for negative start_line", async () => { + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 500, + -1, + undefined, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + error: "Invalid start_line value", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + }) + }) + + it("should return error for negative end_line", async () => { + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 500, + undefined, + -1, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + error: "Invalid end_line value", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + }) + }) + + it("should return error when start_line > end_line", async () => { + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 500, + 5, + 3, + mockRooIgnoreController, + ) + + expect(result).toEqual({ + relativePath: testFilePath, + error: "start_line must be less than or equal to end_line", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + }) + }) + }) + + describe("processFileForReading line numbering verification", () => { + it("should correctly number lines for full file read", async () => { + const specificMockLines = ["first line", "second line", "third line"] + const specificMockContentString = specificMockLines.join("\n") + const { createInterface: mockRlCreateInterface } = require("node:readline") + const { promises: mockFsPromises } = require("node:fs") + + mockRlCreateInterface.mockImplementation(() => ({ + [Symbol.asyncIterator]: async function* () { + for (const line of specificMockLines) yield line + }, + close: jest.fn(), + })) + mockFsPromises.readFile.mockResolvedValue(specificMockContentString) + + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + -1, // full read + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(result.contentWithLineNumbers).toBe("1 | first line\n2 | second line\n3 | third line") + expect(result.totalLinesInFile).toBe(3) + }) + + it("should correctly number lines for truncated read", async () => { + // Simulating a file that has 3 lines, but we only want to read 2. + const fullFileSpecificMockLines = ["first line", "second line", "third line"] + const { createInterface: mockRlCreateInterface } = require("node:readline") + // fs.promises.readFile mock is not strictly needed here if truncation path is taken, + // but good practice if the test was ever to change to not truncate. + // For this test, countFileLines needs to see 3 lines. + mockRlCreateInterface.mockImplementation(() => ({ + [Symbol.asyncIterator]: async function* () { + for (const line of fullFileSpecificMockLines) yield line + }, + close: jest.fn(), + })) + + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 2, // truncate at 2 lines + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(result.contentWithLineNumbers).toBe("1 | first line\n2 | second line") + }) + + it("should correctly number lines for range read", async () => { + const specificMockLinesForRangeTest = [ + "range test line 1", + "range test line 2", // This is the one we want to start with (line 2) + "range test line 3", // This is the one we want to end with (line 3) + "range test line 4", + ] + const { createInterface: mockRlCreateInterface } = require("node:readline") + // countFileLines and readLines both use readline, so this mock serves both. + mockRlCreateInterface.mockImplementation(() => ({ + [Symbol.asyncIterator]: async function* () { + for (const line of specificMockLinesForRangeTest) { + yield line + } + }, + close: jest.fn(), + })) + + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 500, // maxReadFileLine (not strictly relevant for pure range read if range is smaller) + 2, // requestedStartLine + 3, // requestedEndLine + mockRooIgnoreController, + ) + + expect(result.contentWithLineNumbers).toBe("2 | range test line 2\n3 | range test line 3") + expect(result.totalLinesInFile).toBe(specificMockLinesForRangeTest.length) + expect(result.actualStartLine).toBe(2) + expect(result.actualEndLine).toBe(3) + expect(result.wasRangeRead).toBe(true) + expect(result.notice).toBeUndefined() + }) + }) + + it("should handle access denied files", async () => { + const denyingController = new RooIgnoreController("/mock/cwd") + denyingController.validateAccess = jest.fn().mockReturnValue(false) as any + + const result = await processFileForReading(absoluteFilePath, testFilePath, 500, 1, 5, denyingController) + + expect(result).toEqual({ + relativePath: testFilePath, + error: "Access to file denied by .rooignore", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + }) + }) + + it("should handle empty files", async () => { + const { createReadStream } = require("node:fs") + const { createInterface } = require("node:readline") + + createReadStream.mockImplementation(() => ({ + on: jest.fn((event, _callback) => { + if (event === "data") { + // No data emitted for empty file + return this + } + return this + }), + pipe: jest.fn(), + })) + + createInterface.mockImplementation(() => ({ + [Symbol.asyncIterator]: jest.fn().mockImplementation(function* () { + // No lines yielded for empty file + return + }), + close: jest.fn(), + })) + + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 500, + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(result).toMatchObject({ + relativePath: testFilePath, + notice: "File is empty.", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + }) + }) + + it("should handle files with source code definitions", async () => { + const { parseSourceCodeDefinitionsForFile: localMockedParse } = jest.requireMock("../../services/tree-sitter") + const definitions = "function test() {}" + localMockedParse.mockResolvedValue(definitions) + + const { createInterface } = require("node:readline") + createInterface.mockImplementation(() => ({ + [Symbol.asyncIterator]: jest.fn().mockImplementation(function* () { + for (let i = 1; i <= 5; i++) { + yield `line ${i}` + } + }), + close: jest.fn(), + })) + + const result = await processFileForReading( + absoluteFilePath, + testFilePath, + 0, // maxReadFileLine = 0 to trigger definitions lookup + undefined, + undefined, + mockRooIgnoreController, + ) + + expect(localMockedParse).toHaveBeenCalled() + expect(result).toMatchObject({ + relativePath: testFilePath, + sourceCodeDefinitions: definitions, + notice: "Content omitted (maxReadFileLine: 0). Showing definitions if available.", + totalLinesInFile: 5, + isBinary: false, + wasTruncated: true, + wasRangeRead: false, + }) + }) +}) diff --git a/src/shared/fileReadUtils.ts b/src/shared/fileReadUtils.ts new file mode 100644 index 0000000000..0b4fce8fff --- /dev/null +++ b/src/shared/fileReadUtils.ts @@ -0,0 +1,269 @@ +/** + * Utilities for reading and processing files with various options and controls + */ + +import { RooIgnoreController } from "../core/ignore/RooIgnoreController" +import { isBinaryFile } from "isbinaryfile" +import * as fs from "node:fs" +import * as readline from "node:readline" +import { parseSourceCodeDefinitionsForFile } from "../services/tree-sitter" + +// Utility functions +async function countFileLines(filePath: string): Promise { + const fileStream = fs.createReadStream(filePath) + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity, + }) + + let count = 0 + for await (const _ of rl) { + count++ + } + return count +} + +async function readLines(filePath: string, endLine: number | undefined, startLine = 0): Promise { + const fileStream = fs.createReadStream(filePath) + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity, + }) + + const lines: string[] = [] + let currentLine = 0 + + for await (const line of rl) { + if (currentLine >= startLine && (endLine === undefined || currentLine <= endLine)) { + lines.push(line) + } + currentLine++ + if (endLine !== undefined && currentLine > endLine) break + } + + return lines +} + +function addLineNumbers(lines: string[], startAt = 1): string { + return lines.map((line, i) => `${startAt + i} | ${line}`).join("\n") +} + +async function extractTextFromFile(filePath: string): Promise { + const content = await fs.promises.readFile(filePath, "utf-8") + return addLineNumbers(content.split("\n"), 1) +} + +/** + * Represents the result of processing a file read operation + */ +export interface ProcessedFileReadResult { + /** Relative path to the file from workspace root */ + relativePath: string + /** File content with line numbers (if applicable) */ + contentWithLineNumbers?: string + /** Notice or warning message about the read operation */ + notice?: string + /** Extracted source code definitions (if applicable) */ + sourceCodeDefinitions?: string + /** Error message if reading failed */ + error?: string + /** Actual start line that was read (1-based) */ + actualStartLine?: number + /** Actual end line that was read (1-based) */ + actualEndLine?: number + /** Total number of lines in the file */ + totalLinesInFile: number + /** Whether the file is binary */ + isBinary: boolean + /** Whether the content was truncated */ + wasTruncated: boolean + /** Whether a range was read (vs full file) */ + wasRangeRead: boolean +} + +/** + * Processes a file for reading with various options and controls + * @param absolutePath Absolute path to the file + * @param relativePath Relative path from workspace root + * @param maxReadFileLine Maximum lines to read + * @param requestedStartLine Optional start line (1-based) + * @param requestedEndLine Optional end line (1-based) + * @param rooIgnoreController Optional ignore controller + * @returns Processed file read result + */ +export async function processFileForReading( + absolutePath: string, + relativePath: string, + maxReadFileLine: number, + requestedStartLine: number | undefined, + requestedEndLine: number | undefined, + rooIgnoreController: RooIgnoreController | undefined, +): Promise { + // Initial checks & setup + if (rooIgnoreController && !rooIgnoreController.validateAccess(relativePath)) { + return { + relativePath, + error: "Access to file denied by .rooignore", + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, + } + } + + // Validate range parameters + const baseErrorResult = { + relativePath, + totalLinesInFile: 0, + isBinary: false, // Assuming not binary until checked, or error occurs before + wasTruncated: false, + wasRangeRead: false, // If range params are invalid, it's not a valid range read + } + + if (requestedStartLine !== undefined) { + if (typeof requestedStartLine !== "number" || isNaN(requestedStartLine) || requestedStartLine < 1) { + return { ...baseErrorResult, error: "Invalid start_line value" } + } + } + if (requestedEndLine !== undefined) { + if (typeof requestedEndLine !== "number" || isNaN(requestedEndLine) || requestedEndLine < 1) { + return { ...baseErrorResult, error: "Invalid end_line value" } + } + } + if (requestedStartLine !== undefined && requestedEndLine !== undefined && requestedStartLine > requestedEndLine) { + return { ...baseErrorResult, error: "start_line must be less than or equal to end_line" } + } + + // Count lines and check for binary (moved after range validation) + let totalLinesInFile = 0 + try { + totalLinesInFile = await countFileLines(absolutePath) + } catch (error) { + return { + relativePath, + error: `Failed to count lines: ${error instanceof Error ? error.message : String(error)}`, + totalLinesInFile: 0, + isBinary: false, + wasTruncated: false, + wasRangeRead: false, // If counting lines fails, it's not a successful range read + } + } + + const isBinary = await isBinaryFile(absolutePath).catch(() => false) + // Determine wasRangeRead *after* validation and *before* it's used for logic + const wasRangeRead = requestedStartLine !== undefined || requestedEndLine !== undefined + const startLine0Based = requestedStartLine ? requestedStartLine - 1 : 0 + const endLine0Based = requestedEndLine ? requestedEndLine - 1 : undefined + + // Initialize result object + const result: ProcessedFileReadResult = { + relativePath, + totalLinesInFile, + isBinary, + wasTruncated: false, + wasRangeRead, + } + + // Handle binary files + if (isBinary) { + result.notice = "File is binary. Content display may be limited." + return result + } + + // Determine read strategy + if (wasRangeRead) { + // Range read logic + const linesArray = await readLines(absolutePath, endLine0Based, startLine0Based) + result.contentWithLineNumbers = addLineNumbers(linesArray, requestedStartLine || 1) + result.actualStartLine = requestedStartLine || 1 + result.actualEndLine = result.actualStartLine + linesArray.length - 1 + + if (requestedEndLine && result.actualEndLine < requestedEndLine) { + result.notice = `File ended at line ${result.actualEndLine} (requested to ${requestedEndLine})` + } + } else { + // Full or partial read logic + const wasTruncated = maxReadFileLine >= 0 && totalLinesInFile > maxReadFileLine + result.wasTruncated = wasTruncated + + if (maxReadFileLine === 0) { + result.notice = "Content omitted (maxReadFileLine: 0). Showing definitions if available." + } else if (wasTruncated) { + const linesArray = await readLines(absolutePath, maxReadFileLine - 1, 0) + result.contentWithLineNumbers = addLineNumbers(linesArray, 1) + result.actualStartLine = 1 + result.actualEndLine = maxReadFileLine + result.notice = `Showing only ${maxReadFileLine} of ${totalLinesInFile} total lines. Use start_line and end_line if you need to read more.` + } else { + result.contentWithLineNumbers = await extractTextFromFile(absolutePath) + result.actualStartLine = 1 + result.actualEndLine = totalLinesInFile + } + + // Get source code definitions if needed + if ((wasTruncated && maxReadFileLine > 0) || (maxReadFileLine === 0 && !wasRangeRead)) { + try { + const definitions = await parseSourceCodeDefinitionsForFile(absolutePath, rooIgnoreController) + if (definitions) { + result.sourceCodeDefinitions = definitions + } + } catch (error) { + if (error instanceof Error && !error.message.startsWith("Unsupported language:")) { + console.error(`Error parsing definitions: ${error.message}`) + } + } + } + } + + // Handle empty files + if (totalLinesInFile === 0 && !isBinary) { + result.notice = "File is empty." + } + + return result +} + +/** + * Formats a processed file result to a string representation + * @param result The processed file result + * @returns Formatted string + */ +export function formatProcessedFileResultToString(result: ProcessedFileReadResult): string { + // Handle errors first + if (result.error) { + return `${result.relativePath}${result.error}` + } + + // Initialize XML components + let xmlInfo = "" + let contentTag = "" + const pathTag = `${result.relativePath}\n` + + // Build xmlInfo from result properties + if (result.notice) { + xmlInfo += `${result.notice}\n` + } + if (result.sourceCodeDefinitions) { + xmlInfo += `${result.sourceCodeDefinitions}\n` + } + + // Build contentTag based on file type and read mode + if (result.isBinary) { + contentTag = `\n` + } else if (result.totalLinesInFile === 0) { + contentTag = `\n` + } else if (result.wasRangeRead) { + const lineRangeAttr = `lines="${result.actualStartLine}-${result.actualEndLine}"` + contentTag = `\n${result.contentWithLineNumbers || ""}\n` + } else { + if (result.contentWithLineNumbers === undefined) { + contentTag = "" + } else { + const lineRangeAttr = `lines="${result.actualStartLine}-${result.actualEndLine}"` + contentTag = `\n${result.contentWithLineNumbers || ""}\n` + } + } + + // Assemble final XML string + return `${pathTag}${contentTag}${xmlInfo}` +} diff --git a/src/shared/tools.ts b/src/shared/tools.ts index 6fc32b98c7..5fcae451ac 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -26,6 +26,15 @@ export interface TextContent { partial: boolean } +/** + * Represents a file to be attached to a task, optionally with line range specifications + */ +export interface AttachedFileSpec { + path: string + startLine?: number + endLine?: number +} + export const toolParamNames = [ "command", "path", @@ -64,6 +73,7 @@ export const toolParamNames = [ "start_line", "end_line", "query", + "files", ] as const export type ToolParamName = (typeof toolParamNames)[number] @@ -154,7 +164,7 @@ export interface SwitchModeToolUse extends ToolUse { export interface NewTaskToolUse extends ToolUse { name: "new_task" - params: Partial, "mode" | "message">> + params: Partial, "mode" | "message" | "files">> } export interface SearchAndReplaceToolUse extends ToolUse { diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index e6b8bb601a..5027c4620e 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -573,6 +573,7 @@ export const ChatRowContent = ({ ) case "newTask": + const files = tool.files return ( <>
@@ -590,9 +591,8 @@ export const ChatRowContent = ({ marginTop: "4px", backgroundColor: "var(--vscode-badge-background)", border: "1px solid var(--vscode-badge-background)", - borderRadius: "4px 4px 0 0", + borderRadius: "4px", overflow: "hidden", - marginBottom: "2px", }}>
+ + {files && files.length > 0 && ( + <> +
+ {files.map((file) => { + return ( + { + vscode.postMessage({ + type: "openFile", + text: "./" + file.path, + values: { + line: file.startLine, + }, + }) + }}> + @ + {file.path.replace(/\\/g, "/").split("/").pop() + + (file.endLine + ? `:${file.startLine}-${file.endLine}` + : file.startLine + ? `:${file.startLine}` + : "")} + + ) + })} +
+ + )}
diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index b7cdf75f25..edd68001da 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -18,9 +18,8 @@ import { CustomSupportPrompts } from "@roo/support-prompt" import { experimentDefault } from "@roo/experiments" import { TelemetrySetting } from "@roo/TelemetrySetting" import { RouterModels } from "@roo/api" +import { AttachedFileSpec } from "../../../src/shared/tools" -import { vscode } from "@src/utils/vscode" -import { convertTextMateToHljs } from "@src/utils/textMateToHljs" export interface ExtensionStateContextType extends ExtensionState { historyPreviewCollapsed?: boolean // Add the new state property @@ -31,6 +30,7 @@ export interface ExtensionStateContextType extends ExtensionState { hasSystemPromptOverride?: boolean currentCheckpoint?: string filePaths: string[] + attachedFiles?: AttachedFileSpec[] openedTabs: Array<{ label: string; isActive: boolean; path?: string }> condensingApiConfigId?: string setCondensingApiConfigId: (value: string) => void @@ -200,6 +200,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode codebaseIndexEmbedderModelId: "", }, codebaseIndexModels: { ollama: {}, openai: {} }, + attachedFiles: [], }) const [didHydrateState, setDidHydrateState] = useState(false)