feat: preserve EOL in files (#16087)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Adib234 <30782825+Adib234@users.noreply.github.com>
Co-authored-by: Jack Wotherspoon <jackwoth@google.com>

packages/core/src/tools/edit.ts

@@ -6,6 +6,7 @@
 
 import * as fsPromises from 'node:fs/promises';
 import * as path from 'node:path';
+import * as os from 'node:os';
 import * as crypto from 'node:crypto';
 import * as Diff from 'diff';
 import {
@@ -34,7 +35,7 @@ import {
 } from './modifiable-tool.js';
 import { IdeClient } from '../ide/ide-client.js';
 import { FixLLMEditWithInstruction } from '../utils/llm-edit-fixer.js';
-import { safeLiteralReplace } from '../utils/textUtils.js';
+import { safeLiteralReplace, detectLineEnding } from '../utils/textUtils.js';
 import { EditStrategyEvent } from '../telemetry/types.js';
 import { logEditStrategy } from '../telemetry/loggers.js';
 import { EditCorrectionEvent } from '../telemetry/types.js';
@@ -258,17 +259,6 @@ async function calculateRegexReplacement(
   };
 }
 
-/**
- * Detects the line ending style of a string.
- * @param content The string content to analyze.
- * @returns '\r\n' for Windows-style, '\n' for Unix-style.
- */
-function detectLineEnding(content: string): '\r\n' | '\n' {
-  // If a Carriage Return is found, assume Windows-style endings.
-  // This is a simple but effective heuristic.
-  return content.includes('\r\n') ? '\r\n' : '\n';
-}
-
 export async function calculateReplacement(
   config: Config,
   context: ReplacementContext,
@@ -812,9 +802,13 @@ class EditToolInvocation
       await this.ensureParentDirectoriesExistAsync(this.params.file_path);
       let finalContent = editData.newContent;
 
-      // Restore original line endings if they were CRLF
-      if (!editData.isNewFile && editData.originalLineEnding === '\r\n') {
-        finalContent = finalContent.replace(/\n/g, '\r\n');
+      // Restore original line endings if they were CRLF, or use OS default for new files
+      const useCRLF =
+        (!editData.isNewFile && editData.originalLineEnding === '\r\n') ||
+        (editData.isNewFile && os.EOL === '\r\n');
+
+      if (useCRLF) {
+        finalContent = finalContent.replace(/\r?\n/g, '\r\n');
       }
       await this.config
         .getFileSystemService()

packages/core/src/tools/line-endings.test.ts

@@ -0,0 +1,282 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import {
+  describe,
+  it,
+  expect,
+  beforeEach,
+  afterEach,
+  vi,
+  type Mocked,
+} from 'vitest';
+import { detectLineEnding } from '../utils/textUtils.js';
+import { WriteFileTool } from './write-file.js';
+import { EditTool } from './edit.js';
+import type { Config } from '../config/config.js';
+import { ApprovalMode } from '../policy/types.js';
+import { ToolConfirmationOutcome } from './tools.js';
+import type { ToolRegistry } from './tool-registry.js';
+import path from 'node:path';
+import fs from 'node:fs';
+import os from 'node:os';
+import { GeminiClient } from '../core/client.js';
+import type { BaseLlmClient } from '../core/baseLlmClient.js';
+import {
+  ensureCorrectEdit,
+  ensureCorrectFileContent,
+} from '../utils/editCorrector.js';
+import { StandardFileSystemService } from '../services/fileSystemService.js';
+import { WorkspaceContext } from '../utils/workspaceContext.js';
+import {
+  createMockMessageBus,
+  getMockMessageBusInstance,
+} from '../test-utils/mock-message-bus.js';
+
+const rootDir = path.resolve(os.tmpdir(), 'gemini-cli-line-ending-test-root');
+
+// --- MOCKS ---
+vi.mock('../core/client.js');
+vi.mock('../utils/editCorrector.js');
+vi.mock('../ide/ide-client.js', () => ({
+  IdeClient: {
+    getInstance: vi.fn().mockResolvedValue({
+      openDiff: vi.fn(),
+      isDiffingEnabled: vi.fn().mockReturnValue(false),
+    }),
+  },
+}));
+
+let mockGeminiClientInstance: Mocked<GeminiClient>;
+let mockBaseLlmClientInstance: Mocked<BaseLlmClient>;
+const mockEnsureCorrectEdit = vi.fn<typeof ensureCorrectEdit>();
+const mockEnsureCorrectFileContent = vi.fn<typeof ensureCorrectFileContent>();
+
+// Mock Config
+const fsService = new StandardFileSystemService();
+const mockConfigInternal = {
+  getTargetDir: () => rootDir,
+  getApprovalMode: vi.fn(() => ApprovalMode.DEFAULT),
+  setApprovalMode: vi.fn(),
+  getGeminiClient: vi.fn(),
+  getBaseLlmClient: vi.fn(),
+  getFileSystemService: () => fsService,
+  getIdeMode: vi.fn(() => false),
+  getWorkspaceContext: () => new WorkspaceContext(rootDir),
+  getApiKey: () => 'test-key',
+  getModel: () => 'test-model',
+  getSandbox: () => false,
+  getDebugMode: () => false,
+  getQuestion: () => undefined,
+  getToolDiscoveryCommand: () => undefined,
+  getToolCallCommand: () => undefined,
+  getMcpServerCommand: () => undefined,
+  getMcpServers: () => undefined,
+  getUserAgent: () => 'test-agent',
+  getUserMemory: () => '',
+  setUserMemory: vi.fn(),
+  getGeminiMdFileCount: () => 0,
+  setGeminiMdFileCount: vi.fn(),
+  getDisableLLMCorrection: vi.fn(() => false),
+  validatePathAccess: vi.fn().mockReturnValue(null),
+  getToolRegistry: () =>
+    ({
+      registerTool: vi.fn(),
+      discoverTools: vi.fn(),
+    }) as unknown as ToolRegistry,
+  isInteractive: () => false,
+};
+const mockConfig = mockConfigInternal as unknown as Config;
+
+vi.mock('../telemetry/loggers.js', () => ({
+  logFileOperation: vi.fn(),
+  logEditStrategy: vi.fn(),
+  logEditCorrectionEvent: vi.fn(),
+}));
+
+// --- END MOCKS ---
+
+describe('Line Ending Preservation', () => {
+  let tempDir: string;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    tempDir = fs.mkdtempSync(
+      path.join(os.tmpdir(), 'line-ending-test-external-'),
+    );
+    if (!fs.existsSync(rootDir)) {
+      fs.mkdirSync(rootDir, { recursive: true });
+    }
+
+    mockGeminiClientInstance = new (vi.mocked(GeminiClient))(
+      mockConfig,
+    ) as Mocked<GeminiClient>;
+    vi.mocked(GeminiClient).mockImplementation(() => mockGeminiClientInstance);
+
+    mockBaseLlmClientInstance = {
+      generateJson: vi.fn(),
+    } as unknown as Mocked<BaseLlmClient>;
+
+    vi.mocked(ensureCorrectEdit).mockImplementation(mockEnsureCorrectEdit);
+    vi.mocked(ensureCorrectFileContent).mockImplementation(
+      mockEnsureCorrectFileContent,
+    );
+
+    mockConfigInternal.getGeminiClient.mockReturnValue(
+      mockGeminiClientInstance,
+    );
+    mockConfigInternal.getBaseLlmClient.mockReturnValue(
+      mockBaseLlmClientInstance,
+    );
+  });
+
+  afterEach(() => {
+    if (fs.existsSync(tempDir)) {
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    }
+    if (fs.existsSync(rootDir)) {
+      fs.rmSync(rootDir, { recursive: true, force: true });
+    }
+    vi.restoreAllMocks();
+  });
+
+  describe('detectLineEnding', () => {
+    it('should detect CRLF', () => {
+      expect(detectLineEnding('line1\r\nline2')).toBe('\r\n');
+      expect(detectLineEnding('line1\r\n')).toBe('\r\n');
+    });
+
+    it('should detect LF', () => {
+      expect(detectLineEnding('line1\nline2')).toBe('\n');
+      expect(detectLineEnding('line1\n')).toBe('\n');
+      expect(detectLineEnding('line1')).toBe('\n'); // Default to LF if no newline
+    });
+  });
+
+  describe('WriteFileTool', () => {
+    let tool: WriteFileTool;
+    const abortSignal = new AbortController().signal;
+
+    beforeEach(() => {
+      const bus = createMockMessageBus();
+      getMockMessageBusInstance(bus).defaultToolDecision = 'ask_user';
+      tool = new WriteFileTool(mockConfig, bus);
+    });
+
+    it('should preserve CRLF when overwriting an existing file', async () => {
+      const filePath = path.join(rootDir, 'crlf_file.txt');
+      const originalContent = 'line1\r\nline2\r\n';
+      fs.writeFileSync(filePath, originalContent); // Write with CRLF (or however Node writes binary buffer)
+      // Ensure strictly CRLF
+      fs.writeFileSync(filePath, Buffer.from('line1\r\nline2\r\n'));
+
+      // Proposed content from LLM (usually LF)
+      const proposedContent = 'line1\nline2\nline3\n';
+
+      // Mock corrections to return proposed content as-is (but usually normalized)
+      mockEnsureCorrectEdit.mockResolvedValue({
+        params: {
+          file_path: filePath,
+          old_string: originalContent,
+          new_string: proposedContent,
+        },
+        occurrences: 1,
+      });
+
+      const params = { file_path: filePath, content: proposedContent };
+      const invocation = tool.build(params);
+
+      // Force approval
+      const confirmDetails = await invocation.shouldConfirmExecute(abortSignal);
+      if (
+        confirmDetails &&
+        typeof confirmDetails === 'object' &&
+        'onConfirm' in confirmDetails
+      ) {
+        await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce);
+      }
+
+      await invocation.execute(abortSignal);
+
+      const writtenContent = fs.readFileSync(filePath, 'utf8');
+      // Expect all newlines to be CRLF
+      expect(writtenContent).toBe('line1\r\nline2\r\nline3\r\n');
+    });
+
+    it('should use OS EOL for new files', async () => {
+      const filePath = path.join(rootDir, 'new_os_eol_file.txt');
+      const proposedContent = 'line1\nline2\n';
+
+      mockEnsureCorrectFileContent.mockResolvedValue(proposedContent);
+
+      const params = { file_path: filePath, content: proposedContent };
+      const invocation = tool.build(params);
+
+      const confirmDetails = await invocation.shouldConfirmExecute(abortSignal);
+      if (
+        confirmDetails &&
+        typeof confirmDetails === 'object' &&
+        'onConfirm' in confirmDetails
+      ) {
+        await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce);
+      }
+
+      await invocation.execute(abortSignal);
+
+      const writtenContent = fs.readFileSync(filePath, 'utf8');
+
+      if (os.EOL === '\r\n') {
+        expect(writtenContent).toBe('line1\r\nline2\r\n');
+      } else {
+        expect(writtenContent).toBe('line1\nline2\n');
+      }
+    });
+  });
+
+  describe('EditTool', () => {
+    let tool: EditTool;
+    const abortSignal = new AbortController().signal;
+
+    beforeEach(() => {
+      const bus = createMockMessageBus();
+      getMockMessageBusInstance(bus).defaultToolDecision = 'ask_user';
+      tool = new EditTool(mockConfig, bus);
+    });
+
+    it('should preserve CRLF when editing a file', async () => {
+      const filePath = path.join(rootDir, 'edit_crlf.txt');
+      const originalContent = 'line1\r\nline2\r\nline3\r\n';
+      fs.writeFileSync(filePath, Buffer.from(originalContent));
+
+      const oldString = 'line2';
+      const newString = 'modified';
+
+      const params = {
+        file_path: filePath,
+        old_string: oldString,
+        new_string: newString,
+        instruction: 'Change line2 to modified',
+      };
+      const invocation = tool.build(params);
+
+      // Force approval
+      const confirmDetails = await invocation.shouldConfirmExecute(abortSignal);
+      if (
+        confirmDetails &&
+        typeof confirmDetails === 'object' &&
+        'onConfirm' in confirmDetails
+      ) {
+        await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce);
+      }
+
+      await invocation.execute(abortSignal);
+
+      const writtenContent = fs.readFileSync(filePath, 'utf8');
+
+      expect(writtenContent).toBe('line1\r\nmodified\r\nline3\r\n');
+    });
+  });
+});

packages/core/src/tools/write-file.test.ts

@@ -901,6 +901,7 @@ describe('WriteFileTool', () => {
         errorMessage: 'Generic write error',
         expectedMessagePrefix: 'Error writing to file',
         mockFsExistsSync: false,
+        restoreAllMocks: false,
       },
     ])(
       'should return $errorType error when write fails with $errorCode',

packages/core/src/tools/write-file.ts

@@ -7,6 +7,7 @@
 import fs from 'node:fs';
 import fsPromises from 'node:fs/promises';
 import path from 'node:path';
+import os from 'node:os';
 import * as Diff from 'diff';
 import { WRITE_FILE_TOOL_NAME } from './tool-names.js';
 import type { Config } from '../config/config.js';
@@ -33,6 +34,7 @@ import {
   ensureCorrectEdit,
   ensureCorrectFileContent,
 } from '../utils/editCorrector.js';
+import { detectLineEnding } from '../utils/textUtils.js';
 import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js';
 import type {
   ModifiableDeclarativeTool,
@@ -301,9 +303,19 @@ class WriteFileToolInvocation extends BaseToolInvocation<
         await fsPromises.mkdir(dirName, { recursive: true });
       }
 
+      let finalContent = fileContent;
+      const useCRLF =
+        !isNewFile && originalContent
+          ? detectLineEnding(originalContent) === '\r\n'
+          : os.EOL === '\r\n';
+
+      if (useCRLF) {
+        finalContent = finalContent.replace(/\r?\n/g, '\r\n');
+      }
+
       await this.config
         .getFileSystemService()
-        .writeTextFile(this.resolvedPath, fileContent);
+        .writeTextFile(this.resolvedPath, finalContent);
 
       // Generate diff for display result
       const fileName = path.basename(this.resolvedPath);

packages/core/src/utils/editCorrector.test.ts

@@ -639,6 +639,33 @@ describe('editCorrector', () => {
         expect(result.params).toEqual(originalParams);
       });
     });
+
+    describe('Scenario Group 7: Trimming with Newline Preservation', () => {
+      it('Test 7.1: should preserve trailing newlines in new_string when trimming is applied', async () => {
+        const currentContent = '  find me'; // Matches old_string initially
+        const originalParams = {
+          file_path: '/test/file.txt',
+          old_string: '  find me', // Matches, but has whitespace to trim
+          new_string: '  replaced\n\n', // Needs trimming but preserve newlines
+        };
+
+        const result = await ensureCorrectEdit(
+          '/test/file.txt',
+          currentContent,
+          originalParams,
+          mockGeminiClientInstance,
+          mockBaseLlmClientInstance,
+          abortSignal,
+          false,
+        );
+
+        // old_string should be trimmed to 'find me' because 'find me' also exists uniquely in '  find me'
+        expect(result.params.old_string).toBe('find me');
+        // new_string should be trimmed of spaces but keep ALL newlines
+        expect(result.params.new_string).toBe('replaced\n\n');
+        expect(result.occurrences).toBe(1);
+      });
+    });
   });
 
   describe('ensureCorrectFileContent', () => {

packages/core/src/utils/editCorrector.ts

@@ -689,13 +689,20 @@ Return ONLY the corrected string in the specified JSON format with the key 'corr
   }
 }
 
+function trimPreservingTrailingNewline(str: string): string {
+  const trimmedEnd = str.trimEnd();
+  const trailingWhitespace = str.slice(trimmedEnd.length);
+  const trailingNewlines = trailingWhitespace.replace(/[^\r\n]/g, '');
+  return str.trim() + trailingNewlines;
+}
+
 function trimPairIfPossible(
   target: string,
   trimIfTargetTrims: string,
   currentContent: string,
   expectedReplacements: number,
 ) {
-  const trimmedTargetString = target.trim();
+  const trimmedTargetString = trimPreservingTrailingNewline(target);
   if (target.length !== trimmedTargetString.length) {
     const trimmedTargetOccurrences = countOccurrences(
       currentContent,
@@ -703,7 +710,8 @@ function trimPairIfPossible(
     );
 
     if (trimmedTargetOccurrences === expectedReplacements) {
-      const trimmedReactiveString = trimIfTargetTrims.trim();
+      const trimmedReactiveString =
+        trimPreservingTrailingNewline(trimIfTargetTrims);
       return {
         targetString: trimmedTargetString,
         pair: trimmedReactiveString,

packages/core/src/utils/textUtils.ts

@@ -54,6 +54,17 @@ export function isBinary(
   return false;
 }
 
+/**
+ * Detects the line ending style of a string.
+ * @param content The string content to analyze.
+ * @returns '\r\n' for Windows-style, '\n' for Unix-style.
+ */
+export function detectLineEnding(content: string): '\r\n' | '\n' {
+  // If a Carriage Return is found, assume Windows-style endings.
+  // This is a simple but effective heuristic.
+  return content.includes('\r\n') ? '\r\n' : '\n';
+}
+
 /**
  * Truncates a string to a maximum length, appending a suffix if truncated.
  * @param str The string to truncate.