fix: preserve newlines in content parameters and update error handling in writeToFileTool tests

Author: daniel-lxs

src/core/assistant-message/parseAssistantMessage.ts

@@ -24,7 +24,10 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 			const paramClosingTag = `</${currentParamName}>`
 			if (currentParamValue.endsWith(paramClosingTag)) {
 				// End of param value.
-				currentToolUse.params[currentParamName] = currentParamValue.slice(0, -paramClosingTag.length).trim()
+				// Don't trim content parameters to preserve newlines
+				const paramValue = currentParamValue.slice(0, -paramClosingTag.length)
+				currentToolUse.params[currentParamName] =
+					currentParamName === "content" ? paramValue : paramValue.trim()
 				currentParamName = undefined
 				continue
 			} else {
@@ -72,9 +75,8 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 					const contentEndIndex = toolContent.lastIndexOf(contentEndTag)
 
 					if (contentStartIndex !== -1 && contentEndIndex !== -1 && contentEndIndex > contentStartIndex) {
-						currentToolUse.params[contentParamName] = toolContent
-							.slice(contentStartIndex, contentEndIndex)
-							.trim()
+						// Don't trim content to preserve newlines
+						currentToolUse.params[contentParamName] = toolContent.slice(contentStartIndex, contentEndIndex)
 					}
 				}
 
@@ -138,7 +140,9 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 		// Stream did not complete tool call, add it as partial.
 		if (currentParamName) {
 			// Tool call has a parameter that was not completed.
-			currentToolUse.params[currentParamName] = accumulator.slice(currentParamValueStartIndex).trim()
+			// Don't trim content parameters to preserve newlines
+			const paramValue = accumulator.slice(currentParamValueStartIndex)
+			currentToolUse.params[currentParamName] = currentParamName === "content" ? paramValue : paramValue.trim()
 		}
 
 		contentBlocks.push(currentToolUse)

src/core/assistant-message/parseAssistantMessageV2.ts

@@ -76,13 +76,12 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 				)
 			) {
 				// Found the closing tag for the parameter.
-				const value = assistantMessage
-					.slice(
-						currentParamValueStart, // Start after the opening tag.
-						currentCharIndex - closeTag.length + 1, // End before the closing tag.
-					)
-					.trim()
-				currentToolUse.params[currentParamName] = value
+				const value = assistantMessage.slice(
+					currentParamValueStart, // Start after the opening tag.
+					currentCharIndex - closeTag.length + 1, // End before the closing tag.
+				)
+				// Don't trim content parameters to preserve newlines
+				currentToolUse.params[currentParamName] = currentParamName === "content" ? value : value.trim()
 				currentParamName = undefined // Go back to parsing tool content.
 				// We don't continue loop here, need to check for tool close or other params at index i.
 			} else {
@@ -146,10 +145,8 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 					const contentEnd = toolContentSlice.lastIndexOf(contentEndTag)
 
 					if (contentStart !== -1 && contentEnd !== -1 && contentEnd > contentStart) {
-						const contentValue = toolContentSlice
-							.slice(contentStart + contentStartTag.length, contentEnd)
-							.trim()
-
+						// Don't trim content to preserve newlines
+						const contentValue = toolContentSlice.slice(contentStart + contentStartTag.length, contentEnd)
 						currentToolUse.params[contentParamName] = contentValue
 					}
 				}
@@ -251,9 +248,9 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 
 	// Finalize any open parameter within an open tool use.
 	if (currentToolUse && currentParamName) {
-		currentToolUse.params[currentParamName] = assistantMessage
-			.slice(currentParamValueStart) // From param start to end of string.
-			.trim()
+		const value = assistantMessage.slice(currentParamValueStart) // From param start to end of string.
+		// Don't trim content parameters to preserve newlines
+		currentToolUse.params[currentParamName] = currentParamName === "content" ? value : value.trim()
 		// Tool use remains partial.
 	}
 

src/core/tools/__tests__/writeToFileTool.test.ts

@@ -401,37 +401,31 @@ describe("writeToFileTool", () => {
 	})
 
 	describe("parameter validation", () => {
-		it("returns early without error on missing path parameter", async () => {
+		it("errors and resets on missing path parameter", async () => {
 			await executeWriteFileTool({ path: undefined })
 
-			// With the new behavior, it should return early without errors
-			expect(mockCline.consecutiveMistakeCount).toBe(0)
-			expect(mockCline.recordToolError).not.toHaveBeenCalled()
-			expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled()
-			expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled()
-			expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled()
+			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("returns early without error on empty path parameter", async () => {
+		it("errors and resets on empty path parameter", async () => {
 			await executeWriteFileTool({ path: "" })
 
-			// Empty string is falsy in the context of !relPath check, so it returns early
-			expect(mockCline.consecutiveMistakeCount).toBe(0)
-			expect(mockCline.recordToolError).not.toHaveBeenCalled()
-			expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled()
-			expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled()
-			expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled()
+			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("returns early without error on missing content parameter", async () => {
+		it("errors and resets on missing content parameter", async () => {
 			await executeWriteFileTool({ content: undefined })
 
-			// With the new behavior, it should return early without errors
-			expect(mockCline.consecutiveMistakeCount).toBe(0)
-			expect(mockCline.recordToolError).not.toHaveBeenCalled()
-			expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled()
-			expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled()
-			expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled()
+			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()
 		})
 	})
 })

src/core/tools/writeToFileTool.ts

@@ -8,7 +8,7 @@ import { formatResponse } from "../prompts/responses"
 import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools"
 import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
 import { fileExistsAtPath } from "../../utils/fs"
-import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
+import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
 import { getReadablePath } from "../../utils/path"
 import { isPathOutsideWorkspace } from "../../utils/pathUtils"
 import { detectCodeOmission } from "../../integrations/editor/detect-omission"
@@ -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) {
@@ -100,23 +116,7 @@ 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) {
+			if (predictedLineCount === undefined) {
 				cline.consecutiveMistakeCount++
 				cline.recordToolError("write_to_file")
 
@@ -212,7 +212,8 @@ export async function writeToFileTool(
 				return
 			}
 
-			const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
+			// Call saveChanges to update the DiffViewProvider properties
+			await cline.diffViewProvider.saveChanges()
 
 			// Track file edit operation
 			if (relPath) {
@@ -221,31 +222,10 @@ export async function writeToFileTool(
 
 			cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request
 
-			if (userEdits) {
-				await cline.say(
-					"user_feedback_diff",
-					JSON.stringify({
-						tool: fileExists ? "editedExistingFile" : "newFileCreated",
-						path: getReadablePath(cline.cwd, relPath),
-						diff: userEdits,
-					} satisfies ClineSayTool),
-				)
+			// Get the formatted response message
+			const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
 
-				pushToolResult(
-					`The user made the following updates to your content:\n\n${userEdits}\n\n` +
-						`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
-						`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
-							finalContent || "",
-						)}\n</final_file_content>\n\n` +
-						`Please note:\n` +
-						`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
-						`2. Proceed with the task using this updated file content as the new baseline.\n` +
-						`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
-						`${newProblemsMessage}`,
-				)
-			} else {
-				pushToolResult(`The content was successfully saved to ${relPath.toPosix()}.${newProblemsMessage}`)
-			}
+			pushToolResult(message)
 
 			await cline.diffViewProvider.reset()
 

src/integrations/editor/DiffViewProvider.ts

@@ -129,7 +129,8 @@ export class DiffViewProvider {
 		// Replace all content up to the current line with accumulated lines.
 		const edit = new vscode.WorkspaceEdit()
 		const rangeToReplace = new vscode.Range(0, 0, endLine, 0)
-		const contentToReplace = accumulatedLines.slice(0, endLine + 1).join("\n") + "\n"
+		const contentToReplace =
+			accumulatedLines.slice(0, endLine).join("\n") + (accumulatedLines.length > 0 ? "\n" : "")
 		edit.replace(document.uri, rangeToReplace, this.stripAllBOMs(contentToReplace))
 		await vscode.workspace.applyEdit(edit)
 		// Update decorations.
@@ -229,12 +230,11 @@ export class DiffViewProvider {
 		// show a diff with all the EOL differences.
 		const newContentEOL = this.newContent.includes("\r\n") ? "\r\n" : "\n"
 
-		// `trimEnd` to fix issue where editor adds in extra new line
-		// automatically.
-		const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL
+		// Normalize EOL characters without trimming content
+		const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL)
 
 		// Just in case the new content has a mix of varying EOL characters.
-		const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL
+		const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL)
 
 		if (normalizedEditedContent !== normalizedNewContent) {
 			// User made changes before approving edit.