Skip to content

fix: improve log file position tracking accuracy#164

Merged
jdx merged 1 commit intomainfrom
fix/log-file-performance
Jan 19, 2026
Merged

fix: improve log file position tracking accuracy#164
jdx merged 1 commit intomainfrom
fix/log-file-performance

Conversation

@jdx
Copy link
Owner

@jdx jdx commented Jan 19, 2026

Summary

Fixes the TODO at src/cli/logs.rs:207 about building log file length when reading to avoid gaps.

Issues fixed:

  1. Race condition on initialization: Previously used a separate metadata() call to get file length after opening. Content written between the metadata check and starting to watch would be missed. Now uses seek(End) + stream_position() atomically on the already-opened file.

  2. Position drift after reading: Previously calculated new position by summing line lengths:

    info.cur += lines.iter().fold(0, |acc, l| acc + l.len() as u64);

    This missed newline characters that are stripped by lines(), causing the position to drift behind the actual file position. Now uses stream_position() after reading to get the accurate position.

Test plan

  • All 76 tests pass
  • Clippy passes
  • pitchfork logs --tail works correctly with multiple daemons

🤖 Generated with Claude Code


Note

Improves log tail accuracy and eliminates an init race in src/cli/logs.rs.

  • In get_log_file_infos, open the log file, seek(End), then use stream_position() to set cur, replacing metadata().len() to avoid missing writes between metadata and open
  • In tail_logs, replace manual byte counting of read lines with stream_position() to correctly advance cur (including newline bytes)

Written by Cursor Bugbot for commit 48be4e4. This will update automatically on new commits. Configure here.

Fix two issues in log file tailing:

1. Race condition on initialization: Previously used separate metadata()
   call to get file length, which could miss content written between
   the metadata check and file open. Now uses seek(End) + stream_position()
   atomically on the opened file.

2. Position drift after reading: Previously calculated new position by
   summing line lengths, but this missed newline characters that are
   stripped by lines(). Now uses stream_position() after reading to get
   the accurate file position.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings January 19, 2026 15:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a TODO item by improving log file position tracking to eliminate race conditions and position drift. The changes ensure accurate file position tracking when tailing logs from multiple daemon processes.

Changes:

  • Replaced separate metadata call with atomic seek-to-end and position retrieval to prevent race conditions during initialization
  • Switched from manual line-length calculation to stream_position() after reading to account for stripped newline characters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jdx jdx merged commit 12afca6 into main Jan 19, 2026
4 checks passed
@jdx jdx deleted the fix/log-file-performance branch January 19, 2026 15:33
@jdx jdx mentioned this pull request Jan 19, 2026
jdx added a commit that referenced this pull request Jan 19, 2026
## 🤖 New release

* `pitchfork-cli`: 1.0.2 -> 1.1.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [1.1.0](v1.0.2...v1.1.0) -
2026-01-19

### Added

- add file watching to auto-restart daemons
([#165](#165))
- support boolean values for retry configuration
([#170](#170))
- disable web UI by default
([#172](#172))
- auto-generate JSON schema from Rust types
([#167](#167))

### Fixed

- improve cron watcher granularity for sub-minute schedules
([#163](#163))
- improve log file position tracking accuracy
([#164](#164))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Releases `pitchfork-cli` v1.1.0 and syncs version across `Cargo.toml`,
`Cargo.lock`, `docs/cli/*`, and `pitchfork.usage.kdl`.
> 
> - **Added**: file watching to auto-restart daemons; boolean support
for retry config; web UI disabled by default; JSON schema generation
from Rust types
> - **Fixed**: improved cron watcher granularity (sub-minute); more
accurate log file position tracking
> - Updated `CHANGELOG.md` with 1.1.0 notes
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
2e6d4c6. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

2 participants