Repository navigation
Never re-append a merged Telemetry file - #26
Conversation
Once a merge's append succeeded, a failed rename to .migrated left the file as .merging and the next launch appended it again. It now falls back to deleting the source, or parking it as .merging-done, which later runs only delete. A directory or link named like a leftover is reported and left alone rather than skipped silently.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit details
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 |
There was a problem hiding this comment.
🟡 Changes recommended
The migration-complete flag prevents parked .merging-done files from receiving the promised later cleanup.
1 open finding
What changed in this PR
Prevents telemetry migration retries from re-appending already merged records.
Changes:
- Adds
.merging-donerecovery state and safer cleanup fallback. - Reports non-file leftovers and adds regression tests.
| File | Description |
|---|---|
TelemetryFolderMigration.swift |
Adds merge completion recovery and leftover validation. |
TelemetryFolderMigrationTests.swift |
Tests failed cleanup and non-file leftovers. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| return entries.contains { entry in | ||
| guard let stage = Self.stage(of: entry), stage != .migrated, | ||
| guard let stage = Self.stage(of: entry), stage == .live || stage == .merging, |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Summary
Review of ClaudeBar's matching change found two gaps in recovering an interrupted Telemetry Folder move, and CodexBar's code from #25 had them too.
.migratedleft the file as.merging, so the next launch appended every line again. It now falls back to deleting the source. If that fails too, it parks the file as.merging-done, which later runs only delete, andhasPendingFiles()doesn't count it.….jsonl.migratedor.merging) was already skipped, just silently. It's now reported and left alone, and nothing under it is removed.If even the rename to
.merging-donefails, the file stays.mergingand a later launch can repeat its lines. Bar UI ignores repeated records, so nothing gets counted twice.Test plan
swift test: 109 tests, 0 failures;swift test --parallelexits 0.migratedrename falls back to deleting the source; a source that can be neither renamed nor deleted is never appended again