From 8f58737550f364c0aabb7858ecb9face6d625460 Mon Sep 17 00:00:00 2001 From: Ruakij Date: Thu, 12 Jun 2025 17:48:14 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20#4113:=20Move=20relPath=20&=20newContent?= =?UTF-8?q?=20checks=20in=20writeToFileTool=20earlie=E2=80=A6=20(#4378)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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")