checkpoint resume: address review findings · Entire
checkpoint resume: address review findings
75b158b→main·
pfleidi·1w ago·3 files·+124 added/-25 removed
Log commit-resolution failures at debug for diagnosability, contain the remote-fallback lookup swap inside resumeAutoTarget so callers no longer thread it to their deferred close, correct the auto-detection docstring to the actual local-checkpoint / local-branch / remote-fallback / commit order, and cover the --checkpoint flag, ambiguous prefixes, and the worktree-clash pointer with tests.
Sessions
01KX4A9S37VZTVXCK59QGTVYJJView transcript
[?
Implement Checkpoint Resume CommandClaude Code·2 steps](/content/gh/entireio/cli/session/ba137a72-eaf9-443b-b868-49da78b39915#timeline-01KX4A9S37VZTVXCK59QGTVYJJ/index.html)
Changes
3
cmd/entire/cli
checkpoint/id
Mid.go+2/-2
Mcheckpoint_resume.go+26/-20
Mcheckpoint_resume_test.go+96/-3
45 unmodified lines
46
47
48
49
50
49
50
51
52
53
45 unmodified lines
// prefixShapeRegex matches strings shaped like a checkpoint ID or a prefix
// of one: 1-12 lowercase hex characters (legacy) or 1-26 Crockford base32
// characters (ULID). Kept next to Pattern/ulidPattern so the alphabet and
// width bounds cannot drift from the full-ID patterns above.
// characters (ULID). Kept next to Pattern/ulidPattern as a reminder to update
// them together when the ID formats change.
var prefixShapeRegex = regexp.MustCompile(`^(?:[0-9a-f]{1,12}|[0-9ABCDEFGHJKMNPQRSTVWXYZ]{1,26})$`)
// CouldBePrefix reports whether s is shaped like a checkpoint ID or a prefix
Mcmd/entire/cli/checkpoint/id/id.go+2/-2
4 unmodified lines
5
6
7
8
9
10
11
93 unmodified lines
105
106
107
107
108
109
108
109
110
111
112
113
115
116
117
118
119
120
121
122
123
124
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
130
131
132
133
134
135
135
136
137
138
139
140
139
140
141
142
143
144
142
145
146
147
148
149
150
148
151
152
150
153
154
155
156
21 unmodified lines
178
179
180
181
182
183
184
185
186
4 unmodified lines
"errors"
"fmt"
"io"
"log/slog"
"github.com/entireio/cli/cmd/entire/cli/agent/external"
"github.com/entireio/cli/cmd/entire/cli/checkpoint"
93 unmodified lines
case commitFlag != "":
return resumeCommitTarget(ctx, cmd, lookup, commitFlag, force)
case target != "":
var resumeErr error
lookup, resumeErr = resumeAutoTarget(ctx, cmd, lookup, target, force)
return resumeErr
return resumeAutoTarget(ctx, cmd, lookup, target, force)
default:
return runCheckpointResumePicker(ctx, cmd, lookup, force)
}
// resumeAutoTarget resolves a positional target: checkpoint-ID prefix first
// (local, then remote fallback only if nothing local matches), then local
// branch, then commit revision. Only local branches are auto-detected as
// branch targets: branchCommit also resolves origin/<name> and, for a target
// like "HEAD", would wrongly match refs/remotes/origin/HEAD, misrouting
// revision syntax that must fall through to commit resolution instead
// (--branch still handles remote-only branches explicitly via runResume).
// Returns the possibly-swapped lookup so the caller's deferred close stays
// correct.
func resumeAutoTarget(ctx context.Context, cmd *cobra.Command, lookup *explainCheckpointLookup, target string, force bool) (*explainCheckpointLookup, error) {
// resumeAutoTarget resolves a positional target, trying in order: local
// checkpoint-ID prefix, local branch, remote checkpoint fallback, commit
// revision. The local branch check runs before the remote checkpoint fetch so
// branch names never pay a network round-trip. Only local branches are
// auto-detected as branch targets: branchCommit also resolves origin/<name>
// and, for a target like "HEAD", would wrongly match refs/remotes/origin/HEAD,
// misrouting revision syntax that must fall through to commit resolution
// instead (--branch still handles remote-only branches explicitly via
// runResume). A lookup swapped in by the remote fallback is closed here; the
// caller keeps ownership of the lookup it passed in.
func resumeAutoTarget(ctx context.Context, cmd *cobra.Command, lookup *explainCheckpointLookup, target string, force bool) error {
// Targets that can't be checkpoint IDs (e.g. "feature/foo") skip the
// store lookup and its remote-fetch fallback entirely.
shapedLikeCheckpoint := id.CouldBePrefix(target)
if shapedLikeCheckpoint {
if matches := matchCheckpointPrefix(lookup, target); len(matches) > 0 {
return lookup, resumeMatchedCheckpoints(ctx, cmd, lookup, target, matches, force)
return resumeMatchedCheckpoints(ctx, cmd, lookup, target, matches, force)
}
}
if branchExistsLocally(lookup.repo, target) {
return lookup, runResume(ctx, cmd, target, force)
return runResume(ctx, cmd, target, force)
}
if shapedLikeCheckpoint {
var matches []id.CheckpointID
matches, lookup = matchCheckpointPrefixWithRemoteFallback(ctx, cmd.ErrOrStderr(), lookup, target)
matches, fresh := matchCheckpointPrefixWithRemoteFallback(ctx, cmd.ErrOrStderr(), lookup, target)
if fresh != lookup {
defer func() { _ = fresh.Close() }()
lookup = fresh
}
if len(matches) > 0 {
return lookup, resumeMatchedCheckpoints(ctx, cmd, lookup, target, matches, force)
return resumeMatchedCheckpoints(ctx, cmd, lookup, target, matches, force)
}
}
err := resumeCommitTarget(ctx, cmd, lookup, target, force)
if errors.Is(err, errNoResumeCommit) {
return lookup, fmt.Errorf("nothing matched %q as a checkpoint ID, branch, or commit\nHint: run 'entire checkpoint list' to see available checkpoints", target)
return fmt.Errorf("nothing matched %q as a checkpoint ID, branch, or commit\nHint: run 'entire checkpoint list' to see available checkpoints", target)
}
return lookup, err
return err
}
func resumeMatchedCheckpoints(ctx context.Context, cmd *cobra.Command, lookup *explainCheckpointLookup, prefix string, matches []id.CheckpointID, force bool) error {
21 unmodified lines
renderAmbiguousPrefixFailure(errW, ref, "commits", buildAmbiguousCommitMatches(lookup.repo, ambiguousMatches))
return NewSilentError(err)
}
logging.Debug(ctx, "checkpoint resume: commit resolution failed",
slog.String("ref", ref),
slog.String("error", err.Error()))
return fmt.Errorf("%w matching %q", errNoResumeCommit, ref)
}
commit, err := lookup.repo.CommitObject(hash)
Mcmd/entire/cli/checkpoint_resume.go+26/-20
3 unmodified lines
4
5
6
7
8
9
10
64 unmodified lines
75
76
77
77
78
79
80
81
76 unmodified lines
158
159
160
160
161
162
163
163
164
165
166
167
7 unmodified lines
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
3 unmodified lines
"bytes"
"context"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
64 unmodified lines
t.Errorf("output should mention restore-only fallback, got: %s", out.String())
}
branch, err := GetCurrentBranch(context.Background())
if err != nil || branch != "master" {
t.Errorf("HEAD moved: branch = %q err = %v, want master", branch, err)
}
}
func TestCheckpointResumeFlag_Checkpoint(t *testing.T) {
repo, _, _ := setupCheckpointResumeRepo(t)
cpID := id.MustCheckpointID("abc123def456")
writeCommittedResumeCheckpoint(t, repo, cpID, "session-flag", time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC))
cmd, out := newCheckpointResumeTestCmd(t)
cmd.SetArgs([]string{"--checkpoint", "abc123"})
if err := cmd.Execute(); err != nil {
t.Fatalf("Execute() error = %v\noutput: %s", err, out.String())
}
if !strings.Contains(out.String(), "session-flag") {
t.Errorf("output should mention restored session ID, got: %s", out.String())
}
}
func TestCheckpointResumeFlag_AmbiguousCheckpointPrefix(t *testing.T) {
repo, _, _ := setupCheckpointResumeRepo(t)
cpA := id.MustCheckpointID("abc123def456")
cpB := id.MustCheckpointID("abc123aaa111")
writeCommittedResumeCheckpoint(t, repo, cpA, "session-a", time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC))
writeCommittedResumeCheckpoint(t, repo, cpB, "session-b", time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC))
cmd, out := newCheckpointResumeTestCmd(t)
cmd.SetArgs([]string{"--checkpoint", "abc123"})
if err := cmd.Execute(); err == nil {
t.Fatal("Execute() = nil, want error for ambiguous checkpoint prefix")
}
output := out.String()
if !strings.Contains(output, "Ambiguous checkpoint prefix") {
t.Errorf("output should render the ambiguity failure, got: %s", output)
}
for _, cpID := range []id.CheckpointID{cpA, cpB} {
if !strings.Contains(output, cpID.String()) {
t.Errorf("output should list match %s, got: %s", cpID, output)
}
}
}
// When the checkpoint's branch is checked out in another worktree, resume must
// point there instead of switching branches or restoring logs.
func TestCheckpointResume_WorktreeClash(t *testing.T) {
repo, w, baseHead := setupCheckpointResumeRepo(t)
cpID := id.MustCheckpointID("abc123def456")
writeCommittedResumeCheckpoint(t, repo, cpID, "session-clash", time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC))
// Put the trailer commit on a branch that is NOT master: commit on master,
// point "feat" at it, then move master back to the base commit.
trailerCommit, err := w.Commit("work\n\nEntire-Checkpoint: "+cpID.String(), &git.CommitOptions{
AllowEmptyCommits: true,
Author: &object.Signature{Name: "Test User", Email: "test@example.com"},
})
if err != nil {
t.Fatalf("commit: %v", err)
}
if err := repo.Storer.SetReference(plumbing.NewHashReference(plumbing.NewBranchReferenceName("feat"), trailerCommit)); err != nil {
t.Fatalf("create feat: %v", err)
}
if err := repo.Storer.SetReference(plumbing.NewHashReference(plumbing.NewBranchReferenceName(masterBaseBranch), baseHead)); err != nil {
t.Fatalf("reset master: %v", err)
}
clashDir := filepath.Join(t.TempDir(), "clash-wt")
worktreeAdd := exec.CommandContext(context.Background(), "git", "worktree", "add", clashDir, "feat")
if addOut, err := worktreeAdd.CombinedOutput(); err != nil {
t.Fatalf("git worktree add: %v\n%s", err, addOut)
}
t.Cleanup(func() {
if err := exec.CommandContext(context.Background(), "git", "worktree", "remove", clashDir, "--force").Run(); err != nil {
t.Logf("git worktree remove: %v", err)
}
})
cmd, out := newCheckpointResumeTestCmd(t)
cmd.SetArgs([]string{cpID.String()})
if err := cmd.Execute(); err != nil {
t.Fatalf("Execute() error = %v\noutput: %s", err, out.String())
}
output := out.String()
if !strings.Contains(output, "already checked out") {
t.Errorf("output should mention the worktree clash, got: %s", output)
}
if !strings.Contains(output, "entire checkpoint resume "+cpID.String()) {
t.Errorf("output should include the checkpoint-specific resume command, got: %s", output)
}
branch, err := GetCurrentBranch(context.Background())
if err != nil || branch != masterBaseBranch {
t.Errorf("HEAD moved: branch = %q err = %v, want master", branch, err)
}
}
func TestCheckpointResumeCommit_NoTrailer(t *testing.T) {
tmpDir := t.TempDir()
t.Chdir(tmpDir)