fix(logs): drop stale-stat gate that hid appends on Windows#43
Conversation
The previous fix (#41) switched `-f` follow mode from fs.watchFile to userspace polling, but kept the `fsPromise.stat(file).size > position` gate before reading. On Windows + NTFS, stat() returns a stale size for a short window after another process appends to the file, so the gate evaluates false and the read never fires. Linux/macOS happen to update the size promptly, masking the bug. Drop the gate and read directly from `position` in a loop until read() returns 0 bytes. The read() syscall sees the true file end at call time and doesn't depend on metadata being fresh. Re-opens #40.
📝 WalkthroughWalkthroughUpdated the follow-mode log tailing in ChangesFollow-mode tailing buffer refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/cli/commands/logs.tsOops! Something went wrong! :( ESLint: 8.27.0 Error: ESLint configuration in --config » eslint-config-oclif is invalid:
Referenced from: /.eslintrc Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/commands/logs.ts (1)
211-218: 💤 Low valueConsider using
TextDecoderfor correct multi-byte UTF-8 handling.If a multi-byte UTF-8 character (emoji, non-ASCII path, etc.) spans the 64KB buffer boundary, calling
.toString("utf-8")independently on each chunk produces replacement characters at the split. UsingTextDecoderwith streaming mode handles partial sequences correctly.♻️ Suggested improvement
+ const decoder = new TextDecoder("utf-8", { fatal: false }); const readNewData = async () => { let fd: fsPromise.FileHandle | undefined; try { fd = await fsPromise.open(currentLogFile, "r"); let chunk = ""; while (true) { const { bytesRead } = await fd.read(readBuf, 0, readBuf.length, position); if (bytesRead === 0) break; position += bytesRead; - chunk += readBuf.subarray(0, bytesRead).toString("utf-8"); + chunk += decoder.decode(readBuf.subarray(0, bytesRead), { stream: true }); if (bytesRead < readBuf.length) break; } + chunk += decoder.decode(); // flush any remaining partial sequence if (!chunk) return;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/commands/logs.ts` around lines 211 - 218, The current read loop builds string chunks by calling readBuf.subarray(...).toString("utf-8") which can corrupt multi-byte UTF‑8 sequences split across buffer boundaries; replace the chunk accumulation with a TextDecoder in streaming mode (create new TextDecoder("utf-8") before the loop and call decoder.decode(readBuf.subarray(0, bytesRead), { stream: true }) for each read, then after the loop append decoder.decode(undefined) to flush any remainder), updating the usage around variables fd, readBuf, position, chunk so the final text is correctly decoded without replacement characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/cli/commands/logs.ts`:
- Around line 211-218: The current read loop builds string chunks by calling
readBuf.subarray(...).toString("utf-8") which can corrupt multi-byte UTF‑8
sequences split across buffer boundaries; replace the chunk accumulation with a
TextDecoder in streaming mode (create new TextDecoder("utf-8") before the loop
and call decoder.decode(readBuf.subarray(0, bytesRead), { stream: true }) for
each read, then after the loop append decoder.decode(undefined) to flush any
remainder), updating the usage around variables fd, readBuf, position, chunk so
the final text is correctly decoded without replacement characters.
1 participant
Summary
fs.watchFilewith userspace polling but kept thefsPromise.stat(file).size > positiongate. On Windows + NTFS,stat()returns a stale size for a short window after another process appends, so the gate stays false and the read never fires. Linux/macOS update size promptly, masking the bug — which is why CI passed there but the windows-latest job continued to fail attest/cli/logs.test.ts:462(expected … to contain 'APPENDED_LINE').positionin a loop untilread()returns 0 bytes.read()sees the true file end at syscall time and doesn't depend on metadata being fresh.Test plan
npm run build && npm run build:testnpx vitest run test/cli/logs.test.ts— all 29 tests pass locally on Linux (including thecontinues watching old file if no new file appearsregression case).Summary by CodeRabbit
bitsocial logs --followcommand by optimizing how log files are monitored and tailed. The command now reads log updates more efficiently and better handles incomplete log lines when filtering or streaming output.