Fix checkpoint v2 rotation push handling#1148
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes v2 checkpoint “full generation” rotation push behavior so that locally-rotated generations are correctly published (including archive refs) and rotation conflicts with remote state are handled more robustly during push/recovery.
Changes:
- Adds persistent bookkeeping for pending v2 full-generation rotations and publishes pending rotations before pushing refs.
- Improves rotation-conflict recovery by selecting the related remote archive when the remote has rotated multiple times.
- Expands push behavior and tests to cover multiple local archives (e.g., migration/repeated rotations) and stale remote archive refs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/push_v2.go | Adds pending-rotation publication, improves remote-rotation conflict handling, and adjusts which refs are pushed. |
| cmd/entire/cli/strategy/push_v2_test.go | Adds tests for local rotation publication, repeated rotations, stale archives, and multi-rotation remote conflict scenarios. |
| cmd/entire/cli/strategy/cleanup.go | Switches ls-remote parsing to strings.SplitSeq. |
| cmd/entire/cli/checkpoint/v2_read.go | Switches ls-remote parsing to strings.SplitSeq. |
| cmd/entire/cli/checkpoint/v2_pending_rotation.go | Introduces on-disk pending-rotation state stored under the git common dir. |
| cmd/entire/cli/checkpoint/v2_generation.go | Records a pending full-rotation marker after successful rotation reset (best-effort). |
| cmd/entire/cli/checkpoint/v2_generation_test.go | Adds coverage ensuring rotation still succeeds if the pending marker cannot be recorded. |
Cache --git-common-dir on V2GitStore so pending-publication operations stop forking git rev-parse for every read/write. Read pending publications once in pushV2Refs and pass the slice down, eliminating the second locked read inside publishPendingV2FullGenerationPublications. Skip the atomic rewrite in RemovePendingFullGenerationPublications when no rows actually matched, and drop the unused ClearPendingFullGenerationPublications export. Entire-Checkpoint: b664852f94af
|
Bugbot run |
…tation-logic # Conflicts: # cmd/entire/cli/migrate.go
|
Bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d014d6d. Configure here.
|
Bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c2446ed. Configure here.
https://entire.io/gh/entireio/cli/trails/324
Problem
Archived checkpoints v2
/full/<N>refs are intended to be disjoint cleanup units for raw transcripts. In production, old/full/currentdata was being merged back into fresh local/full/currentafter rotation, causing the same checkpoint IDs to appear in many archived generations. That keeps raw transcript objects reachable and prevents git GC from reclaiming old generations.The failure path was:
/full/currentinto/full/<N>and reset local/full/current/full/currentnormally/full/currentback into the fresh local generationFix
/full/<N>refs./full/currentwith a guarded force-with-lease from the expected previous current hash to the live local current head./full/currentrecovery from generic-merging the old generation back after a local rotation./full/current.lockfile.WithTimeouthelper and filtered removal so concurrent hook processes do not drop newly queued publications.Coverage
Added regression coverage for:
/full/currentVerification
Ran focused checkpoint/strategy/migration/lockfile regression tests, plus:
Note
Medium Risk
Changes v2 checkpoint rotation/push behavior and remote conflict recovery around
/full/currentand archived generations, which can affect data retention/GC if incorrect. Logic is well-covered by new tests but touches concurrency, ref updates, and push-with-lease semantics.Overview
Fixes v2
/full/currentrotation publication so archived generations aren’t accidentally merged back into the fresh generation during push recovery.Local rotations and migrations now write a pending publication marker (stored under the git common dir) before moving refs; pre-push reads these markers to push archived
/full/<N>refs first, then updates remote/full/currentvia a guarded--force-with-lease, and clears the markers on success.Tightens rotation conflict handling by detecting remote archive refs by hash mismatch (not just missing refs), batching archive fetches, and merging local-only checkpoints into the related remote archive before adopting the remote fresh
/full/current. Also updates ls-remote parsing and addslockfile.WithTimeoutto serialize marker file access, with extensive new regression tests for rotation, migration rollback, and push behavior.Reviewed by Cursor Bugbot for commit c2446ed. Configure here.