Skip to content

fix(filesystem): ignore final newline in tail output#4175

Open
Sean-Kenneth-Doherty wants to merge 1 commit into
modelcontextprotocol:mainfrom
Sean-Kenneth-Doherty:codex/filesystem-tail-trailing-newline
Open

fix(filesystem): ignore final newline in tail output#4175
Sean-Kenneth-Doherty wants to merge 1 commit into
modelcontextprotocol:mainfrom
Sean-Kenneth-Doherty:codex/filesystem-tail-trailing-newline

Conversation

@Sean-Kenneth-Doherty
Copy link
Copy Markdown

@Sean-Kenneth-Doherty Sean-Kenneth-Doherty commented May 16, 2026

Summary

  • Fix tailFile so a normal final newline is not returned as an empty trailing line.
  • Add regression coverage that asserts tailFile content instead of only checking handle cleanup.
  • Keep intentional blank lines intact by removing only the split sentinel from the EOF chunk.

Root Cause

tailFile splits the final chunk on \n. For text files ending in a newline, split("\n") produces a final empty sentinel, which the reverse reader counted as the last line. That made read_text_file with tail: 1 return an empty string for ordinary POSIX-style files like line1\nline2\nline3\n.

Validation

  • npm exec -w src/filesystem -- vitest run __tests__/lib.test.ts
  • npm run build -w src/filesystem

@Sean-Kenneth-Doherty Sean-Kenneth-Doherty marked this pull request as ready for review May 16, 2026 21:17
@Sean-Kenneth-Doherty
Copy link
Copy Markdown
Author

Marked ready after rechecking the patch locally.

Why this is ready:

  • the fix is scoped to tailFile only
  • it preserves intentionally blank lines and removes only the EOF split sentinel from the final chunk
  • regression tests cover tail: 1 and tail: 2 on ordinary newline-terminated text

Validation:

  • npm exec -w src/filesystem -- vitest run __tests__/lib.test.ts - 46 passed
  • npm run build -w src/filesystem - passed
  • hosted Python/TypeScript test and build checks are passing

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