Skip to content

Commit 9975cbf

Browse files
committed
fix: address CodeRabbit review comments on CRLF preservation
- detectFileLineEnding: loop 8KB chunks up to 64KB instead of fixed single-buffer read, handling \r at chunk boundaries correctly - read(): restore trailing newline stripped by readline by checking the file's final byte - test teardown: use setValue() to explicitly restore allowedDirectories instead of shallow-merging the full config
1 parent 976516b commit 9975cbf

2 files changed

Lines changed: 69 additions & 6 deletions

File tree

src/utils/files/text.ts

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export class TextFileHandler implements FileHandler {
6060
// readline strips \r\n → \n; we need to restore the original style
6161
// so that downstream consumers (e.g. write_file) preserve line endings.
6262
const lineEnding = await this.detectFileLineEnding(filePath);
63+
const endsWithNewline = await this.fileEndsWithNewline(filePath);
6364

6465
const result = await this.readFileWithSmartPositioning(filePath, offset, length, 'text/plain', includeStatusMessage);
6566

@@ -68,6 +69,11 @@ export class TextFileHandler implements FileHandler {
6869
result.content = normalizeLineEndings(result.content, lineEnding);
6970
}
7071

72+
// Restore trailing newline stripped by readline
73+
if (endsWithNewline && typeof result.content === 'string' && !result.content.endsWith(lineEnding)) {
74+
result.content += lineEnding;
75+
}
76+
7177
return result;
7278
}
7379

@@ -144,15 +150,61 @@ export class TextFileHandler implements FileHandler {
144150
}
145151

146152
/**
147-
* Detect line ending style by reading the first few KB of a file
153+
* Detect line ending style by scanning file chunks until a newline is found.
154+
* Reads in 8KB chunks up to 64KB to handle files whose first line exceeds 8KB.
148155
*/
149156
private async detectFileLineEnding(filePath: string): Promise<LineEndingStyle> {
157+
const CHUNK_SIZE = 8192;
158+
const MAX_SCAN = 65536; // 64KB cap
159+
const fd = await fs.open(filePath, 'r');
160+
try {
161+
const buffer = Buffer.alloc(CHUNK_SIZE);
162+
let position = 0;
163+
let prevEndedWithCR = false;
164+
while (position < MAX_SCAN) {
165+
const { bytesRead } = await fd.read(buffer, 0, CHUNK_SIZE, position);
166+
if (bytesRead === 0) {
167+
return prevEndedWithCR ? '\r' : '\n';
168+
}
169+
const chunk = buffer.toString('utf-8', 0, bytesRead);
170+
// Handle \r at previous chunk boundary followed by \n here
171+
if (prevEndedWithCR) {
172+
return chunk[0] === '\n' ? '\r\n' : '\r';
173+
}
174+
for (let i = 0; i < chunk.length; i++) {
175+
if (chunk[i] === '\r') {
176+
if (i + 1 < chunk.length) {
177+
return chunk[i + 1] === '\n' ? '\r\n' : '\r';
178+
}
179+
// \r at end of chunk — check next chunk for \n
180+
prevEndedWithCR = true;
181+
break;
182+
}
183+
if (chunk[i] === '\n') {
184+
return '\n';
185+
}
186+
}
187+
position += bytesRead;
188+
}
189+
// No newline found within cap — default to LF
190+
return '\n';
191+
} finally {
192+
await fd.close();
193+
}
194+
}
195+
196+
/**
197+
* Check whether the file ends with a newline character (\n or \r).
198+
* Used to restore trailing newlines stripped by readline.
199+
*/
200+
private async fileEndsWithNewline(filePath: string): Promise<boolean> {
201+
const stats = await fs.stat(filePath);
202+
if (stats.size === 0) return false;
150203
const fd = await fs.open(filePath, 'r');
151204
try {
152-
const buffer = Buffer.alloc(8192);
153-
const { bytesRead } = await fd.read(buffer, 0, 8192, 0);
154-
const sample = buffer.toString('utf-8', 0, bytesRead);
155-
return detectLineEnding(sample);
205+
const buf = Buffer.alloc(1);
206+
await fd.read(buf, 0, 1, stats.size - 1);
207+
return buf[0] === 0x0A || buf[0] === 0x0D; // \n or \r
156208
} finally {
157209
await fd.close();
158210
}

test/test-crlf-preservation.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@ async function setup() {
3939
}
4040

4141
async function teardown(originalConfig) {
42-
await configManager.updateConfig(originalConfig);
42+
// Explicitly restore allowedDirectories — shallow merge via updateConfig()
43+
// won't remove keys that were added during setup
44+
await configManager.setValue(
45+
'allowedDirectories',
46+
originalConfig.allowedDirectories ?? []
47+
);
4348
await fs.rm(TEST_DIR, { recursive: true, force: true });
4449
console.log(' Teardown: cleaned up');
4550
}
@@ -196,6 +201,12 @@ async function testReadWriteRoundtrip() {
196201
'Must not contain bare LF after roundtrip'
197202
);
198203

204+
// Verify trailing newline is preserved
205+
assert.ok(
206+
rawAfter.endsWith('\r\n'),
207+
'Trailing CRLF must be preserved after roundtrip'
208+
);
209+
199210
console.log(' PASS');
200211
}
201212

0 commit comments

Comments
 (0)