Fix #4113: Move relPath & newContent checks in writeToFileTool earlie… (#4378)

Fix #4113: Move relPath & newContent checks in writeToFileTool earlier and run after they exist or block is non-partial
Add tests covering the issue.
This commit is contained in:
Ruakij 2025-06-12 17:48:14 +02:00 committed by GitHub
parent 91a477d831
commit 8f58737550
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 46 additions and 17 deletions

View file

@ -399,4 +399,33 @@ describe("writeToFileTool", () => {
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
})
})
describe("parameter validation", () => {
it("errors and resets on missing path parameter", async () => {
await executeWriteFileTool({ path: undefined })
expect(mockCline.consecutiveMistakeCount).toBe(1)
expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file")
expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "path")
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
})
it("errors and resets on empty path parameter", async () => {
await executeWriteFileTool({ path: "" })
expect(mockCline.consecutiveMistakeCount).toBe(1)
expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file")
expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "path")
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
})
it("errors and resets on missing content parameter", async () => {
await executeWriteFileTool({ content: undefined })
expect(mockCline.consecutiveMistakeCount).toBe(1)
expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file")
expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "content")
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
})
})
})

View file

@ -26,12 +26,28 @@ export async function writeToFileTool(
let newContent: string | undefined = block.params.content
let predictedLineCount: number | undefined = parseInt(block.params.line_count ?? "0")
if (!relPath || newContent === undefined) {
if (block.partial && (!relPath || newContent === undefined)) {
// checking for newContent ensure relPath is complete
// wait so we can determine if it's a new file or editing an existing file
return
}
if (!relPath) {
cline.consecutiveMistakeCount++
cline.recordToolError("write_to_file")
pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "path"))
await cline.diffViewProvider.reset()
return
}
if (newContent === undefined) {
cline.consecutiveMistakeCount++
cline.recordToolError("write_to_file")
pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "content"))
await cline.diffViewProvider.reset()
return
}
const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath)
if (!accessAllowed) {
@ -96,22 +112,6 @@ export async function writeToFileTool(
return
} else {
if (!relPath) {
cline.consecutiveMistakeCount++
cline.recordToolError("write_to_file")
pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "path"))
await cline.diffViewProvider.reset()
return
}
if (newContent === undefined) {
cline.consecutiveMistakeCount++
cline.recordToolError("write_to_file")
pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "content"))
await cline.diffViewProvider.reset()
return
}
if (predictedLineCount === undefined) {
cline.consecutiveMistakeCount++
cline.recordToolError("write_to_file")