From 37fb18d0c21f4ec7c3e3fc148fed1c4a9289e9e2 Mon Sep 17 00:00:00 2001 From: Ruakij Date: Thu, 5 Jun 2025 12:26:57 +0200 Subject: [PATCH] Fix #4113: Move relPath & newContent checks in writeToFileTool earlier and run after they exist or block is non-partial Add tests covering the issue. --- .../tools/__tests__/writeToFileTool.test.ts | 29 ++++++++++++++++ src/core/tools/writeToFileTool.ts | 34 +++++++++---------- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/src/core/tools/__tests__/writeToFileTool.test.ts b/src/core/tools/__tests__/writeToFileTool.test.ts index 7df1ee3eb6..e0789f766c 100644 --- a/src/core/tools/__tests__/writeToFileTool.test.ts +++ b/src/core/tools/__tests__/writeToFileTool.test.ts @@ -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() + }) + }) }) diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index 63191acb7e..f7543d6d8b 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -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")