Add entire doctor migrate-checkpoints (git-branch → git-refs)#1611
Add entire doctor migrate-checkpoints (git-branch → git-refs)#1611Soph wants to merge 3 commits into
entire doctor migrate-checkpoints (git-branch → git-refs)#1611Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 856c7ca. Configure here.
| result, err := checkpoint.MigrateBranchToRefs(ctx, repo, dryRun) | ||
| if err != nil { | ||
| return fmt.Errorf("migrate checkpoints: %w", err) | ||
| } |
There was a problem hiding this comment.
Ctrl+C shows noisy migrate error
Low Severity
The migrate-checkpoints RunE wraps errors from MigrateBranchToRefs and PushMigratedCheckpointRefs with fmt.Errorf without mapping context.Canceled to NewSilentError, so interrupting the command prints a noisy Cobra error instead of exiting quietly.
Additional Locations (1)
Triggered by learned rule: Map context.Canceled to NewSilentError on user cancellation
Reviewed by Cursor Bugbot for commit 856c7ca. Configure here.
| if err != nil { | ||
| return fmt.Errorf("push migrated refs: %w", err) | ||
| } | ||
| fmt.Fprintf(out, "Pushed %d checkpoint ref(s).\n", pushed) |
There was a problem hiding this comment.
Disabled push reports zero pushed
Medium Severity
After the user confirms pushing migrated refs, PushMigratedCheckpointRefs returns (0, nil) when push_sessions is disabled, and the command still prints Pushed 0 checkpoint ref(s). even though nothing was sent to the remote and refs remain queued locally.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 856c7ca. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR adds a doctor-time migration path to move existing checkpoints from the legacy single branch store (entire/checkpoints/v1) into the per-checkpoint ref layout used by the git-refs checkpoint store (refs/entire/checkpoints/<shard>/<id>), enabling repos to transition storage backends without rewriting branch history.
Changes:
- Add
entire doctor migrate-checkpointsto migrate v1-branch checkpoint subtrees into per-checkpoint ref commits, with--dry-runand optional interactive “push now”. - Implement
checkpoint.MigrateBranchToRefsplus unit tests validating byte-identical trees, idempotency, fast-forward advancement, and dry-run behavior. - Extract/refactor checkpoint-ref queue flushing in
manual_commit_push.goto share logic between pre-push (fail-soft) and migration “push now” (error-surfacing).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/manual_commit_push.go | Extracts queued checkpoint-ref flush into a shared helper and adds an error-surfacing push entrypoint for migration. |
| cmd/entire/cli/doctor.go | Registers the new doctor migrate-checkpoints subcommand. |
| cmd/entire/cli/doctor_migrate.go | Implements the doctor migrate-checkpoints command, including interactive “push now” prompt and dry-run flag. |
| cmd/entire/cli/checkpoint/migrate.go | Adds the core migration routine to convert v1-branch checkpoints into per-checkpoint refs. |
| cmd/entire/cli/checkpoint/migrate_test.go | Adds tests covering migration correctness, idempotency, advancement, dry-run, and no-branch no-op. |
| if err := refsStore.setRef(ctx, cid, commitHash); err != nil { | ||
| return fmt.Errorf("set ref for checkpoint %s: %w", cid, err) | ||
| } | ||
| result.Migrated = append(result.Migrated, cid) | ||
| return nil |
| repo, err := OpenRepository(ctx) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("open repository: %w", err) | ||
| } | ||
| defer repo.Close() | ||
| return flushCheckpointRefsQueue(ctx, repo, ps.pushTarget()) | ||
| } |
| // migrateCheckpointsPushRemote is the remote the opt-in "push now" targets; the | ||
| // actual push URL is resolved from checkpoint-remote settings when configured. | ||
| const migrateCheckpointsPushRemote = "origin" |
| parent := plumbing.ZeroHash | ||
| if existing, err := repo.Reference(refName, true); err == nil { | ||
| parent = existing.Hash() | ||
| if commit, cerr := repo.CommitObject(parent); cerr == nil && commit.TreeHash == cpTreeHash { | ||
| result.Skipped++ | ||
| return nil | ||
| } | ||
| } |
| // the existing ref when present (re-migration fast-forwards) or as an | ||
| // orphan for a brand-new ref — then point the ref at it and enqueue it. | ||
| msg := fmt.Sprintf("Import checkpoint %s (migrated from git-branch)", cid) | ||
| commitHash, err := CreateCommit(ctx, repo, cpTreeHash, parent, msg, authorName, authorEmail) |
There was a problem hiding this comment.
I know we decided to commit the existing subtree objects only, but Paulo brought up a good point here:
With that in mind, the current CheckpointVersion is stored in metadata.json. This blocks us from migrating from branch to refs without changing the checkpoint metadata itself. As a result, every migrated checkpoint would require a new Blob for the updated metadata.json and a new Tree to hold it, on top of the new Commit we would need regardless. Which is not ideal, as one of the properties of the migration was co-existence of both formats with minimum overhead.
We already have a similar issue with SessionFilePaths, which treats paths differently depending on whether the checkpoint was created for a branch or for a ref.
Part of me is wondering whether we should update both the CheckpointVersion as well as the SessionFilePaths during the migration since this information wouldn't reflect the reality of the new storage location anymore:
- The
CheckpointVersionwould still point tobranch-v1 SessionFilePathswould still point to the shard path relative to the root of the checkpoint branch and not relative to their new storage location
There was a problem hiding this comment.
FWIW: Part of me is also wondering whether we should remove the CheckpointVersion completely given that "If you can read this, you already know how to read it" 😓
Convert checkpoints on the entire/checkpoints/v1 branch into per-checkpoint refs (refs/entire/checkpoints/<shard>/<id>), the git-refs store's layout. checkpoint.MigrateBranchToRefs walks the v1 branch tip and, for each checkpoint, wraps its CURRENT subtree object in a fresh commit and points the ref at it — existing branch commits are not remapped. Since the git-refs ref tree is the branch's <shard>/<id> subtree byte-for-byte, a migrated checkpoint reads identically under either backend. It is idempotent: a checkpoint whose ref already carries the same tree is skipped, and a changed checkpoint re-migrates by fast-forward (new commit parents on the existing ref, so no history is lost). New/advanced refs are enqueued for push; the function itself never pushes. The `doctor migrate-checkpoints` command reports migrated/skipped/total and, per the requested policy, only pushes when it can prompt: interactively it asks whether to push now; non-interactively it never pushes (refs stay queued and flush on the next push once git-refs is primary). `--dry-run` reports what would change without writing refs. Push reuse: the git-refs pre-push queue-flush is extracted into strategy.flushCheckpointRefsQueue (shared by the fail-soft pre-push path and the new error-surfacing strategy.PushMigratedCheckpointRefs), so the "push now" option goes through the exact same fast-forward + fetch/replay logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: ceed5d17e529
…ntRefs - doctor migrate-checkpoints now reuses the existing confirmDoctorFix helper instead of a near-duplicate confirm form — picking up its context-cancellation guard (huh opens the TTY during startup regardless of ctx state) and dropping the huh/errors/context imports. - PushMigratedCheckpointRefs takes the *git.Repository the command already opened rather than re-opening one, matching the strategy entry-point pattern and removing a redundant repo open per invocation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01f324c13852
…remote, parent validation) - Map context.Canceled to NewSilentError so Ctrl-C exits quietly instead of printing a noisy Cobra error (matches the clean.go convention). [cursor] - When the user confirms push but checkpoint pushing is disabled in settings, report that the refs stay queued instead of "Pushed 0 checkpoint ref(s)". [cursor] - Add a --remote flag (default origin) instead of hardcoding "origin", so repos that push to a different remote work. [copilot] - MigrateBranchToRefs now only uses an existing ref as the new commit's parent when it resolves to a real commit; a ref at an unreadable/non-commit object is treated as absent (orphan) rather than parenting on a bad hash, which would corrupt the commit graph for fetch+replay. [copilot] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 5d352fa1b1bb
Soph
force-pushed
the
feat/checkpoint-migrate
branch
2 times, most recently
from
06b3c99 to
96537c1
Compare
https://entire.io/gh/entireio/cli/trails/735
Adds a migration that converts checkpoints on the
entire/checkpoints/v1branch into per-checkpoint refs underrefs/entire/checkpoints/<shard>/<id>— the layout the git-refs store uses — so a repo on the git-branch store can move to git-refs.How it works
checkpoint.MigrateBranchToRefs(ctx, repo, dryRun)walks the v1 branch tip and, for each checkpoint:<shard>/<id>subtree byte-for-byte, a migrated checkpoint reads identically under either backend.Idempotent: a checkpoint whose ref already carries the same tree is skipped; a changed checkpoint re-migrates by fast-forward (the new commit parents on the existing ref, so nothing is lost from history). Safe to re-run after more branch activity.
The command
entire doctor migrate-checkpoints— reportsmigrated / already-up-to-date / total.Push behavior (as requested): the function only enqueues; the command decides whether to push:
git pushonce git-refs is the primary store.--dry-runreports what would migrate without writing refs.Push reuse
The git-refs pre-push queue-flush is extracted into
strategy.flushCheckpointRefsQueue, shared by the fail-soft pre-push path and the new error-surfacingstrategy.PushMigratedCheckpointRefs. So "push now" goes through the exact same fast-forward + fetch/replay recovery logic (no force). No behavior change to pre-push.Testing
migrate_test.go: basic migration + byte-identical ref trees + read-back via the git-refs store; idempotent re-run (all skipped, refs unmoved); advance-on-branch-change (fast-forward parenting); dry-run writes nothing; no-branch no-op.duplquiet). Checkpoint + strategy unit suites pass; integration 392; git-refs canary 58/58 +1 skip, roger 4/4.Not included (follow-ups)
git-branchmirror for downgrade safety — unchanged from the git-refs store's existing scope.Refs: #1471
🤖 Generated with Claude Code
Note
Medium Risk
Migration rewrites local git refs and checkpoint data layout; push behavior differs between interactive opt-in and fail-soft pre-push, though logic is shared and tested.
Overview
Adds
entire doctor migrate-checkpointsso repos on the git-branch checkpoint store can move to the git-refs layout (refs/entire/checkpoints/<shard>/<id>).checkpoint.MigrateBranchToRefswalks checkpoints onentire/checkpoints/v1, wraps each checkpoint’s current subtree in a new commit, and points the per-checkpoint ref at it (enqueueing push like normal git-refs writes). It is idempotent (same tree → skip; changed tree → fast-forward parent) and supports--dry-run. Unmappable IDs are skipped with a warning.The doctor subcommand reports migrated / skipped / total counts. Interactive runs can opt into an immediate push; non-interactive runs leave refs queued for the next
git push.strategy.flushCheckpointRefsQueueextracts the git-refs push-queue drain logic from pre-push; pre-push still fail-soft on errors, whilePushMigratedCheckpointRefssurfaces errors for the migration “push now” path.migrate_test.gocovers migration fidelity, idempotency, branch updates, dry-run, and no-branch no-op.Reviewed by Cursor Bugbot for commit 856c7ca. Configure here.