fix: DiffViewProvider line boundary validation and trailing newline preservation (#8651)

* fix: DiffViewProvider line boundary validation and content concatenation

Two bugs in DiffViewProvider caused file editing failures:

1. **Line boundary validation errors (#8423, #8429)**
   JetBrains hosts using gRPC strictly validate line numbers. When
   truncateDocument() was called with a line number >= document line count,
   it caused "truncateDocument INTERNAL: Wrong line" errors. This occurred
   when new content had >= lines than the original, making truncation
   unnecessary but still attempted.

2. **Content concatenation on final update**
   When replacing content without a trailing newline, the old content at
   line N+1 was concatenated to the new content. For example, writing
   "Hello World" to a file containing "line1\nline2\n" resulted in
   "Hello Worldline2" instead of just "Hello World".

1. Added `getDocumentLineCount()` abstract method to all DiffViewProvider
   implementations to query the current document line count.

2. Added `safelyTruncateDocument()` private helper that validates line
   numbers before calling truncateDocument():
   ```typescript
   private async safelyTruncateDocument(lineNumber: number): Promise<void> {
     const lineCount = await this.getDocumentLineCount()
     if (lineNumber < lineCount) {
       await this.truncateDocument(lineNumber)
     }
   }
   ```

3. Extended the replacement range on final update to cover the entire
   document, preventing content concatenation:
   ```typescript
   const endLine = isFinal
     ? await this.getDocumentLineCount()
     : currentLine + 1
   ```

- src/integrations/editor/DiffViewProvider.ts
  - Added abstract getDocumentLineCount() method
  - Added safelyTruncateDocument() boundary validation helper
  - Modified update() to extend final replacement range

- src/hosts/vscode/VscodeDiffViewProvider.ts
  - Implemented getDocumentLineCount() using editor.document.lineCount

- src/hosts/external/ExternalDiffviewProvider.ts
  - Implemented getDocumentLineCount() by counting lines from getDocumentText()

- src/integrations/editor/FileEditProvider.ts
  - Implemented getDocumentLineCount() from documentContent

- src/integrations/editor/__tests__/DiffViewProvider.test.ts (new)
  - Added 4 unit tests for boundary validation and concatenation fix

Fixes #8423, #8429

* fix: preserve trailing newlines in file edits

Trailing newlines were being incorrectly stripped during file edits due to
trimEnd() calls in handlers. This caused files to lose their final newline
even when the original file had one.

Changes:
- Remove trimEnd() from WriteToFileToolHandler and ApplyPatchHandler that
  was stripping trailing newlines before content reached the editor
- Remove dead code in DiffViewProvider.update() that tried to restore
  newlines after the document was already written
- Add trailing newline fix-up in VscodeDiffViewProvider to handle VS Code's
  applyEdit sometimes normalizing newlines on full-document replacements
- Fix FileEditProvider.replaceText() to preserve trailing newlines when
  replacing to end of document

* fix: preserve trailing newlines in diff text ops

Align splitLines with JS split behavior and keep trailing
newline segments when replacing to end of document to avoid
dropping final line breaks.

.changeset/fix-diff-boundary-validation.md

@@ -0,0 +1,9 @@
+---
+"claude-dev": patch
+---
+
+Fix two bugs in DiffViewProvider file editing:
+
+1. **Line boundary validation**: Add `safelyTruncateDocument()` to prevent out-of-bounds line errors on JetBrains hosts (fixes #8423, #8429). The gRPC protocol strictly validates line numbers, causing "truncateDocument INTERNAL: Wrong line" errors when `truncateDocument()` was called with a line number >= document line count.
+
+2. **Content concatenation on final update**: When replacing content without a trailing newline, the old content at line N+1 was concatenated to the new content. Fixed by extending the replacement range to cover the entire document on final update.

cli/pkg/hostbridge/diff.go

@@ -47,7 +47,10 @@ func (s *DiffService) generateDiffID() string {
 	return fmt.Sprintf("diff_%d_%d", os.Getpid(), id)
 }
 
-// splitLines splits content into lines, preserving line ending information
+// splitLines splits content into lines, preserving trailing newlines.
+// This matches the behavior of JavaScript's String.split("\n"):
+// - "hello\nworld\n" -> ["hello", "world", ""]
+// - "hello\nworld" -> ["hello", "world"]
 func splitLines(content string) []string {
 	if content == "" {
 		return []string{}
@@ -65,10 +68,9 @@ func splitLines(content string) []string {
 		}
 	}
 
-	// Add the last line if it doesn't end with newline
-	if current != "" {
-		lines = append(lines, current)
-	}
+	// Always add the last segment - if content ends with newline, this will be
+	// an empty string which preserves the trailing newline when joined back
+	lines = append(lines, current)
 
 	return lines
 }
@@ -176,9 +178,19 @@ func (s *DiffService) ReplaceText(ctx context.Context, req *proto.ReplaceTextReq
 		endLine = startLine
 	}
 
+	// Check if we're replacing to the end of the document
+	replacingToEnd := endLine >= len(session.lines)
+
 	// Split new content into lines
 	newLines := splitLines(newContent)
 
+	// Remove trailing empty line for proper splicing, BUT only when NOT replacing
+	// to the end of the document. When replacing to the end, keep the trailing
+	// empty string to preserve trailing newlines from the content.
+	if !replacingToEnd && len(newLines) > 0 && newLines[len(newLines)-1] == "" {
+		newLines = newLines[:len(newLines)-1]
+	}
+
 	// Ensure we have enough lines in the current content
 	for len(session.lines) < endLine {
 		session.lines = append(session.lines, "")

src/core/task/tools/handlers/ApplyPatchHandler.ts

@@ -529,7 +529,7 @@ export class ApplyPatchHandler implements IFullyManagedTool {
 					changes[path] = {
 						type: PatchActionType.UPDATE,
 						oldContent: originalFiles[path],
-						newContent: this.applyChunks(originalFiles[path]!, action.chunks, path).trimEnd(),
+						newContent: this.applyChunks(originalFiles[path]!, action.chunks, path),
 						movePath: action.movePath,
 					}
 					break

src/core/task/tools/handlers/WriteToFileToolHandler.ts

@@ -481,8 +481,6 @@ export class WriteToFileToolHandler implements IFullyManagedTool {
 			return
 		}
 
-		newContent = newContent.trimEnd() // remove any trailing newlines, since it's automatically inserted by the editor
-
 		return { relPath, absolutePath, fileExists, diff, content, newContent, workspaceContext }
 	}
 

src/hosts/external/ExternalDiffviewProvider.ts

@@ -42,6 +42,18 @@ export class ExternalDiffViewProvider extends DiffViewProvider {
 		})
 	}
 
+	protected override async getDocumentLineCount(): Promise<number> {
+		const text = await this.getDocumentText()
+		if (!text) {
+			return 0
+		}
+		// Count lines: split by newline, but handle trailing newline correctly
+		const lines = text.split("\n")
+		// If text ends with newline, split creates an extra empty string at the end
+		// which represents the "line" after the final newline - this is correct line count
+		return lines.length
+	}
+
 	protected async saveDocument(): Promise<Boolean> {
 		if (!this.activeDiffEditorId) {
 			return false

src/hosts/vscode/VscodeDiffViewProvider.ts

@@ -102,11 +102,31 @@ export class VscodeDiffViewProvider extends DiffViewProvider {
 
 		// Replace the text in the diff editor document.
 		const document = this.activeDiffEditor?.document
+		const replacingToEnd = rangeToReplace.endLine >= document.lineCount
 		const edit = new vscode.WorkspaceEdit()
 		const range = new vscode.Range(rangeToReplace.startLine, 0, rangeToReplace.endLine, 0)
 		edit.replace(document.uri, range, content)
 		await vscode.workspace.applyEdit(edit)
 
+		// VS Code can normalize trailing newlines on full-document replacements.
+		// Only fix up when replacing to the end to avoid touching untouched content.
+		if (replacingToEnd) {
+			const desiredTrailingNewlines = countTrailingNewlines(content)
+			const actualTrailingNewlines = countTrailingNewlines(document.getText())
+			const newlineDelta = desiredTrailingNewlines - actualTrailingNewlines
+
+			if (newlineDelta > 0) {
+				const fixEdit = new vscode.WorkspaceEdit()
+				fixEdit.insert(document.uri, document.lineAt(document.lineCount - 1).range.end, "\n".repeat(newlineDelta))
+				await vscode.workspace.applyEdit(fixEdit)
+			} else if (newlineDelta < 0) {
+				const fixEdit = new vscode.WorkspaceEdit()
+				const startLine = Math.max(0, document.lineCount + newlineDelta)
+				fixEdit.delete(document.uri, new vscode.Range(startLine, 0, document.lineCount, 0))
+				await vscode.workspace.applyEdit(fixEdit)
+			}
+		}
+
 		if (currentLine !== undefined) {
 			// Update decorations for the entire changed section
 			this.activeLineController?.setActiveLine(currentLine)
@@ -152,6 +172,10 @@ export class VscodeDiffViewProvider extends DiffViewProvider {
 		this.activeLineController?.clear()
 	}
 
+	protected override async getDocumentLineCount(): Promise<number> {
+		return this.activeDiffEditor?.document.lineCount ?? 0
+	}
+
 	protected override async getDocumentText(): Promise<string | undefined> {
 		if (!this.activeDiffEditor || !this.activeDiffEditor.document) {
 			return undefined
@@ -193,3 +217,11 @@ export class VscodeDiffViewProvider extends DiffViewProvider {
 		this.activeLineController = undefined
 	}
 }
+
+function countTrailingNewlines(text: string): number {
+	let count = 0
+	for (let i = text.length - 1; i >= 0 && text[i] === "\n"; i -= 1) {
+		count += 1
+	}
+	return count
+}

src/integrations/editor/DiffViewProvider.ts

@@ -96,6 +96,24 @@ export abstract class DiffViewProvider {
 	 */
 	protected abstract truncateDocument(lineNumber: number): Promise<void>
 
+	/**
+	 * Returns the current line count of the document being edited.
+	 * Used for boundary validation before calling truncateDocument.
+	 */
+	protected abstract getDocumentLineCount(): Promise<number>
+
+	/**
+	 * Safely truncates the document, ensuring the line number is within bounds.
+	 * This prevents errors on hosts that strictly validate line numbers (e.g., JetBrains via gRPC).
+	 */
+	private async safelyTruncateDocument(lineNumber: number): Promise<void> {
+		const lineCount = await this.getDocumentLineCount()
+		// Only truncate if there's content beyond the specified line
+		if (lineNumber < lineCount) {
+			await this.truncateDocument(lineNumber)
+		}
+	}
+
 	/**
 	 * Get the contents of the diff editor document.
 	 *
@@ -182,8 +200,20 @@ export abstract class DiffViewProvider {
 			// Replace all content up to the current line with accumulated lines
 			// This is necessary (as compared to inserting one line at a time) to handle cases where html tags
 			// on previous lines are auto closed for example
-			const contentToReplace = accumulatedLines.slice(0, currentLine + 1).join("\n") + "\n"
-			const rangeToReplace = { startLine: 0, endLine: currentLine + 1 }
+			let contentToReplace = accumulatedLines.slice(0, currentLine + 1).join("\n")
+			if (!isFinal) {
+				// During streaming, add trailing newline for cursor positioning
+				contentToReplace += "\n"
+			}
+
+			// For the final update, replace the entire document to prevent concatenation
+			// when content doesn't end with a newline. Without this, replacing lines 0-N
+			// with content lacking a trailing newline causes line N+1's content to be
+			// directly appended to our content (e.g., "Hello World" + "# Old Header" becomes
+			// "Hello World# Old Header").
+			const endLine = isFinal ? await this.getDocumentLineCount() : currentLine + 1
+
+			const rangeToReplace = { startLine: 0, endLine }
 			await this.replaceText(contentToReplace, rangeToReplace, currentLine)
 
 			// Scroll to the actual change location if provided.
@@ -211,16 +241,7 @@ export abstract class DiffViewProvider {
 		this.streamedLines = accumulatedLines
 		if (isFinal) {
 			// Handle any remaining lines if the new content is shorter than the original
-			await this.truncateDocument(this.streamedLines.length)
-
-			// Add empty last line if original content had one
-			const hasEmptyLastLine = this.originalContent?.endsWith("\n")
-			if (hasEmptyLastLine) {
-				const accumulatedLines = accumulatedContent.split("\n")
-				if (accumulatedLines[accumulatedLines.length - 1] !== "") {
-					accumulatedContent += "\n"
-				}
-			}
+			await this.safelyTruncateDocument(this.streamedLines.length)
 		}
 	}
 

src/integrations/editor/FileEditProvider.ts

@@ -44,10 +44,16 @@ export class FileEditProvider extends DiffViewProvider {
 		// Split the document into lines
 		const lines = this.documentContent.split("\n")
 
+		// Check if we're replacing to the end of the document
+		const replacingToEnd = rangeToReplace.endLine >= lines.length
+
 		// Replace the specified range with the new content
 		const newContentLines = content.split("\n")
-		// Remove trailing empty line if present in newContentLines for proper splicing
-		if (newContentLines[newContentLines.length - 1] === "") {
+
+		// Remove trailing empty line for proper splicing, BUT only when NOT replacing
+		// to the end of the document. When replacing to the end, keep the trailing
+		// empty string to preserve trailing newlines from the content.
+		if (!replacingToEnd && newContentLines[newContentLines.length - 1] === "") {
 			newContentLines.pop()
 		}
 
@@ -78,6 +84,13 @@ export class FileEditProvider extends DiffViewProvider {
 		}
 	}
 
+	protected async getDocumentLineCount(): Promise<number> {
+		if (!this.documentContent) {
+			return 0
+		}
+		return this.documentContent.split("\n").length
+	}
+
 	protected async getDocumentText(): Promise<string | undefined> {
 		return this.documentContent
 	}

src/integrations/editor/__tests__/DiffViewProvider.test.ts

@@ -0,0 +1,226 @@
+import * as assert from "assert"
+import { describe, it } from "mocha"
+import { DiffViewProvider } from "../DiffViewProvider"
+
+class TestBoundaryDiffViewProvider extends DiffViewProvider {
+	public documentText: string = ""
+	public truncatedAt: number | undefined
+
+	async openDiffEditor(): Promise<void> {}
+	async scrollEditorToLine(line: number): Promise<void> {}
+	async scrollAnimation(startLine: number, endLine: number): Promise<void> {}
+
+	async truncateDocument(lineNumber: number): Promise<void> {
+		this.truncatedAt = lineNumber
+		const lines = this.documentText.split("\n")
+		if (lineNumber < lines.length) {
+			this.documentText = lines.slice(0, lineNumber).join("\n")
+		}
+	}
+
+	async getDocumentLineCount(): Promise<number> {
+		return this.documentText.split("\n").length
+	}
+
+	async getDocumentText(): Promise<string | undefined> {
+		return this.documentText
+	}
+
+	async saveDocument(): Promise<Boolean> {
+		return true
+	}
+	async closeAllDiffViews(): Promise<void> {}
+	async resetDiffView(): Promise<void> {}
+
+	async replaceText(
+		content: string,
+		rangeToReplace: { startLine: number; endLine: number },
+		currentLine: number | undefined,
+	): Promise<void> {
+		// Minimal implementation for update() to work
+		const lines = this.documentText.split("\n")
+
+		// Check if we're replacing to the end of the document
+		const replacingToEnd = rangeToReplace.endLine >= lines.length
+
+		const newLines = content.split("\n")
+
+		// Remove trailing empty line for proper splicing, BUT only when NOT replacing
+		// to the end of the document. When replacing to the end, keep the trailing
+		// empty string to preserve trailing newlines from the content.
+		if (!replacingToEnd && newLines[newLines.length - 1] === "") {
+			newLines.pop()
+		}
+
+		lines.splice(rangeToReplace.startLine, rangeToReplace.endLine - rangeToReplace.startLine, ...newLines)
+		this.documentText = lines.join("\n")
+	}
+
+	public setup(initialContent: string) {
+		this.isEditing = true
+		this.documentText = initialContent
+		this.originalContent = initialContent
+		this.truncatedAt = undefined
+	}
+}
+
+describe("DiffViewProvider Boundary Validation", () => {
+	it("should replace entire document on final update to prevent concatenation", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		// Start with multi-line content
+		provider.setup("line1\nline2\nline3\n")
+
+		// Update with content that has no trailing newline
+		// This previously caused "Hello World" + "line2" concatenation
+		await provider.update("Hello World", true)
+
+		const result = await provider.getDocumentText()
+		// Should be just "Hello World", not "Hello Worldline2\nline3\n"
+		assert.strictEqual(result, "Hello World")
+	})
+
+	it("safelyTruncateDocument should no-op when lineNumber >= lineCount", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		provider.setup("line1\nline2\nline3")
+		// lineCount is 3
+
+		// Access private method via any cast or just call update which calls it
+		// But update calls it with streamedLines.length.
+		// Let's use update to trigger it.
+
+		// If we update with same content, streamedLines.length will be 3.
+		// safelyTruncateDocument(3) should be called.
+		// 3 >= 3, so it should NOT call truncateDocument.
+
+		await provider.update("line1\nline2\nline3", true)
+
+		assert.strictEqual(provider.truncatedAt, undefined, "Should not have called truncateDocument")
+	})
+
+	it("final update replaces entire document so truncation is no-op", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		provider.setup("line1\nline2\nline3")
+
+		// Update with fewer lines
+		await provider.update("line1\n", true)
+
+		// With the fix, the final update replaces the entire document (0 to lineCount).
+		// So replaceText handles all the content, and truncation becomes unnecessary.
+		// The document should contain just "line1\n" and truncation should NOT be called
+		// because after replaceText, the document already has the correct content.
+
+		// Note: truncation might still be called but should be a no-op since document is already correct
+		assert.strictEqual(provider.documentText, "line1\n")
+	})
+
+	it("update() with shorter content replaces entire document", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		provider.setup("line1\nline2\nline3\nline4")
+
+		// Update with 2 lines
+		await provider.update("line1\nline2", true)
+
+		// With the fix, the final update replaces the entire document (0 to lineCount).
+		// The document should contain just "line1\nline2".
+
+		assert.strictEqual(provider.documentText, "line1\nline2")
+	})
+})
+
+describe("DiffViewProvider Newline Preservation", () => {
+	it("preserves trailing newline when content ends with newline", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		// Original file has trailing newline
+		provider.setup("line1\nline2\n")
+
+		// New content also has trailing newline
+		await provider.update("new1\nnew2\n", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "new1\nnew2\n", "Trailing newline should be preserved")
+		assert.strictEqual(result?.endsWith("\n"), true)
+	})
+
+	it("does not add trailing newline when content does not end with newline", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		// Original file has trailing newline
+		provider.setup("line1\nline2\n")
+
+		// New content does NOT have trailing newline
+		await provider.update("new1\nnew2", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "new1\nnew2", "Should not have trailing newline")
+		assert.strictEqual(result?.endsWith("\n"), false)
+	})
+
+	it("adds trailing newline when content ends with newline but original did not", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		// Original file does NOT have trailing newline
+		provider.setup("line1\nline2")
+
+		// New content has trailing newline
+		await provider.update("new1\nnew2\n", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "new1\nnew2\n", "Should add trailing newline")
+		assert.strictEqual(result?.endsWith("\n"), true)
+	})
+
+	it("preserves no trailing newline when neither original nor new content has one", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		// Original file does NOT have trailing newline
+		provider.setup("line1\nline2")
+
+		// New content also does NOT have trailing newline
+		await provider.update("new1\nnew2", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "new1\nnew2", "Should not have trailing newline")
+		assert.strictEqual(result?.endsWith("\n"), false)
+	})
+
+	it("handles shortening file while preserving trailing newline", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		// Original: 10 lines with trailing newline
+		provider.setup("line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10\n")
+
+		// New: 3 lines with trailing newline
+		await provider.update("line1\nline2\nline3\n", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "line1\nline2\nline3\n", "Should shorten and preserve trailing newline")
+	})
+
+	it("handles lengthening file while preserving trailing newline", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		// Original: 3 lines with trailing newline
+		provider.setup("line1\nline2\nline3\n")
+
+		// New: 5 lines with trailing newline
+		await provider.update("line1\nline2\nline3\nline4\nline5\n", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "line1\nline2\nline3\nline4\nline5\n", "Should lengthen and preserve trailing newline")
+	})
+
+	it("handles single line content with trailing newline", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		provider.setup("old content\n")
+
+		await provider.update("Hello World\n", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "Hello World\n")
+	})
+
+	it("handles single line content without trailing newline", async () => {
+		const provider = new TestBoundaryDiffViewProvider()
+		provider.setup("old content\nline2\n")
+
+		await provider.update("Hello World", true)
+
+		const result = await provider.getDocumentText()
+		assert.strictEqual(result, "Hello World")
+	})
+})