Detect local v2 checkpoint fallback reads#1232
Conversation
…v2-fallback-detection
Entire-Checkpoint: bb6788f9bf28
There was a problem hiding this comment.
Pull request overview
This PR adds a compatibility “fall-forward” read path so repositories with existing local v2 committed checkpoint refs can still be read even when v2 settings are disabled/removed, by centralizing committed-store selection and teaching multiple commands to use the shared store APIs.
Changes:
- Introduces
checkpoint.NewCommittedReader(ctx, repo, opts)that selects v1/v2/dual based on settings and presence of a local v2/mainref, and exposes a unifiedCommittedStoreinterface (read/list + summary updates + author lookup). - Threads an explicit worktree root through settings resolution via
settings.WithWorktreeRoot, and uses it where commands operate on repos other than the current cwd (e.g., local dispatch). - Updates multiple commands (resume/explain/rewind/review/strategy) and v2 store behavior (fetch targets resolved from store repo root) plus broad test updates.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/session.go | Falls forward from v1 to v2 committed metadata when reading checkpoint descriptions. |
| cmd/entire/cli/strategy/session_test.go | Adds coverage for description fallback to v2 when v1 misses. |
| cmd/entire/cli/strategy/push_v2.go | Updates v2 store construction to new signature. |
| cmd/entire/cli/strategy/push_v2_test.go | Updates v2 store construction across push tests. |
| cmd/entire/cli/strategy/phase_postcommit_test.go | Updates v2 store construction in post-commit tests. |
| cmd/entire/cli/strategy/manual_commit.go | Centralizes committed store selection via checkpoint.NewCommittedReader. |
| cmd/entire/cli/strategy/manual_commit_test.go | Adds coverage that listing uses local v2 when settings disabled. |
| cmd/entire/cli/strategy/manual_commit_rewind.go | Switches rewind/restore paths to use the unified committed store. |
| cmd/entire/cli/strategy/manual_commit_hooks.go | Uses new v2 store constructor in finalize flow. |
| cmd/entire/cli/strategy/manual_commit_condensation.go | Uses unified committed store for listing/reads; updates v2 write store construction. |
| cmd/entire/cli/strategy/generation_repair.go | Updates v2 store construction for generation repair command. |
| cmd/entire/cli/strategy/generation_repair_test.go | Updates v2 store construction in generation repair tests. |
| cmd/entire/cli/strategy/common.go | Replaces manual v1 tree scanning with committed store list APIs; factors mapping helper. |
| cmd/entire/cli/strategy/cleanup.go | Updates v2 store construction for cleanup candidate listing. |
| cmd/entire/cli/strategy/clean_test.go | Updates v2 store construction in clean tests. |
| cmd/entire/cli/settings/settings.go | Adds WithWorktreeRoot and git-common-dir based preferences resolution for explicit repo loads. |
| cmd/entire/cli/settings/settings_test.go | Adds test ensuring explicit worktree root controls which repo’s settings are loaded. |
| cmd/entire/cli/rewind.go | Uses unified committed store for transcript restoration path. |
| cmd/entire/cli/review_helpers.go | Uses unified committed store when resolving HEAD review checkpoint metadata. |
| cmd/entire/cli/review_context.go | Uses unified committed store when building review context from committed checkpoints. |
| cmd/entire/cli/resume.go | Refactors resume metadata resolution to prefer committed store, with local-tree and remote fallbacks. |
| cmd/entire/cli/resume_test.go | Adds tests for v2 local fallback behavior and store-derived metadata selection. |
| cmd/entire/cli/gitremote/gitremote.go | Adds GetRemoteURLInDir for remote URL resolution in a specific directory. |
| cmd/entire/cli/explain.go | Refactors explain to use the unified committed store, including prompt-only reads and summary updates. |
| cmd/entire/cli/explain_test.go | Adds/updates tests for local v2 ref behavior and selected-store summary generation. |
| cmd/entire/cli/explain_export.go | Updates export streaming/json paths to use committed store and settings gating. |
| cmd/entire/cli/explain_export_test.go | Updates v2 store construction in export tests. |
| cmd/entire/cli/doctor.go | Updates v2 store construction for v2 health checks. |
| cmd/entire/cli/dispatch/mode_local.go | Creates committed store using the target repo’s settings via WithWorktreeRoot. |
| cmd/entire/cli/dispatch/mode_local_test.go | Adds test that explicit repo dispatch uses the target repo’s checkpoint settings. |
| cmd/entire/cli/checkpoint/v2_store.go | Simplifies NewV2GitStore by removing the per-instance fetch-remote field. |
| cmd/entire/cli/checkpoint/v2_store_test.go | Updates v2 store construction throughout tests. |
| cmd/entire/cli/checkpoint/v2_resolve_test.go | Updates v2 store construction in resolve tests. |
| cmd/entire/cli/checkpoint/v2_read.go | Resolves v2 full-ref fetch targets from store repo root; adds v2 ReadSessionPrompts. |
| cmd/entire/cli/checkpoint/v2_read_test.go | Adds coverage that v2 full-ref fetch uses store repo (not cwd); updates constructors. |
| cmd/entire/cli/checkpoint/v2_precompute_test.go | Updates v2 store construction in precompute tests. |
| cmd/entire/cli/checkpoint/v2_generation_test.go | Updates v2 store construction in generation tests. |
| cmd/entire/cli/checkpoint/v2_committed.go | Adds v2 GetCheckpointAuthor implementation for unified store support. |
| cmd/entire/cli/checkpoint/v2_committed_tripwire_test.go | Updates v2 store construction in tripwire test. |
| cmd/entire/cli/checkpoint/remote/util.go | Extends FetchURL to support explicit worktree roots; adds GetRemoteURLInDir wrapper. |
| cmd/entire/cli/checkpoint/committed.go | Adds prompt-only read helpers; factors shared committed-info decoding and shared author lookup. |
| cmd/entire/cli/checkpoint/committed_reader_resolve.go | Adds unified CommittedStore + NewCommittedReader(ctx, repo, opts) with local v2 ref detection; improves v1 fallback safety. |
| cmd/entire/cli/checkpoint/committed_reader_resolve_test.go | Updates tests for new mode resolution and new prompt-only reader path; updates v2 construction. |
| cmd/entire/cli/checkpoint/checkpoint_test.go | Extends list committed tests to assert multi-session SessionIDs behavior. |
| cmd/entire/cli/checkpoint_reader.go | Removes legacy committed reader selection helper (replaced by checkpoint package factory). |
| cmd/entire/cli/attach.go | Updates v2 write/read store construction to new signature. |
Comments suppressed due to low confidence (1)
cmd/entire/cli/resume.go:302
- readCheckpointInfoFromStore logs and continues when ReadSessionMetadata fails. If the failure is due to context cancellation/deadline, the cancellation can be masked and the function may return ErrCheckpointNotFound instead. Consider returning immediately when ctx.Err() != nil (or when metaErr is a context cancellation) rather than continuing.
for i := range summary.Sessions {
metadata, metaErr := store.ReadSessionMetadata(ctx, checkpointID, i)
if metaErr != nil {
logging.Debug(ctx, "read checkpoint metadata: session metadata read failed",
slog.String("checkpoint_id", checkpointID.String()),
slog.Int("session_index", i),
slog.String("error", metaErr.Error()),
)
continue
}
Entire-Checkpoint: 38e3a39e90b8
Entire-Checkpoint: f5200712a9f1
|
Bugbot run |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
cmd/entire/cli/strategy/session.go:234
- The comment says "Find the first session's prompt/context path", but the code selects the latest session directory (
len(summary.Sessions)-1). Please update the comment to match the behavior (or adjust the selection logic if the first session is intended).
Entire-Checkpoint: eea453f4443a
Entire-Checkpoint: 34b012eb198c
Entire-Checkpoint: 636e48ea4107
|
Bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, 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 bf15518. Configure here.
| content, err := r.v2.ReadSessionMetadataAndPrompts(ctx, checkpointID, sessionIndex) | ||
| if err == nil { | ||
| return content, nil | ||
| } |
There was a problem hiding this comment.
Metadata-prompts skips v1 fallback
Medium Severity
DualCheckpointReader.ReadSessionMetadataAndPrompts returns as soon as the v2 read succeeds, even when Prompts is empty. ReadSessionPrompts already falls back to v1 in that case, but explain loads display content via ReadSessionMetadataAndPrompts, so checkpoints with prompts only in v1 can still show no prompts.
Reviewed by Cursor Bugbot for commit bf15518. Configure here.


https://entire.io/gh/entireio/cli/trails/396
What
This PR adds a temporary compatibility read path for repositories that already have local v2 checkpoint data. When refs/entire/checkpoints/v2/main exists locally, committed checkpoint reads can fall forward to v2 storage even if checkpoints_version/checkpoints_v2 settings are disabled or removed.
This branch does not change settings defaults or remove v2 write paths. Those changes can happen separately after reads can safely find existing v2-only checkpoint storage.
How
Notes
This is intended as a temporary compatibility path for repositories that wrote exclusively to v2 storage before the rollback. It only checks the local v2 ref to avoid adding remote fetch cost to local-only commands. More cleanup can remove the dual-read compatibility once v2 migration work resumes.
Verification
Note
Medium Risk
Changes the checkpoint read-path selection and v1/v2 fallback behavior (including remote
/full/*fetch targeting), which could affect which transcript/metadata is returned and when network fetches occur.Overview
Introduces
checkpoint.NewCommittedReaderreturning a newCommittedStoreabstraction that centralizes v1/v2/dual selection and falls forward to v2 whenrefs/entire/checkpoints/v2/mainexists locally, even if v2 settings are off.Extends committed checkpoint APIs to support prompt-only reads (
ReadSessionPrompts,ReadSessionMetadataAndPrompts) and enriches listing output with orderedSessionIDs; dual-read fallback logic is tightened to avoid mismatching v1 sessions when v2 metadata is missing/malformed, and merged lists are explicitly time-sorted.Refactors CLI call sites (e.g.,
explain, local dispatch, branch checkpoint listing) onto the shared store, removes the oldcheckpoint_reader.gowiring, and updates v2/full/*fetching to resolve the effective remote from the store repo root (plus adds author lookup for v2 via shared ref-walk logic).Reviewed by Cursor Bugbot for commit bf15518. Configure here.