diff --git a/src/utils/files/base.ts b/src/utils/files/base.ts index 278a784f..3cda6dbc 100644 --- a/src/utils/files/base.ts +++ b/src/utils/files/base.ts @@ -126,6 +126,9 @@ export interface FileMetadata { /** For text files */ lineCount?: number; + /** Whether the read reached the end of the file */ + reachedEOF?: boolean; + /** For PDF files */ isPdf?: boolean; author?: string; diff --git a/src/utils/files/text.ts b/src/utils/files/text.ts index 52e37bff..c5324178 100644 --- a/src/utils/files/text.ts +++ b/src/utils/files/text.ts @@ -23,6 +23,7 @@ import { FileResult, FileInfo } from './base.js'; +import { detectLineEnding, normalizeLineEndings, type LineEndingStyle } from '../lineEndingHandler.js'; // TODO: Centralize these constants with filesystem.ts to avoid silent drift // These duplicate concepts from filesystem.ts and should be moved to a shared @@ -55,8 +56,31 @@ export class TextFileHandler implements FileHandler { const length = options?.length ?? 1000; // Default from config const includeStatusMessage = options?.includeStatusMessage ?? true; - // Binary detection is done at factory level - just read as text - return this.readFileWithSmartPositioning(filePath, offset, length, 'text/plain', includeStatusMessage); + // Detect original line ending before reading. + // readline strips \r\n → \n; we need to restore the original style + // so that downstream consumers (e.g. write_file) preserve line endings. + const lineEnding = await this.detectFileLineEnding(filePath); + const endsWithNewline = await this.fileEndsWithNewline(filePath); + + const result = await this.readFileWithSmartPositioning(filePath, offset, length, 'text/plain', includeStatusMessage); + + // Restore original line endings if the file uses CRLF or CR + if (lineEnding !== '\n' && typeof result.content === 'string') { + result.content = normalizeLineEndings(result.content, lineEnding); + } + + // Restore trailing newline stripped by readline. + // When we haven't reached EOF, the last line we read definitely had + // a newline after it (there's more content following in the file). + // When we have reached EOF, only restore if the original file ended with one. + if (typeof result.content === 'string' && !result.content.endsWith(lineEnding)) { + const reachedEOF = result.metadata?.reachedEOF !== false; + if (!reachedEOF || endsWithNewline) { + result.content += lineEnding; + } + } + + return result; } async write(path: string, content: string, mode: 'rewrite' | 'append' = 'rewrite'): Promise { @@ -131,6 +155,67 @@ export class TextFileHandler implements FileHandler { return undefined; } + /** + * Detect line ending style by scanning file chunks until a newline is found. + * Reads in 8KB chunks up to 64KB to handle files whose first line exceeds 8KB. + */ + private async detectFileLineEnding(filePath: string): Promise { + const CHUNK_SIZE = 8192; + const MAX_SCAN = 65536; // 64KB cap + const fd = await fs.open(filePath, 'r'); + try { + const buffer = Buffer.alloc(CHUNK_SIZE); + let position = 0; + let prevEndedWithCR = false; + while (position < MAX_SCAN) { + const { bytesRead } = await fd.read(buffer, 0, CHUNK_SIZE, position); + if (bytesRead === 0) { + return prevEndedWithCR ? '\r' : '\n'; + } + const chunk = buffer.toString('utf-8', 0, bytesRead); + // Handle \r at previous chunk boundary followed by \n here + if (prevEndedWithCR) { + return chunk[0] === '\n' ? '\r\n' : '\r'; + } + for (let i = 0; i < chunk.length; i++) { + if (chunk[i] === '\r') { + if (i + 1 < chunk.length) { + return chunk[i + 1] === '\n' ? '\r\n' : '\r'; + } + // \r at end of chunk — check next chunk for \n + prevEndedWithCR = true; + break; + } + if (chunk[i] === '\n') { + return '\n'; + } + } + position += bytesRead; + } + // No newline found within cap — default to LF + return '\n'; + } finally { + await fd.close(); + } + } + + /** + * Check whether the file ends with a newline character (\n or \r). + * Used to restore trailing newlines stripped by readline. + */ + private async fileEndsWithNewline(filePath: string): Promise { + const stats = await fs.stat(filePath); + if (stats.size === 0) return false; + const fd = await fs.open(filePath, 'r'); + try { + const buf = Buffer.alloc(1); + await fd.read(buf, 0, 1, stats.size - 1); + return buf[0] === 0x0A || buf[0] === 0x0D; // \n or \r + } finally { + await fd.close(); + } + } + /** * Generate enhanced status message */ @@ -283,7 +368,7 @@ export class TextFileHandler implements FileHandler { ? `${this.generateEnhancedStatusMessage(result.length, -n, fileTotalLines, true)}\n\n${result.join('\n')}` : result.join('\n'); - return { content, mimeType, metadata: {} }; + return { content, mimeType, metadata: { reachedEOF: true } }; } finally { await fd.close(); } @@ -330,7 +415,7 @@ export class TextFileHandler implements FileHandler { ? `${this.generateEnhancedStatusMessage(result.length, -requestedLines, fileTotalLines, true)}\n\n${result.join('\n')}` : result.join('\n'); - return { content, mimeType, metadata: {} }; + return { content, mimeType, metadata: { reachedEOF: true } }; } /** @@ -351,12 +436,15 @@ export class TextFileHandler implements FileHandler { const result: string[] = []; let lineNumber = 0; + let reachedEOF = true; for await (const line of rl) { if (lineNumber >= offset && result.length < length) { result.push(line); + } else if (result.length >= length) { + reachedEOF = false; + break; } - if (result.length >= length) break; lineNumber++; } @@ -365,10 +453,10 @@ export class TextFileHandler implements FileHandler { if (includeStatusMessage) { const statusMessage = this.generateEnhancedStatusMessage(result.length, offset, fileTotalLines, false); const content = `${statusMessage}\n\n${result.join('\n')}`; - return { content, mimeType, metadata: {} }; + return { content, mimeType, metadata: { reachedEOF } }; } else { const content = result.join('\n'); - return { content, mimeType, metadata: {} }; + return { content, mimeType, metadata: { reachedEOF } }; } } @@ -421,6 +509,7 @@ export class TextFileHandler implements FileHandler { const result: string[] = []; let firstLineSkipped = false; + let reachedEOF = true; for await (const line of rl2) { if (!firstLineSkipped && startPosition > 0) { @@ -431,6 +520,7 @@ export class TextFileHandler implements FileHandler { if (result.length < length) { result.push(line); } else { + reachedEOF = false; break; } } @@ -441,7 +531,7 @@ export class TextFileHandler implements FileHandler { ? `${this.generateEnhancedStatusMessage(result.length, offset, fileTotalLines, false)}\n\n${result.join('\n')}` : result.join('\n'); - return { content, mimeType, metadata: {} }; + return { content, mimeType, metadata: { reachedEOF } }; } finally { await fd.close(); } diff --git a/test/test-crlf-preservation.js b/test/test-crlf-preservation.js new file mode 100644 index 00000000..718bd9cb --- /dev/null +++ b/test/test-crlf-preservation.js @@ -0,0 +1,365 @@ +/** + * Test: read_file and edit_block preserve CRLF line endings on Windows + * + * Verifies fix for https://github.com/wonderwhy-er/DesktopCommanderMCP/issues/97 + * + * The bug: TextFileHandler.read() used readline which stripped \r\n to \n. + * When AI clients received LF content and wrote it back via write_file, + * CRLF files silently lost their line endings, causing phantom git diffs. + */ + +import { configManager } from '../dist/config-manager.js'; +import fs from 'fs/promises'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import assert from 'assert'; +import { handleReadFile } from '../dist/handlers/filesystem-handlers.js'; +import { handleEditBlock } from '../dist/handlers/edit-search-handlers.js'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const TEST_DIR = path.join(__dirname, 'test_crlf_preservation'); +const CRLF_FILE = path.join(TEST_DIR, 'crlf_file.txt'); +const LF_FILE = path.join(TEST_DIR, 'lf_file.txt'); +const CRLF_NO_TRAILING_FILE = path.join(TEST_DIR, 'crlf_no_trailing.txt'); +const LF_NO_TRAILING_FILE = path.join(TEST_DIR, 'lf_no_trailing.txt'); + +// The raw CRLF content (5 lines, each ending with \r\n) +const CRLF_CONTENT = 'Line one\r\nLine two\r\nLine three\r\nLine four\r\nLine five\r\n'; +const LF_CONTENT = 'Line one\nLine two\nLine three\nLine four\nLine five\n'; +// Files without trailing newline (last line has no newline after it) +const CRLF_NO_TRAILING_CONTENT = 'Line one\r\nLine two\r\nLine three\r\nLine four\r\nLine five'; +const LF_NO_TRAILING_CONTENT = 'Line one\nLine two\nLine three\nLine four\nLine five'; + +async function setup() { + await fs.mkdir(TEST_DIR, { recursive: true }); + await fs.writeFile(CRLF_FILE, CRLF_CONTENT, 'utf8'); + await fs.writeFile(LF_FILE, LF_CONTENT, 'utf8'); + await fs.writeFile(CRLF_NO_TRAILING_FILE, CRLF_NO_TRAILING_CONTENT, 'utf8'); + await fs.writeFile(LF_NO_TRAILING_FILE, LF_NO_TRAILING_CONTENT, 'utf8'); + + const originalConfig = await configManager.getConfig(); + await configManager.setValue('allowedDirectories', [TEST_DIR]); + console.log(' Setup: created test files'); + return originalConfig; +} + +async function teardown(originalConfig) { + // Explicitly restore allowedDirectories — shallow merge via updateConfig() + // won't remove keys that were added during setup + await configManager.setValue( + 'allowedDirectories', + originalConfig.allowedDirectories ?? [] + ); + await fs.rm(TEST_DIR, { recursive: true, force: true }); + console.log(' Teardown: cleaned up'); +} + +/** + * Test 1: read_file does not modify the CRLF file on disk + */ +async function testReadFileDoesNotModifyDisk() { + console.log('\n Test 1: read_file does not modify CRLF file on disk'); + + const originalBytes = await fs.readFile(CRLF_FILE); + + await handleReadFile({ path: CRLF_FILE }); + + const afterBytes = await fs.readFile(CRLF_FILE); + assert.ok( + originalBytes.equals(afterBytes), + 'File on disk must be byte-for-byte identical after read_file' + ); + console.log(' PASS'); +} + +/** + * Test 2: read_file returns content with CRLF preserved + */ +async function testReadFilePreservesCRLF() { + console.log('\n Test 2: read_file returns CRLF line endings'); + + const result = await handleReadFile({ path: CRLF_FILE }); + const text = result.content[0].text; + + // The returned text should contain \r\n (after stripping the status header) + // Status header format: "[Reading N lines ...]\r\n\r\n" + const contentStart = text.indexOf('Line one'); + assert.ok(contentStart >= 0, 'Content should include "Line one"'); + + const fileContent = text.substring(contentStart); + assert.ok( + fileContent.includes('\r\n'), + 'Returned content must preserve CRLF line endings' + ); + assert.ok( + !fileContent.match(/(? { + process.exit(success ? 0 : 1); + }).catch(error => { + console.error('Unhandled error:', error); + process.exit(1); + }); +}