Skip to content

fix(filesystem): prevent off-by-one line count in tailFile with trailing newlines - #4643

Open
teddiesloco wants to merge 1 commit into
modelcontextprotocol:mainfrom
teddiesloco:fix/tailfile-trailing-newline-off-by-one
Open

fix(filesystem): prevent off-by-one line count in tailFile with trailing newlines#4643
teddiesloco wants to merge 1 commit into
modelcontextprotocol:mainfrom
teddiesloco:fix/tailfile-trailing-newline-off-by-one

Conversation

@teddiesloco

Copy link
Copy Markdown

Summary

When reading the tail of a file that ends with a trailing newline (\n or \r\n), tailFile was returning N-1 lines instead of N lines.

This happens because split('\n') on content ending in \n produces a trailing empty string "". The loop in tailFile counts this empty element toward numLines, resulting in one fewer real line of content being returned. Since most text files on POSIX systems end with a newline, this affects standard usage.

Reproduction

// File content: "line1\nline2\nline3\n" (3 lines)
await tailFile(filePath, 2);
// Before: returns "line3\n" (only 1 line)
// Expected: returns "line2\nline3" (2 lines)

await tailFile(filePath, 1);
// Before: returns "" (empty string!)
// Expected: returns "line3"

Root Cause

  1. The initial chunk read from the end of the file includes the trailing newline.
  2. normalizeLineEndings(chunkText).split('\n') results in ['...', '', ''] or ['...', ''].
  3. The reverse iteration loop for (let i = chunkLines.length - 1; ...) consumes the empty trailing element as a counted line.
  4. Additionally, when a file was read all the way to byte 0 (position === 0), any remaining text from an incomplete line at the start of the file was discarded instead of being prepended to the result.

Fix

  1. On the very first chunk read from the end of the file (isLastChunk), strip the trailing newline after normalizing line endings. Subsequent chunks preserve all newlines.
  2. If position === 0 and there is leftover remainingText and the requested line count hasn't been met, unshift it into the result array.
  3. Behavior now matches standard Unix tail -n N.

Verification

Added 7 regression tests against real filesystem operations covering:

  • Files with trailing newline
  • Files without trailing newline
  • Requested line count exceeding total file lines
  • CRLF (\r\n) line endings with trailing newline
  • Single-line files (with and without trailing newline)
  • Multi-chunk large files spanning across the 1KB buffer boundary

All 159 tests pass across the filesystem server suite (152 existing + 7 new).

…railing newline

When a file ends with a trailing newline (`\n` or `\r\n`), splitting the
last chunk on newline produces an empty trailing element. `tailFile` was
treating this empty element as a valid line, returning N-1 actual lines
instead of N.

This fix strips the trailing newline only from the very first chunk read
(the end of the file) before splitting, matching the behavior of Unix `tail`.
Also handles the edge case where the very first line of the file was previously
dropped if the file was read to the beginning with `remainingText` left.

Adds 7 real-fs regression tests covering trailing/no-trailing newline, CRLF,
single lines, and multi-chunk files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant