diff --git a/src/core/prompts/sections/rules.ts b/src/core/prompts/sections/rules.ts index 3b2e992085..196c03e5c5 100644 --- a/src/core/prompts/sections/rules.ts +++ b/src/core/prompts/sections/rules.ts @@ -15,7 +15,7 @@ function getEditingInstructions(diffStrategy?: DiffStrategy): string { availableTools.push("write_to_file (for creating new files or complete file rewrites)") } - availableTools.push("insert_content (for adding lines to existing files)") + availableTools.push("insert_content (for adding lines to files)") availableTools.push("search_and_replace (for finding and replacing individual pieces of text)") // Base editing instruction mentioning all available tools diff --git a/src/core/tools/__tests__/insertContentTool.test.ts b/src/core/tools/__tests__/insertContentTool.test.ts new file mode 100644 index 0000000000..c9c48ba5b0 --- /dev/null +++ b/src/core/tools/__tests__/insertContentTool.test.ts @@ -0,0 +1,243 @@ +import * as path from "path" +import * as fs from "fs/promises" + +import { fileExistsAtPath } from "../../../utils/fs" +import { ToolUse, ToolResponse } from "../../../shared/tools" +import { insertContentTool } from "../insertContentTool" + +// Mock external dependencies +jest.mock("path", () => { + const originalPath = jest.requireActual("path") + return { + ...originalPath, + resolve: jest.fn().mockImplementation((...args) => args.join("/")), + } +}) + +jest.mock("fs/promises", () => ({ + readFile: jest.fn(), + writeFile: jest.fn(), +})) + +jest.mock("delay", () => jest.fn()) + +jest.mock("../../../utils/fs", () => ({ + fileExistsAtPath: jest.fn().mockResolvedValue(false), +})) + +jest.mock("../../prompts/responses", () => ({ + formatResponse: { + toolError: jest.fn((msg) => `Error: ${msg}`), + rooIgnoreError: jest.fn((path) => `Access denied: ${path}`), + createPrettyPatch: jest.fn((_path, original, updated) => `Diff: ${original} -> ${updated}`), + }, +})) + +jest.mock("../../../utils/path", () => ({ + getReadablePath: jest.fn().mockReturnValue("test/path.txt"), +})) + +jest.mock("../../ignore/RooIgnoreController", () => ({ + RooIgnoreController: class { + initialize() { + return Promise.resolve() + } + validateAccess() { + return true + } + }, +})) + +// Mock insertGroups from diff/insert-groups +jest.mock("../../diff/insert-groups", () => ({ + insertGroups: jest.fn().mockImplementation((lines, groups) => { + let newLines = [...lines] + for (const group of groups) { + const { index, elements } = group + if (index === -1 || index >= newLines.length) { + // Append to end + newLines.push(...elements) + } else if (index < 0) { + // Insert at beginning (index -1 for line 0, but insertGroups expects 0 for beginning) + // This mock simplifies, assuming index -1 is always append. + // For line 1, index is 0. + newLines.splice(0, 0, ...elements) + } else { + newLines.splice(index, 0, ...elements) + } + } + return newLines + }), +})) + +describe("insertContentTool", () => { + const testFilePath = "test/file.txt" + const absoluteFilePath = "/test/file.txt" + + const mockedFileExistsAtPath = fileExistsAtPath as jest.MockedFunction + const mockedFsReadFile = fs.readFile as jest.MockedFunction + const mockedPathResolve = path.resolve as jest.MockedFunction + const mockedInsertGroups = require("../../diff/insert-groups").insertGroups as jest.MockedFunction + + let mockCline: any + let mockAskApproval: jest.Mock + let mockHandleError: jest.Mock + let mockPushToolResult: jest.Mock + let mockRemoveClosingTag: jest.Mock + let toolResult: ToolResponse | undefined + + beforeEach(() => { + jest.clearAllMocks() + + mockedPathResolve.mockReturnValue(absoluteFilePath) + mockedFileExistsAtPath.mockResolvedValue(true) // Assume file exists by default for insert + mockedFsReadFile.mockResolvedValue("") // Default empty file content + + mockCline = { + cwd: "/", + consecutiveMistakeCount: 0, + didEditFile: false, + rooIgnoreController: { + validateAccess: jest.fn().mockReturnValue(true), + }, + diffViewProvider: { + editType: undefined, + isEditing: false, + originalContent: "", + open: jest.fn().mockResolvedValue(undefined), + update: jest.fn().mockResolvedValue(undefined), + reset: jest.fn().mockResolvedValue(undefined), + revertChanges: jest.fn().mockResolvedValue(undefined), + saveChanges: jest.fn().mockResolvedValue({ + newProblemsMessage: "", + userEdits: null, + finalContent: "final content", + }), + scrollToFirstDiff: jest.fn(), + pushToolWriteResult: jest.fn().mockImplementation(async function ( + this: any, + task: any, + cwd: string, + isNewFile: boolean, + ) { + return "Tool result message" + }), + }, + fileContextTracker: { + trackFileContext: jest.fn().mockResolvedValue(undefined), + }, + say: jest.fn().mockResolvedValue(undefined), + ask: jest.fn().mockResolvedValue({ response: "yesButtonClicked" }), // Default to approval + recordToolError: jest.fn(), + sayAndCreateMissingParamError: jest.fn().mockResolvedValue("Missing param error"), + } + + mockAskApproval = jest.fn().mockResolvedValue(true) + mockHandleError = jest.fn().mockResolvedValue(undefined) + mockRemoveClosingTag = jest.fn((tag, content) => content) + + toolResult = undefined + }) + + async function executeInsertContentTool( + params: Partial = {}, + options: { + fileExists?: boolean + isPartial?: boolean + accessAllowed?: boolean + fileContent?: string + askApprovalResponse?: "yesButtonClicked" | "noButtonClicked" | string + } = {}, + ): Promise { + const fileExists = options.fileExists ?? true + const isPartial = options.isPartial ?? false + const accessAllowed = options.accessAllowed ?? true + const fileContent = options.fileContent ?? "" + + mockedFileExistsAtPath.mockResolvedValue(fileExists) + mockedFsReadFile.mockResolvedValue(fileContent) + mockCline.rooIgnoreController.validateAccess.mockReturnValue(accessAllowed) + mockCline.ask.mockResolvedValue({ response: options.askApprovalResponse ?? "yesButtonClicked" }) + + const toolUse: ToolUse = { + type: "tool_use", + name: "insert_content", + params: { + path: testFilePath, + line: "1", + content: "New content", + ...params, + }, + partial: isPartial, + } + + await insertContentTool( + mockCline, + toolUse, + mockAskApproval, + mockHandleError, + (result: ToolResponse) => { + toolResult = result + }, + mockRemoveClosingTag, + ) + + return toolResult + } + + describe("new file creation logic", () => { + it("creates a new file and inserts content at line 0 (append)", async () => { + const contentToInsert = "New Line 1\nNew Line 2" + await executeInsertContentTool( + { line: "0", content: contentToInsert }, + { fileExists: false, fileContent: "" }, + ) + + expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedFsReadFile).not.toHaveBeenCalled() // Should not read if file doesn't exist + expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(contentToInsert, true) + expect(mockCline.diffViewProvider.editType).toBe("create") + expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) + }) + + it("creates a new file and inserts content at line 1 (beginning)", async () => { + const contentToInsert = "Hello World!" + await executeInsertContentTool( + { line: "1", content: contentToInsert }, + { fileExists: false, fileContent: "" }, + ) + + expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedFsReadFile).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(contentToInsert, true) + expect(mockCline.diffViewProvider.editType).toBe("create") + expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) + }) + + it("creates an empty new file if content is empty string", async () => { + await executeInsertContentTool({ line: "1", content: "" }, { fileExists: false, fileContent: "" }) + + expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedFsReadFile).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("", true) + expect(mockCline.diffViewProvider.editType).toBe("create") + expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) + }) + + it("returns an error when inserting content at an arbitrary line number into a new file", async () => { + const contentToInsert = "Arbitrary insert" + const result = await executeInsertContentTool( + { line: "5", content: contentToInsert }, + { fileExists: false, fileContent: "" }, + ) + + expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedFsReadFile).not.toHaveBeenCalled() + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("insert_content") + expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("non-existent file")) + expect(mockCline.diffViewProvider.update).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.pushToolWriteResult).not.toHaveBeenCalled() + }) + }) +}) diff --git a/src/core/tools/insertContentTool.ts b/src/core/tools/insertContentTool.ts index af8d91713f..704d2bcb54 100644 --- a/src/core/tools/insertContentTool.ts +++ b/src/core/tools/insertContentTool.ts @@ -51,7 +51,7 @@ export async function insertContentTool( return } - if (!content) { + if (content === undefined) { cline.consecutiveMistakeCount++ cline.recordToolError("insert_content") pushToolResult(await cline.sayAndCreateMissingParamError("insert_content", "content")) @@ -70,17 +70,6 @@ export async function insertContentTool( const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false const absolutePath = path.resolve(cline.cwd, relPath) - const fileExists = await fileExistsAtPath(absolutePath) - - if (!fileExists) { - cline.consecutiveMistakeCount++ - cline.recordToolError("insert_content") - const formattedError = `File does not exist at path: ${absolutePath}\n\n\nThe specified file could not be found. Please verify the file path and try again.\n` - await cline.say("error", formattedError) - pushToolResult(formattedError) - return - } - const lineNumber = parseInt(line, 10) if (isNaN(lineNumber) || lineNumber < 0) { cline.consecutiveMistakeCount++ @@ -89,13 +78,26 @@ export async function insertContentTool( return } + const fileExists = await fileExistsAtPath(absolutePath) + let fileContent: string = "" + if (!fileExists) { + if (lineNumber > 1) { + cline.consecutiveMistakeCount++ + cline.recordToolError("insert_content") + const formattedError = `Cannot insert content at line ${lineNumber} into a non-existent file. For new files, 'line' must be 0 (to append) or 1 (to insert at the beginning).` + await cline.say("error", formattedError) + pushToolResult(formattedError) + return + } + } else { + fileContent = await fs.readFile(absolutePath, "utf8") + } + cline.consecutiveMistakeCount = 0 - // Read the file - const fileContent = await fs.readFile(absolutePath, "utf8") - cline.diffViewProvider.editType = "modify" + cline.diffViewProvider.editType = fileExists ? "modify" : "create" cline.diffViewProvider.originalContent = fileContent - const lines = fileContent.split("\n") + const lines = fileExists ? fileContent.split("\n") : [] const updatedContent = insertGroups(lines, [ { @@ -116,7 +118,7 @@ export async function insertContentTool( const diff = formatResponse.createPrettyPatch(relPath, fileContent, updatedContent) - if (!diff) { + if (fileExists && !diff) { pushToolResult(`No changes needed for '${relPath}'`) return } @@ -151,11 +153,7 @@ export async function insertContentTool( cline.didEditFile = true // Get the formatted response message - const message = await cline.diffViewProvider.pushToolWriteResult( - cline, - cline.cwd, - false, // Always false for insert_content - ) + const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) pushToolResult(message)