Explorar el Código

fix: prevent write_to_file from creating files at truncated paths (#10415)

Co-authored-by: daniel-lxs <[email protected]>
Co-authored-by: Roo Code <[email protected]>
Matt Rubens hace 11 horas
padre
commit
dea33f978c

+ 20 - 1
src/core/tools/WriteToFileTool.ts

@@ -187,6 +187,7 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
 			pushToolResult(message)
 
 			await task.diffViewProvider.reset()
+			this.resetPartialState()
 
 			task.processQueuedMessages()
 
@@ -194,15 +195,26 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
 		} catch (error) {
 			await handleError("writing file", error as Error)
 			await task.diffViewProvider.reset()
+			this.resetPartialState()
 			return
 		}
 	}
 
+	// Track the last seen path during streaming to detect when the path has stabilized
+	private lastSeenPartialPath: string | undefined = undefined
+
 	override async handlePartial(task: Task, block: ToolUse<"write_to_file">): Promise<void> {
 		const relPath: string | undefined = block.params.path
 		let newContent: string | undefined = block.params.content
 
-		if (!relPath || newContent === undefined) {
+		// During streaming, the partial-json library may return truncated string values
+		// when chunk boundaries fall mid-value. To avoid creating files at incorrect paths,
+		// we wait until the path stops changing between consecutive partial blocks before
+		// creating the file. This ensures we have the complete, final path value.
+		const pathHasStabilized = this.lastSeenPartialPath !== undefined && this.lastSeenPartialPath === relPath
+		this.lastSeenPartialPath = relPath
+
+		if (!pathHasStabilized || !relPath || newContent === undefined) {
 			return
 		}
 
@@ -259,6 +271,13 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
 			)
 		}
 	}
+
+	/**
+	 * Reset state when the tool finishes (called from execute or on error)
+	 */
+	resetPartialState(): void {
+		this.lastSeenPartialPath = undefined
+	}
 }
 
 export const writeToFileTool = new WriteToFileTool()

+ 17 - 3
src/core/tools/__tests__/writeToFileTool.spec.ts

@@ -111,6 +111,7 @@ describe("writeToFileTool", () => {
 
 	beforeEach(() => {
 		vi.clearAllMocks()
+		writeToFileTool.resetPartialState()
 
 		mockedPathResolve.mockReturnValue(absoluteFilePath)
 		mockedFileExistsAtPath.mockResolvedValue(false)
@@ -278,10 +279,14 @@ describe("writeToFileTool", () => {
 		)
 
 		it.skipIf(process.platform === "win32")(
-			"creates parent directories early when file does not exist (partial)",
+			"creates parent directories when path has stabilized (partial)",
 			async () => {
+				// First call - path not yet stabilized
 				await executeWriteFileTool({}, { fileExists: false, isPartial: true })
+				expect(mockedCreateDirectoriesForFile).not.toHaveBeenCalled()
 
+				// Second call with same path - path is now stabilized
+				await executeWriteFileTool({}, { fileExists: false, isPartial: true })
 				expect(mockedCreateDirectoriesForFile).toHaveBeenCalledWith(absoluteFilePath)
 			},
 		)
@@ -394,9 +399,14 @@ describe("writeToFileTool", () => {
 			expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled()
 		})
 
-		it("streams content updates during partial execution", async () => {
+		it("streams content updates during partial execution after path stabilizes", async () => {
+			// First call - path not yet stabilized, early return (no file operations)
 			await executeWriteFileTool({}, { isPartial: true })
+			expect(mockCline.ask).not.toHaveBeenCalled()
+			expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled()
 
+			// Second call with same path - path is now stabilized, file operations proceed
+			await executeWriteFileTool({}, { isPartial: true })
 			expect(mockCline.ask).toHaveBeenCalled()
 			expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath)
 			expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, false)
@@ -442,11 +452,15 @@ describe("writeToFileTool", () => {
 			expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
 		})
 
-		it("handles partial streaming errors", async () => {
+		it("handles partial streaming errors after path stabilizes", async () => {
 			mockCline.diffViewProvider.open.mockRejectedValue(new Error("Open failed"))
 
+			// First call - path not yet stabilized, no error yet
 			await executeWriteFileTool({}, { isPartial: true })
+			expect(mockHandleError).not.toHaveBeenCalled()
 
+			// Second call with same path - path is now stabilized, error occurs
+			await executeWriteFileTool({}, { isPartial: true })
 			expect(mockHandleError).toHaveBeenCalledWith("handling partial write_to_file", expect.any(Error))
 		})
 	})