Remove checkpoints v1.1 mirror machinery - #1454
Conversation
…into feat/remove-checkpoints-v1.1-overhead
Remove checkpoints_version handling so committed metadata resolves only through entire/checkpoints/v1. Keep the mirror field temporarily unset so downstream mirror-removal commits can compile while they delete the old callers. Entire-Checkpoint: 17f99b7f5376
Stop maintaining and diagnosing the retired v1.1 committed metadata mirror. Committed reads and writes now use the v1 metadata branch directly, and tests that only covered the custom-ref path are removed. Entire-Checkpoint: 35f3afc670c0
Update checkpoint documentation for the v1-only committed metadata path after retiring the custom read mirror. Entire-Checkpoint: 62eca7314d85
Drop an unused resume test helper left behind after pruning v1.1-specific resume coverage. Entire-Checkpoint: a59356210b3b
Remove no-op settings-based committed ref resolution and stale facade surface after collapsing committed metadata to v1. Route the remaining resume path through checkpoint.Open and keep summary generation on the committed store it uses. Entire-Checkpoint: 14d6de264f7f
Stores.temporary was always assigned the same *GitStore pointer as Primary, so it was derivable state with no purpose. Temporary() now returns Primary directly, keeping the shadow-branch intent at call sites without a duplicate field. Entire-Checkpoint: 6270a68f519c
|
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 0b2f11f. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR removes the checkpoints v1.1 “custom ref” mirror topology and associated maintenance/diagnostics paths, returning committed checkpoint reads/writes/pushes to a single v1 ref (entire/checkpoints/v1). This aligns the codebase with the newer checkpoint-store abstraction direction by dropping the unused/refuted alternate topology and its settings surface.
Changes:
- Remove v1.1 mirror machinery across strategy, hooks, push/fetch/reconcile, doctor/bundle, and tests; committed metadata resolves to v1 only.
- Simplify checkpoint ref topology resolution and
checkpoint.Openoptions to default to v1, ignoring the retiredstrategy_options.checkpoints_version. - Update documentation/tests accordingly; adjust GitHub remote parsing to delegate to
gitremote.ParseURL(including Entire mirror remotes) and add validation for extra path segments.
Reviewed changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go.sum | Bump dependency checksums (ogen, goldmark). |
| go.mod | Bump ogen and goldmark versions. |
| docs/architecture/sessions-and-checkpoints.md | Remove v1.1 mirror documentation; document v1-only committed storage. |
| cmd/entire/cli/strategy/v1_custom_ref_mirror.go | Delete v1.1 mirror implementation. |
| cmd/entire/cli/strategy/v1_custom_ref_mirror_test.go | Delete v1.1 mirror unit tests. |
| cmd/entire/cli/strategy/rewind_test.go | Remove v1.1 transcript read-path test coverage. |
| cmd/entire/cli/strategy/push_common.go | Stop advancing mirror during fetch/rebase ref updates (v1-only). |
| cmd/entire/cli/strategy/push_common_test.go | Update tests/comments to use a generic non-branch ref example; remove v1.1 expectations. |
| cmd/entire/cli/strategy/metadata_reconcile.go | Stop advancing mirror during metadata ref reconciliation (v1-only). |
| cmd/entire/cli/strategy/metadata_reconcile_test.go | Remove reconciliation test asserting v1.1 mirror behavior. |
| cmd/entire/cli/strategy/manual_commit_test.go | Remove v1.1 committed-read mirror test for rewind-point prompt reads. |
| cmd/entire/cli/strategy/manual_commit_push.go | Remove pre-push mirror refresh behavior. |
| cmd/entire/cli/strategy/manual_commit_hooks.go | Remove best-effort mirror updates after committed writes in hooks. |
| cmd/entire/cli/strategy/manual_commit_condensation.go | Remove best-effort mirror updates after condensation writes. |
| cmd/entire/cli/strategy/common.go | Replace “advance primary + mirror” with v1-only ref updates in EnsurePrimaryRef. |
| cmd/entire/cli/strategy/common_test.go | Update EnsurePrimaryRef tests to v1-only behavior. |
| cmd/entire/cli/strategy/cleanup.go | Update orphan cleanup to advance only v1. |
| cmd/entire/cli/strategy/clean_test.go | Remove orphan detection test that depended on v1.1 read topology. |
| cmd/entire/cli/strategy/checkpoint_remote.go | Remove mirror refresh after metadata fetch. |
| cmd/entire/cli/strategy/checkpoint_remote_test.go | Remove test assertions tied to v1.1 mirroring after fetch. |
| cmd/entire/cli/settings/settings.go | Remove v1.1 mirror opt-in helpers from settings surface. |
| cmd/entire/cli/settings/settings_test.go | Remove tests for v1.1 checkpoints_version parsing helpers. |
| cmd/entire/cli/search/search_test.go | Add tests for Entire mirror remote parsing and extra path segment rejection. |
| cmd/entire/cli/search/github.go | Delegate remote parsing to gitremote.ParseURL; validate/normalize GitHub remotes incl. Entire mirrors. |
| cmd/entire/cli/review_context_test.go | Remove v1.1 custom-ref read-path test cases. |
| cmd/entire/cli/resume.go | Open checkpoint stores via checkpoint.Open and remove v1.1 bootstrap special-casing. |
| cmd/entire/cli/resume_test.go | Remove resume behavior tests that relied on v1.1 reads/topology. |
| cmd/entire/cli/paths/paths.go | Remove v1.1 custom ref constant. |
| cmd/entire/cli/integration_test/remote_operations_test.go | Remove integration coverage for pushing v1.1 mirror ref. |
| cmd/entire/cli/integration_test/attach_test.go | Remove integration coverage for attach creating/updating v1.1 ref. |
| cmd/entire/cli/head_checkpoint_flags_test.go | Remove v1.1 custom-ref read-path test coverage. |
| cmd/entire/cli/git_operations.go | Remove mirror refresh after fetching metadata from origin. |
| cmd/entire/cli/git_operations_test.go | Remove fetch+mirror v1.1 ref test. |
| cmd/entire/cli/explain.go | Remove mirror repair after summary write; simplify summary generation call path. |
| cmd/entire/cli/explain_test.go | Update summary-generation tests to v1-only behavior. |
| cmd/entire/cli/doctor.go | Remove doctor check/repair flow for v1.1 read mirror drift. |
| cmd/entire/cli/doctor_test.go | Remove tests covering v1.1 mirror diagnosis/repair and doctor wiring. |
| cmd/entire/cli/doctor_bundle.go | Remove mirror diagnosis reporting from bundles; keep refs listing. |
| cmd/entire/cli/doctor_bundle_test.go | Update bundle expectations to exclude mirror status lines. |
| cmd/entire/cli/dispatch/mode_local.go | Update comments around repo-scoped store construction (no v1.1 topology). |
| cmd/entire/cli/dispatch/mode_local_test.go | Remove dispatch tests that relied on v1.1 custom-ref reads. |
| cmd/entire/cli/checkpoint/store.go | Update GitStore comment to remove mirror advancement mention. |
| cmd/entire/cli/checkpoint/open.go | Remove settings-based ref resolution; default to v1 and keep explicit ref override. |
| cmd/entire/cli/checkpoint/committed_refs.go | Collapse CommittedRefs to v1-only and remove mirror fields/topology branching. |
| cmd/entire/cli/checkpoint/committed_refs_test.go | Update tests to assert checkpoints_version is ignored and v1 refs are used. |
| cmd/entire/cli/checkpoint/committed_read_store_test.go | Remove v1.1 read-path tests; assert checkpoints_version is ignored. |
| cmd/entire/cli/attach.go | Remove injected-settings topology override; stop mirroring after committed writes. |
| cmd/entire/cli/attach_test.go | Remove attach test asserting v1.1 mirror advancement. |
| CLAUDE.md | Remove v1.1 mirror behavior from architecture notes. |
Decouple checkpoint summary generation from the git store concrete type and clarify why diagnostic bundles still include refs/entire after removing the v1.1 mirror machinery. Entire-Checkpoint: da874a38be9e
Soph
left a comment
There was a problem hiding this comment.
Reviewed the full diff. Behavior-preserving v1.1 removal: the AdvanceCommittedPrimary/AdvanceLocalRef → setRefHash swap drops only the mirror side-effect (correct), checkpoint.Open centralizes ref + blob-fetcher wiring cleanly, and docs (CLAUDE.md + sessions-and-checkpoints.md) are updated in lockstep. Verified locally: go build, go vet, and checkpoint/strategy/settings package tests all green; grepped every removed symbol — no dangling references (only the intentional historical CHANGELOG entry and the TestNewGitStore_IgnoresCheckpointsVersion regression test). Main residual risk (inert local v1.1 refs) is intentional and documented. LGTM.
…ts-v1.1-overhead # Conflicts: # cmd/entire/cli/strategy/manual_commit_push.go
https://entire.io/gh/entireio/cli/trails/597
Why
The checkpoints v1.1 rollout is no longer moving forward. Keeping the custom-ref read mirror,
checkpoints_versionsetting, and mirror repair paths would preserve a second checkpoint topology that conflicts with the newer checkpoint-store abstraction direction from #1433.What changed
Committed checkpoint metadata now resolves to
entire/checkpoints/v1only. The retired v1.1 mirror maintenance, pre-push mirroring, doctor/bundle diagnosis, settings parsing, tests, and active architecture docs were removed.Implementation decisions
This keeps the
checkpoint.Openconstruction seam from #1451, but trims no-op settings and stale facade surface after the committed refs were collapsed to v1. No fallback reads or migration fromrefs/entire/checkpoints/v1.1were added; existing v1.1 refs become inert git data.Tradeoffs and alternatives considered
We removed
strategy_options.checkpoints_versioninstead of leaving it as a compatibility no-op because the option was never public or active. Keeping it would invite future code to preserve the retired v1.1 topology.Reviewer notes
Published changelog entries are intentionally left unchanged.
Note
Medium Risk
Wide changes to committed-metadata read/write, push, fetch, resume, and doctor paths; wrong ref handling could hide checkpoints until users fetch v1, though behavior is intentionally unified on a single topology.
Overview
Removes the retired checkpoints v1.1 path: committed metadata no longer uses a separate read mirror (
refs/entire/checkpoints/v1.1),strategy_options.checkpoints_version, or settings-driven ref topology.ResolveCommittedRefsalways returns the v1-onlyCommittedRefs(Primary, Read, and Push all onentire/checkpoints/v1).The
strategy/v1_custom_ref_mirrorlayer is deleted—no post-write mirroring, pre-push mirror refresh, fetch-time mirror updates, orAdvanceCommittedPrimarymirror side effects. Ref updates go through directsetRefHashon the v1 branch.entire doctorno longer diagnoses or repairs mirror drift; diagnostic bundles drop mirror status lines.checkpoint.Openis simplified (no settings-based ref resolution;Temporary()aliases Primary). Call sites such as attach, explain summary save, and git metadata fetch stop calling mirror helpers.paths.MetadataRefNameandMirrorsToV1CustomRefare removed. Architecture docs and large v1.1-specific test suites are removed or rewritten for v1-only behavior.Unrelated:
ParseGitHubRemotenow delegates togitremote.ParseURL(including Entire mirror remotes). Minorgo.modbumps (ogen,goldmark). Existing local v1.1 refs are not migrated—they become inert git refs.Reviewed by Cursor Bugbot for commit 0b2f11f. Configure here.