mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
Fix #3652: Allow insertContentTool to create new files with content
This commit is contained in:
parent
08a0c897ef
commit
8805f7bc01
3 changed files with 264 additions and 23 deletions
|
|
@ -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
|
||||
|
|
|
|||
243
src/core/tools/__tests__/insertContentTool.test.ts
Normal file
243
src/core/tools/__tests__/insertContentTool.test.ts
Normal file
|
|
@ -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<typeof fileExistsAtPath>
|
||||
const mockedFsReadFile = fs.readFile as jest.MockedFunction<typeof fs.readFile>
|
||||
const mockedPathResolve = path.resolve as jest.MockedFunction<typeof path.resolve>
|
||||
const mockedInsertGroups = require("../../diff/insert-groups").insertGroups as jest.MockedFunction<any>
|
||||
|
||||
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<ToolUse["params"]> = {},
|
||||
options: {
|
||||
fileExists?: boolean
|
||||
isPartial?: boolean
|
||||
accessAllowed?: boolean
|
||||
fileContent?: string
|
||||
askApprovalResponse?: "yesButtonClicked" | "noButtonClicked" | string
|
||||
} = {},
|
||||
): Promise<ToolResponse | undefined> {
|
||||
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()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -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<error_details>\nThe specified file could not be found. Please verify the file path and try again.\n</error_details>`
|
||||
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)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue