Skip to content

Commit a14451d

Browse files
computermodecomputermode
authored
Merge pull request #1249 from entireio/disallow-checkpoints-v2-setting
Remove checkpoints v2 write paths
2 parents be0bdf7 + 67e7ce7 commit a14451d

22 files changed

Lines changed: 263 additions & 6037 deletions

‎cmd/entire/cli/attach.go‎

Lines changed: 16 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"github.com/entireio/cli/cmd/entire/cli/logging"
2323
"github.com/entireio/cli/cmd/entire/cli/paths"
2424
"github.com/entireio/cli/cmd/entire/cli/session"
25-
"github.com/entireio/cli/cmd/entire/cli/settings"
2625
"github.com/entireio/cli/cmd/entire/cli/strategy"
2726
"github.com/entireio/cli/cmd/entire/cli/trailers"
2827
"github.com/entireio/cli/cmd/entire/cli/transcript/compact"
@@ -298,22 +297,8 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ
298297
writeOpts.CompactTranscript = compacted
299298
}
300299

301-
v2 := settings.CheckpointsVersion(logCtx) == 2
302-
if !v2 {
303-
if err := store.WriteCommitted(ctx, writeOpts); err != nil {
304-
return fmt.Errorf("failed to write checkpoint: %w", err)
305-
}
306-
}
307-
// IsCheckpointsV2Enabled is true whenever v2 writes are enabled, including
308-
// both v2-only mode (checkpoints_version == 2) and dual-write mode. Only
309-
// v2-only mode propagates the error.
310-
if settings.IsCheckpointsV2Enabled(logCtx) {
311-
if err := writeAttachCheckpointV2(logCtx, repo, writeOpts); err != nil {
312-
if v2 {
313-
return fmt.Errorf("failed to write checkpoint to v2: %w", err)
314-
}
315-
logging.Warn(logCtx, "attach v2 dual-write failed", "error", err)
316-
}
300+
if err := store.WriteCommitted(ctx, writeOpts); err != nil {
301+
return fmt.Errorf("failed to write checkpoint: %w", err)
317302
}
318303

319304
// Create or update session state.
@@ -337,15 +322,6 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ
337322
return nil
338323
}
339324

340-
// writeAttachCheckpointV2 writes attach-created checkpoints into the v2 refs.
341-
func writeAttachCheckpointV2(ctx context.Context, repo *git.Repository, opts cpkg.WriteCommittedOptions) error {
342-
v2Store := cpkg.NewV2GitStore(repo)
343-
if err := v2Store.WriteCommitted(ctx, opts); err != nil {
344-
return fmt.Errorf("v2 write committed: %w", err)
345-
}
346-
return nil
347-
}
348-
349325
// getHeadCommit returns the HEAD commit object.
350326
func getHeadCommit(repo *git.Repository) (*object.Commit, error) {
351327
headRef, err := repo.Head()
@@ -378,9 +354,7 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository
378354
return repo, nil
379355
}
380356

381-
v2Only := settings.CheckpointsVersion(logCtx) == 2
382-
383-
present, readErr := checkpointPresentLocally(ctx, repo, checkpointID, v2Only)
357+
present, readErr := checkpointPresentLocally(ctx, repo, checkpointID)
384358
if readErr != nil {
385359
return repo, fmt.Errorf("failed to read checkpoint %s: %w", checkpointID, readErr)
386360
}
@@ -389,16 +363,14 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository
389363
}
390364

391365
// Missing locally — try to refresh, then re-check. Use the same fetch
392-
// chain `entire resume` uses for the active storage version (v2 refs live
393-
// under refs/entire/, not refs/heads/, so v1 and v2 need different
394-
// refspecs).
395-
freshRepo, fetchErr := refreshCheckpointRefs(ctx, v2Only)
366+
// chain `entire resume` uses for the v1 metadata branch.
367+
freshRepo, fetchErr := refreshCheckpointRefs(ctx)
396368
if fetchErr != nil {
397369
logging.Warn(logCtx, "failed to refresh metadata branch before attach; proceeding with local state",
398370
slog.String("error", fetchErr.Error()))
399371
} else {
400372
repo = freshRepo
401-
present, readErr = checkpointPresentLocally(ctx, repo, checkpointID, v2Only)
373+
present, readErr = checkpointPresentLocally(ctx, repo, checkpointID)
402374
if readErr != nil {
403375
return repo, fmt.Errorf("failed to read checkpoint %s after refresh: %w", checkpointID, readErr)
404376
}
@@ -408,42 +380,24 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository
408380
}
409381

410382
branchDescription := "entire/checkpoints/v1 branch"
411-
if v2Only {
412-
branchDescription = "v2 /main ref"
413-
}
414383
return repo, fmt.Errorf(
415384
"checkpoint %s referenced by HEAD is missing from the local %s after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first",
416-
checkpointID.String(), branchDescription, suggestCheckpointFetchCommand(logCtx, v2Only),
385+
checkpointID.String(), branchDescription, suggestCheckpointFetchCommand(logCtx),
417386
)
418387
}
419388

420-
// refreshCheckpointRefs runs the resume-equivalent fetch chain for the storage
421-
// version we're about to write to. Returns a freshly-opened repo so go-git
422-
// sees any newly-fetched packfiles and ref updates.
423-
func refreshCheckpointRefs(ctx context.Context, v2Only bool) (*git.Repository, error) {
424-
if v2Only {
425-
_, repo, err := getV2MetadataTree(ctx)
426-
return repo, err
427-
}
389+
// refreshCheckpointRefs runs the resume-equivalent fetch chain for the v1
390+
// metadata branch. Returns a freshly-opened repo so go-git sees any
391+
// newly-fetched packfiles and ref updates.
392+
func refreshCheckpointRefs(ctx context.Context) (*git.Repository, error) {
428393
_, repo, err := getMetadataTree(ctx)
429394
return repo, err
430395
}
431396

432397
// checkpointPresentLocally reports whether the checkpoint already exists on
433-
// the local ref we would write to. For v1 / dual-write, that's the local
434-
// entire/checkpoints/v1 branch (remote-tracking alone is not enough — see
435-
// ensureCheckpointAvailable). For v2-only mode, it's the v2 /main ref, which
436-
// has no remote-tracking analog and is therefore already local-only by
437-
// construction.
438-
func checkpointPresentLocally(ctx context.Context, repo *git.Repository, checkpointID id.CheckpointID, v2Only bool) (bool, error) {
439-
if v2Only {
440-
summary, err := cpkg.NewV2GitStore(repo).ReadCommitted(ctx, checkpointID)
441-
if err != nil {
442-
return false, err //nolint:wrapcheck // Caller wraps with checkpoint ID context
443-
}
444-
return summary != nil, nil
445-
}
446-
398+
// the local v1 ref we would write to. Remote-tracking alone is not enough;
399+
// see ensureCheckpointAvailable.
400+
func checkpointPresentLocally(ctx context.Context, repo *git.Repository, checkpointID id.CheckpointID) (bool, error) {
447401
localRef := plumbing.NewBranchReferenceName(paths.MetadataBranchName)
448402
if _, err := repo.Reference(localRef, true); err != nil {
449403
// Local branch ref doesn't exist — treat as "not present locally".
@@ -459,14 +413,9 @@ func checkpointPresentLocally(ctx context.Context, repo *git.Repository, checkpo
459413
}
460414

461415
// suggestCheckpointFetchCommand returns a git fetch command the user can
462-
// paste to pull the missing metadata ref. v2 refs live under refs/entire/
463-
// (not refs/heads/), so they need an explicit fully-qualified refspec;
464-
// v1 lives on a regular branch and its short name is enough.
465-
func suggestCheckpointFetchCommand(ctx context.Context, v2Only bool) string {
416+
// paste to pull the missing v1 metadata branch.
417+
func suggestCheckpointFetchCommand(ctx context.Context) string {
466418
ref := "entire/checkpoints/v1:entire/checkpoints/v1"
467-
if v2Only {
468-
ref = paths.V2MainRefName + ":" + paths.V2MainRefName
469-
}
470419
if remote.Configured(ctx) {
471420
if url, err := remote.FetchURL(ctx); err == nil && url != "" {
472421
return fmt.Sprintf("git fetch %s %s", url, ref)

‎cmd/entire/cli/attach_test.go‎

Lines changed: 20 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -299,66 +299,16 @@ func TestAttach_OutputContainsCheckpointID(t *testing.T) {
299299
}
300300
}
301301

302-
func TestAttach_V2DualWriteEnabled(t *testing.T) {
303-
setupAttachTestRepo(t)
304-
305-
repoDir := mustGetwd(t)
306-
setAttachCheckpointsV2Enabled(t, repoDir)
307-
308-
sessionID := "test-attach-v2-dual-write"
309-
setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"create hello.txt"},"uuid":"uuid-1"}
310-
{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"tu_1","name":"Write","input":{"file_path":"hello.txt","content":"hello"}}]},"uuid":"uuid-2"}
311-
{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"tu_1","content":"wrote file"}]},"uuid":"uuid-3"}
312-
{"type":"assistant","message":{"role":"assistant","content":[{"type":"text","text":"Done."}]},"uuid":"uuid-4"}
313-
`)
314-
315-
var out bytes.Buffer
316-
if err := runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, attachOptions{Force: true}); err != nil {
317-
t.Fatalf("runAttach failed: %v", err)
318-
}
319-
320-
store, err := session.NewStateStore(context.Background())
321-
if err != nil {
322-
t.Fatal(err)
323-
}
324-
state, err := store.Load(context.Background(), sessionID)
325-
if err != nil {
326-
t.Fatal(err)
327-
}
328-
if state == nil || state.LastCheckpointID.IsEmpty() {
329-
t.Fatal("expected attach to persist a checkpoint ID")
330-
}
331-
332-
repo, err := git.PlainOpen(repoDir)
333-
if err != nil {
334-
t.Fatal(err)
335-
}
336-
337-
cpPath := state.LastCheckpointID.Path()
338-
mainCompact, found := readFileFromRef(t, repo, paths.V2MainRefName, cpPath+"/0/"+paths.CompactTranscriptFileName)
339-
if !found {
340-
t.Fatalf("expected %s on %s", paths.CompactTranscriptFileName, paths.V2MainRefName)
341-
}
342-
if !strings.Contains(mainCompact, "create hello.txt") {
343-
t.Errorf("compact transcript missing prompt, got:\n%s", mainCompact)
344-
}
345-
346-
fullTranscript, found := readFileFromRef(t, repo, paths.V2FullCurrentRefName, cpPath+"/0/"+paths.V2RawTranscriptFileName)
347-
if !found {
348-
t.Fatalf("expected %s on %s", paths.V2RawTranscriptFileName, paths.V2FullCurrentRefName)
349-
}
350-
if !strings.Contains(fullTranscript, "hello.txt") {
351-
t.Errorf("raw transcript missing file content, got:\n%s", fullTranscript)
352-
}
353-
}
354-
355-
func TestAttach_CheckpointsVersion2(t *testing.T) {
302+
// TestAttach_CheckpointsVersion2_FallsBackToV1 verifies that
303+
// strategy_options.checkpoints_version: 2 is now ignored — attach writes
304+
// v1 metadata only, and never creates v2 refs solely because of the setting.
305+
func TestAttach_CheckpointsVersion2_FallsBackToV1(t *testing.T) {
356306
setupAttachTestRepo(t)
357307

358308
repoDir := mustGetwd(t)
359309
setAttachCheckpointsV2Only(t, repoDir)
360310

361-
sessionID := "test-attach-v2-only"
311+
sessionID := "test-attach-v2-disallowed"
362312
setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"create hello.txt"},"uuid":"uuid-1"}
363313
{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"tu_1","name":"Write","input":{"file_path":"hello.txt","content":"hello"}}]},"uuid":"uuid-2"}
364314
{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"tu_1","content":"wrote file"}]},"uuid":"uuid-3"}
@@ -388,53 +338,12 @@ func TestAttach_CheckpointsVersion2(t *testing.T) {
388338
}
389339

390340
cpPath := state.LastCheckpointID.Path()
391-
if _, found := readFileFromRef(t, repo, paths.MetadataBranchName, cpPath+"/"+paths.MetadataFileName); found {
392-
t.Fatalf("did not expect %s metadata for %s when checkpoints_version is 2", paths.MetadataBranchName, cpPath)
393-
}
394-
395-
mainCompact, found := readFileFromRef(t, repo, paths.V2MainRefName, cpPath+"/0/"+paths.CompactTranscriptFileName)
396-
if !found {
397-
t.Fatalf("expected %s on %s", paths.CompactTranscriptFileName, paths.V2MainRefName)
398-
}
399-
if !strings.Contains(mainCompact, "create hello.txt") {
400-
t.Errorf("compact transcript missing prompt, got:\n%s", mainCompact)
401-
}
402-
403-
fullTranscript, found := readFileFromRef(t, repo, paths.V2FullCurrentRefName, cpPath+"/0/"+paths.V2RawTranscriptFileName)
404-
if !found {
405-
t.Fatalf("expected %s on %s", paths.V2RawTranscriptFileName, paths.V2FullCurrentRefName)
406-
}
407-
if !strings.Contains(fullTranscript, "hello.txt") {
408-
t.Errorf("raw transcript missing file content, got:\n%s", fullTranscript)
409-
}
410-
}
411-
412-
func TestAttach_V2DualWriteDisabled(t *testing.T) {
413-
setupAttachTestRepo(t)
414-
415-
repoDir := mustGetwd(t)
416-
417-
sessionID := "test-attach-v2-disabled"
418-
setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"create hello.txt"},"uuid":"uuid-1"}
419-
{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"tu_1","name":"Write","input":{"file_path":"hello.txt","content":"hello"}}]},"uuid":"uuid-2"}
420-
{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"tu_1","content":"wrote file"}]},"uuid":"uuid-3"}
421-
{"type":"assistant","message":{"role":"assistant","content":[{"type":"text","text":"Done."}]},"uuid":"uuid-4"}
422-
`)
423-
424-
var out bytes.Buffer
425-
if err := runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, attachOptions{Force: true}); err != nil {
426-
t.Fatalf("runAttach failed: %v", err)
427-
}
428-
429-
repo, err := git.PlainOpen(repoDir)
430-
if err != nil {
431-
t.Fatal(err)
341+
v1Ref := string(plumbing.NewBranchReferenceName(paths.MetadataBranchName))
342+
if _, found := readFileFromRef(t, repo, v1Ref, cpPath+"/"+paths.MetadataFileName); !found {
343+
t.Fatalf("expected v1 metadata at %s for %s — checkpoints_version: 2 should fall back to v1", paths.MetadataBranchName, cpPath)
432344
}
433345
if _, err := repo.Reference(plumbing.ReferenceName(paths.V2MainRefName), true); err == nil {
434-
t.Fatalf("did not expect %s when checkpoints_v2 is disabled", paths.V2MainRefName)
435-
}
436-
if _, err := repo.Reference(plumbing.ReferenceName(paths.V2FullCurrentRefName), true); err == nil {
437-
t.Fatalf("did not expect %s when checkpoints_v2 is disabled", paths.V2FullCurrentRefName)
346+
t.Fatalf("did not expect %s when checkpoints_version: 2 is configured but disallowed", paths.V2MainRefName)
438347
}
439348
}
440349

@@ -622,36 +531,32 @@ func TestAttach_RefusesWhenCheckpointOnlyInRemoteTrackingRef(t *testing.T) {
622531
}
623532
}
624533

625-
// In v2-only mode, the refuse hint must reference the v2 /main ref and
626-
// its fully-qualified refspec (refs/entire/checkpoints/v2/main lives under
627-
// refs/entire/, not refs/heads/, so a short refspec won't resolve).
628-
func TestAttach_RefuseHint_V2Only(t *testing.T) {
534+
// TestAttach_RefuseHint_CheckpointsVersion2IgnoredFallsBackToV1 verifies that
535+
// strategy_options.checkpoints_version: 2 no longer flips attach into v2-only
536+
// mode — the refuse hint references the v1 metadata branch, not the v2 /main
537+
// ref, because v2 is disallowed and the system falls back to v1.
538+
func TestAttach_RefuseHint_CheckpointsVersion2IgnoredFallsBackToV1(t *testing.T) {
629539
setupAttachTestRepo(t)
630540

631541
repoRoot := mustGetwd(t)
632542
setAttachCheckpointsV2Only(t, repoRoot)
633543

634544
runGitInDir(t, repoRoot, "commit", "--amend", "-m", "init\n\nEntire-Checkpoint: ffffffffeeee")
635545

636-
sessionID := "v2-orphaned-attach"
546+
sessionID := "v2-disallowed-attach"
637547
setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"hi"},"uuid":"u1"}
638548
`)
639549

640550
var out bytes.Buffer
641551
err := runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, attachOptions{Force: true})
642552
if err == nil {
643-
t.Fatal("expected v2-only attach to refuse when checkpoint is missing")
553+
t.Fatal("expected attach to refuse when checkpoint is missing")
644554
}
645-
if !strings.Contains(err.Error(), "missing from the local v2 /main ref") {
646-
t.Errorf("error should describe the v2 /main ref; got: %v", err)
647-
}
648-
v2Refspec := paths.V2MainRefName + ":" + paths.V2MainRefName
649-
if !strings.Contains(err.Error(), v2Refspec) {
650-
t.Errorf("error should include v2 refspec %q; got: %v", v2Refspec, err)
555+
if !strings.Contains(err.Error(), "missing from the local entire/checkpoints/v1 branch") {
556+
t.Errorf("error should describe the v1 branch (v2 is disallowed); got: %v", err)
651557
}
652-
// And must NOT suggest the v1 refspec.
653-
if strings.Contains(err.Error(), "entire/checkpoints/v1:entire/checkpoints/v1") {
654-
t.Errorf("v2-only hint should not reference the v1 branch; got: %v", err)
558+
if !strings.Contains(err.Error(), "entire/checkpoints/v1:entire/checkpoints/v1") {
559+
t.Errorf("error should include v1 refspec; got: %v", err)
655560
}
656561
}
657562

@@ -1627,18 +1532,6 @@ func enableEntire(t *testing.T, repoDir string) {
16271532
}
16281533
}
16291534

1630-
func setAttachCheckpointsV2Enabled(t *testing.T, repoDir string) {
1631-
t.Helper()
1632-
entireDir := filepath.Join(repoDir, ".entire")
1633-
if err := os.MkdirAll(entireDir, 0o750); err != nil {
1634-
t.Fatal(err)
1635-
}
1636-
settingsContent := `{"enabled": true, "strategy_options": {"checkpoints_v2": true}}`
1637-
if err := os.WriteFile(filepath.Join(entireDir, "settings.json"), []byte(settingsContent), 0o600); err != nil {
1638-
t.Fatal(err)
1639-
}
1640-
}
1641-
16421535
func setAttachCheckpointsV2Only(t *testing.T, repoDir string) {
16431536
t.Helper()
16441537
entireDir := filepath.Join(repoDir, ".entire")

0 commit comments

Comments
 (0)