fix: prune stale watcher offset entries - #597
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_53328ed5-7161-4288-ae24-6b57a35729a5) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
📝 WalkthroughWalkthroughThe offset registry now supports tombstones, locked merge-based persistence, atomic writes, and safe pruning of missing files. ChangesOffset Registry and Watcher Pruning
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant JSONLWatcher
participant OffsetRegistry
participant Filesystem
JSONLWatcher->>OffsetRegistry: prune_missing_files(active_roots, active_files)
OffsetRegistry->>Filesystem: inspect tracked paths and root targets
Filesystem-->>OffsetRegistry: availability and stat results
OffsetRegistry-->>JSONLWatcher: prune count and completion status
JSONLWatcher->>OffsetRegistry: flush registry changes
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
@codex review |
|
@cursor review |
|
@BugBot review |
|
@coderabbitai review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5d208530-2a15-477b-aa07-b37fae8b0cc8) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_dd343b91-8061-427e-80f3-c7c02c2f905a) |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c78dbd458
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f2a89304-ddf6-4942-8102-1e476e68525a) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a7c629dbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@cursor review |
|
@BugBot review |
|
@coderabbitai review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bca2205d-4d2f-4326-a279-0b50f4dc4baf) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9a341851-d6ec-4f01-8887-caa12af905ce) |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a7c629dbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1a5fb71d-85bf-4c49-aa5b-8b2172a3545a) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e7a4d7e1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_59249ec5-2f74-4fc1-908a-3a69cf206edc) |
|
Final identity hardening at exact head 931ea24: after a process loads a tombstone, set() now requires a positive live inode before advancing its generation or clearing the tombstone. This prevents a second callback with inode 0 from resurrecting an unavailable file while preserving the same-inode and new-inode recreation paths. Added RED-to-GREEN regression; focused watcher and bridge suite 119/119; a fresh full committed-tree gate is green. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@codex review |
|
@coderabbitai review |
|
@cursor review |
|
@BugBot review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_413bc189-9520-4ed8-a430-deffb82df0cf) |
✅ Action performedReview finished.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2940cd73-c694-4ce5-abd3-b0c077055954) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 931ea2488e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_594f95ff-4ad1-4600-bfa5-858d9d8475e9) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Exact head a8d2bcd closes the final two review findings: tombstones are generation-authoritative across inode history, and OffsetRegistry uses real POSIX or Windows advisory locking. Fresh committed-tree verification: 3,566 passed, 9 skipped, 61 deselected, 1 expected xfail; MCP 3/3; isolated eval and hooks 40/40; Bun 1/1; FTS shell pass; focused watcher and bridge 121/121; Ruff, format, and diff clean. |
|
@codex review |
|
@coderabbitai review |
|
@cursor review |
|
@BugBot review |
✅ Action performedReview finished.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_edba4aa7-586a-4492-9f59-352d4b756d4d) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_87ea57b8-7922-481b-abe5-f46c2e6efa81) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8d2bcd381
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
MERGE-READY on frozen exact head |
|
layerSpec merge-gate v3: PASS — merging a8d2bcd. Verified independently (not relayed): all CI green (3.11/3.12/3.13 + lint/changes/swift + CodeRabbit PASS; Bugbot quota-disclosed; Macroscope neutral); 24 review threads, 0 unresolved (GraphQL-confirmed); every reviewer accounted for on this exact head; head frozen (my convergence guidance stopped the 6-round churn so CI could complete). The two late findings correctly dispositioned WITHOUT re-push: Windows dir-fsync portability = NOT_APPLICABLE (Mac+Linux fleet, no Windows target — verdict-class correct); oversized-tombstone-integer = deferred to the #596/#598 follow-up with malformed-entry sanitization. 6 adversarial rounds closed real data-safety holes on the prune path (unmount-prune of live sessions, tombstone-resurrection, inode-history, delayed-set, inode-0). BLOCKED = branch-protection-needs-authority-approval; merge authority is mine. Data-safe. 🤖 Generated with Claude Code |
Summary
Verification
Risk
This changes registry maintenance only. It never deletes transcript JSONLs or indexed chunks.
No merge requested; ready for owner review.
Note
Medium Risk
Changes durable offset persistence, concurrent merge rules, and fail-closed prune logic; bugs could retain stale entries or incorrectly drop offsets and restart tailing from byte zero, but scope stays limited to the offset registry.
Overview
Hardens #597 offset pruning so stale transcript offsets are dropped only when the watcher has positive evidence the containing subtree is available—not merely because a sibling file exists elsewhere in the watch root.
Prune safety:
prune_missing_filesnow requires the most-specific watch root to look mounted, no broken/unavailable symlinks on the path (including when the tracked path is a symlink), a live JSONL in the candidate’s parent directory from the current poll, and a final missing-file check without transientOSError. Uncertain cases keep the offset and setlast_prune_completefalse soJSONLWatcherretries pruning across polls.Registry integrity: Deletes record generation-aware tombstones so stale in-memory writers cannot resurrect pruned paths; tombstones are sanitized on load/flush and compacted after retention.
flush()merges only dirty paths under an exclusive file lock, refuses to overwrite a corrupt on-disk registry, and persists via temp file +fsync(file and parent dir) +os.replace.mark_rewindbumps generation so confirmed checkpoint rewinds can persist lower offsets without losing to stale flushes.Tests & CI: Large expansion of
test_jsonl_watcher.py(mount/symlink/stat-error cases, concurrency, tombstones, fsync, first-poll prune).run_tests.shmapswatcher.pyto targeted watcher tests in changed-only pre-push. Design/plan docs added underdocs/plans/.Reviewed by Cursor Bugbot for commit a8d2bcd. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Prune stale offset entries from
OffsetRegistrywith fail-closed safety checksOffsetRegistrygains crash-durable writes (fsync + atomicos.replace), cross-platform file locking, and tombstone tracking to prevent stale writers from resurrecting deleted paths.prune_missing_filesremoves only entries confirmed missing via live-evidence checks on watch roots and parent directories; setslast_prune_complete=Falsewhen evidence is insufficient or errors occur.JSONLWatcher.poll_onceruns pruning on first poll, retries untilprune_missing_filescompletes safely, and callsregistry.mark_rewindon checkpoint restores to allow lower confirmed offsets..lockfile on every write; on Windows this uses a retry loop viamsvcrt.locking.Macroscope summarized a8d2bcd.
Summary by CodeRabbit
Bug Fixes
Tests