Guard entire attach against overwriting checkpoints from other machines

cmd/entire/cli/attach.go

@@ -20,6 +20,7 @@ import (
 	"github.com/entireio/cli/cmd/entire/cli/checkpoint/remote"
 	"github.com/entireio/cli/cmd/entire/cli/interactive"
 	"github.com/entireio/cli/cmd/entire/cli/logging"
+	"github.com/entireio/cli/cmd/entire/cli/paths"
 	"github.com/entireio/cli/cmd/entire/cli/session"
 	"github.com/entireio/cli/cmd/entire/cli/settings"
 	"github.com/entireio/cli/cmd/entire/cli/strategy"
@@ -31,6 +32,7 @@ import (
 
 	"github.com/charmbracelet/huh"
 	"github.com/go-git/go-git/v6"
+	"github.com/go-git/go-git/v6/plumbing"
 	"github.com/go-git/go-git/v6/plumbing/object"
 	"github.com/spf13/cobra"
 )
@@ -79,6 +81,7 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ
 		// Init failed — logging will use stderr fallback, non-fatal.
 		_ = err
 	}
+	defer logging.Close()
 
 	logCtx := logging.WithComponent(ctx, "attach")
 
@@ -135,6 +138,14 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ
 	// Determine checkpoint ID: reuse from HEAD if one exists, otherwise generate new.
 	checkpointID, isExistingCheckpoint := resolveCheckpointID(headCommit)
 
+	// If HEAD references an existing checkpoint, make sure we have it locally
+	// before writing — otherwise we'd create a fresh session 0 under the same
+	// ID and overwrite the original on push.
+	repo, err = ensureCheckpointAvailable(ctx, logCtx, repo, checkpointID, isExistingCheckpoint)
+	if err != nil {
+		return err
+	}
+
 	// Write directly to entire/checkpoints/v1.
 	store := cpkg.NewGitStore(repo)
 
@@ -244,6 +255,107 @@ func getHeadCommit(repo *git.Repository) (*object.Commit, error) {
 	return commit, nil
 }
 
+// ensureCheckpointAvailable makes sure the checkpoint referenced by HEAD is
+// present locally before the attach writes to it. Without this guard, attach
+// would create a fresh session 0 under the same ID and overwrite the original
+// session data on push.
+//
+// Only the local branch counts — remote-tracking presence is not enough.
+// If only the remote-tracking ref exists, a subsequent WriteCommitted creates
+// a brand-new orphan local branch with an empty tree, which would clobber
+// the remote on push.
+//
+// Fast path: check local refs directly — no network. If missing, trigger the
+// metadata fetch fallback chain used by `entire resume` (which advances the
+// local ref on success) and re-check. Returns a possibly-freshly-opened repo
+// handle so go-git sees any newly fetched packfiles.
+func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository, checkpointID id.CheckpointID, isExistingCheckpoint bool) (*git.Repository, error) {
+	if !isExistingCheckpoint {
+		return repo, nil
+	}
+
+	v2Only := settings.CheckpointsVersion(logCtx) == 2
+
+	present, readErr := checkpointPresentLocally(ctx, repo, checkpointID, v2Only)
+	if readErr != nil {
+		return repo, fmt.Errorf("failed to read checkpoint %s: %w", checkpointID, readErr)
+	}
+	if present {
+		return repo, nil
+	}
+
+	// Missing locally — try to refresh, then re-check.
+	if _, freshRepo, fetchErr := getMetadataTree(ctx); fetchErr != nil {
+		logging.Warn(logCtx, "failed to refresh metadata branch before attach; proceeding with local state",
+			slog.String("error", fetchErr.Error()))
+	} else {
+		repo = freshRepo
+		present, readErr = checkpointPresentLocally(ctx, repo, checkpointID, v2Only)
+		if readErr != nil {
+			return repo, fmt.Errorf("failed to read checkpoint %s after refresh: %w", checkpointID, readErr)
+		}
+		if present {
+			return repo, nil
+		}
+	}
+
+	branchDescription := "entire/checkpoints/v1 branch"
+	if v2Only {
+		branchDescription = "v2 /main ref"
+	}
+	return repo, fmt.Errorf(
+		"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",
+		checkpointID.String(), branchDescription, suggestCheckpointFetchCommand(logCtx),
+	)
+}
+
+// checkpointPresentLocally reports whether the checkpoint already exists on
+// the local ref we would write to. For v1 / dual-write, that's the local
+// entire/checkpoints/v1 branch (remote-tracking alone is not enough — see
+// ensureCheckpointAvailable). For v2-only mode, it's the v2 /main ref, which
+// has no remote-tracking analog and is therefore already local-only by
+// construction.
+func checkpointPresentLocally(ctx context.Context, repo *git.Repository, checkpointID id.CheckpointID, v2Only bool) (bool, error) {
+	if v2Only {
+		v2URL, urlErr := remote.FetchURL(ctx)
+		if urlErr != nil {
+			logging.Debug(ctx, "attach: using origin for v2 store fetch remote",
+				slog.String("error", urlErr.Error()),
+			)
+		}
+		summary, err := cpkg.NewV2GitStore(repo, v2URL).ReadCommitted(ctx, checkpointID)
+		if err != nil {
+			return false, err //nolint:wrapcheck // Caller wraps with checkpoint ID context
+		}
+		return summary != nil, nil
+	}
+
+	localRef := plumbing.NewBranchReferenceName(paths.MetadataBranchName)
+	if _, err := repo.Reference(localRef, true); err != nil {
+		// Local branch ref doesn't exist — treat as "not present locally".
+		// We deliberately do not fall back to remote-tracking: see
+		// ensureCheckpointAvailable's docstring.
+		return false, nil //nolint:nilerr // Missing ref is the "absent" signal, not an error.
+	}
+	summary, err := cpkg.NewGitStore(repo).ReadCommitted(ctx, checkpointID)
+	if err != nil {
+		return false, err //nolint:wrapcheck // Caller wraps with checkpoint ID context
+	}
+	return summary != nil, nil
+}
+
+// suggestCheckpointFetchCommand returns a git fetch command string the user
+// can paste, aware of whether checkpoint_remote is configured.
+func suggestCheckpointFetchCommand(ctx context.Context) string {
+	const ref = "entire/checkpoints/v1:entire/checkpoints/v1"
+	if remote.Configured(ctx) {
+		if url, err := remote.FetchURL(ctx); err == nil && url != "" {
+			return fmt.Sprintf("git fetch %s %s", url, ref)
+		}
+	}
+	return "git fetch origin " + ref
+}
+
 // resolveCheckpointID returns the checkpoint ID to use for the attach.
 // If HEAD already has an Entire-Checkpoint trailer, reuses that ID (the session
 // gets added as an additional session in the existing checkpoint).

cmd/entire/cli/attach_test.go

@@ -18,9 +18,13 @@ import (
 	_ "github.com/entireio/cli/cmd/entire/cli/agent/factoryaidroid" // register agent
 	_ "github.com/entireio/cli/cmd/entire/cli/agent/geminicli"      // register agent
 	"github.com/entireio/cli/cmd/entire/cli/agent/types"
+	cpkg "github.com/entireio/cli/cmd/entire/cli/checkpoint"
+	"github.com/entireio/cli/cmd/entire/cli/checkpoint/id"
 	"github.com/entireio/cli/cmd/entire/cli/paths"
 	"github.com/entireio/cli/cmd/entire/cli/session"
 	"github.com/entireio/cli/cmd/entire/cli/testutil"
+	"github.com/entireio/cli/cmd/entire/cli/trailers"
+	"github.com/entireio/cli/redact"
 
 	"github.com/go-git/go-git/v6"
 	"github.com/go-git/go-git/v6/plumbing"
@@ -255,6 +259,190 @@ func TestAttach_V2DualWriteDisabled(t *testing.T) {
 	}
 }
 
+func TestAttach_AppendsAsAdditionalSessionWhenIDDiffers(t *testing.T) {
+	setupAttachTestRepo(t)
+
+	firstSessionID := "first-session-a-original"
+	setupClaudeTranscript(t, firstSessionID, `{"type":"user","message":{"role":"user","content":"first"},"uuid":"u1"}
+`)
+	var out bytes.Buffer
+	if err := runAttach(context.Background(), &out, firstSessionID, agent.AgentNameClaudeCode, true); err != nil {
+		t.Fatalf("first attach failed: %v", err)
+	}
+
+	repoRoot := mustGetwd(t)
+	repo, err := git.PlainOpen(repoRoot)
+	if err != nil {
+		t.Fatal(err)
+	}
+	headRef, err := repo.Head()
+	if err != nil {
+		t.Fatal(err)
+	}
+	headCommit, err := repo.CommitObject(headRef.Hash())
+	if err != nil {
+		t.Fatal(err)
+	}
+	existingCheckpoints := trailers.ParseAllCheckpoints(headCommit.Message)
+	if len(existingCheckpoints) != 1 {
+		t.Fatalf("expected one Entire-Checkpoint trailer after first attach; got %v", existingCheckpoints)
+	}
+	checkpointID := existingCheckpoints[0]
+
+	secondSessionID := "second-session-b-append"
+	setupClaudeTranscript(t, secondSessionID, `{"type":"user","message":{"role":"user","content":"second"},"uuid":"u1"}
+`)
+	out.Reset()
+	if err := runAttach(context.Background(), &out, secondSessionID, agent.AgentNameClaudeCode, true); err != nil {
+		t.Fatalf("second attach failed: %v", err)
+	}
+
+	store := cpkg.NewGitStore(repo)
+	summary, err := store.ReadCommitted(context.Background(), checkpointID)
+	if err != nil {
+		t.Fatalf("ReadCommitted(%s): %v", checkpointID, err)
+	}
+	if summary == nil {
+		t.Fatalf("checkpoint %s summary nil after two attaches", checkpointID)
+	}
+	if len(summary.Sessions) != 2 {
+		t.Fatalf("checkpoint has %d sessions, want 2", len(summary.Sessions))
+	}
+
+	idx0, err := store.ReadSessionContent(context.Background(), checkpointID, 0)
+	if err != nil {
+		t.Fatalf("ReadSessionContent(0): %v", err)
+	}
+	idx1, err := store.ReadSessionContent(context.Background(), checkpointID, 1)
+	if err != nil {
+		t.Fatalf("ReadSessionContent(1): %v", err)
+	}
+	haveFirst := idx0.Metadata.SessionID == firstSessionID || idx1.Metadata.SessionID == firstSessionID
+	haveSecond := idx0.Metadata.SessionID == secondSessionID || idx1.Metadata.SessionID == secondSessionID
+	if !haveFirst {
+		t.Errorf("first session %q missing from checkpoint; got [%q, %q]",
+			firstSessionID, idx0.Metadata.SessionID, idx1.Metadata.SessionID)
+	}
+	if !haveSecond {
+		t.Errorf("second session %q missing from checkpoint; got [%q, %q]",
+			secondSessionID, idx0.Metadata.SessionID, idx1.Metadata.SessionID)
+	}
+}
+
+func TestAttach_RefusesWhenCheckpointMissingFromLocalBranch(t *testing.T) {
+	setupAttachTestRepo(t)
+
+	repoRoot := mustGetwd(t)
+	runGitInDir(t, repoRoot, "commit", "--amend", "-m", "init\n\nEntire-Checkpoint: ffffffffeeee")
+
+	sessionID := "orphaned-attach-session"
+	setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"attach please"},"uuid":"u1"}
+`)
+
+	var out bytes.Buffer
+	err := runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, true)
+	if err == nil {
+		t.Fatal("expected error: checkpoint referenced by HEAD is missing locally and attach should refuse")
+	}
+	if !strings.Contains(err.Error(), "missing from the local entire/checkpoints/v1 branch") {
+		t.Errorf("error message should explain the missing-branch situation; got: %v", err)
+	}
+	if !strings.Contains(err.Error(), "git fetch origin entire/checkpoints/v1") {
+		t.Errorf("error message should include the fetch command to fix it; got: %v", err)
+	}
+
+	repo, err := git.PlainOpen(repoRoot)
+	if err != nil {
+		t.Fatal(err)
+	}
+	store := cpkg.NewGitStore(repo)
+	summary, err := store.ReadCommitted(context.Background(), "ffffffffeeee")
+	if err != nil {
+		t.Fatalf("ReadCommitted: %v", err)
+	}
+	if summary != nil {
+		t.Errorf("attach should NOT have created checkpoint ffffffffeeee locally; found %+v", summary)
+	}
+}
+
+// Regression for https://github.com/entireio/cli/pull/1014#pullrequestreview-copilot:
+// Bob clones a repo where Alice's checkpoint is on the remote-tracking ref
+// (refs/remotes/origin/entire/checkpoints/v1) but the local branch doesn't
+// exist yet. ReadCommitted falls back to the remote-tracking tree, so a naive
+// "read and check" guard would think all is well. But WriteCommitted would
+// then create a *fresh* orphan local branch, and Bob's push would clobber
+// Alice's data on origin. Attach must refuse in this shape.
+func TestAttach_RefusesWhenCheckpointOnlyInRemoteTrackingRef(t *testing.T) {
+	setupAttachTestRepo(t)
+
+	repoRoot := mustGetwd(t)
+	repo, err := git.PlainOpen(repoRoot)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Seed the local branch with a checkpoint representing Alice's session.
+	alicesCheckpoint := id.MustCheckpointID("abcdef012345")
+	store := cpkg.NewGitStore(repo)
+	if writeErr := store.WriteCommitted(context.Background(), cpkg.WriteCommittedOptions{
+		CheckpointID: alicesCheckpoint,
+		SessionID:    "alice-original",
+		Strategy:     "manual-commit",
+		Transcript:   redact.AlreadyRedacted([]byte(`{"type":"user","message":"hi"}` + "\n")),
+		AuthorName:   "Alice",
+		AuthorEmail:  "[email protected]",
+	}); writeErr != nil {
+		t.Fatalf("WriteCommitted: %v", writeErr)
+	}
+
+	// Move the populated branch to the remote-tracking ref, then delete the
+	// local ref. This is the shape `git clone` produces for a branch the user
+	// never explicitly checked out locally.
+	localRef := plumbing.NewBranchReferenceName(paths.MetadataBranchName)
+	remoteTrackingRef := plumbing.NewRemoteReferenceName("origin", paths.MetadataBranchName)
+	populated, err := repo.Reference(localRef, true)
+	if err != nil {
+		t.Fatalf("reading local ref: %v", err)
+	}
+	if setErr := repo.Storer.SetReference(plumbing.NewHashReference(remoteTrackingRef, populated.Hash())); setErr != nil {
+		t.Fatalf("SetReference remote-tracking: %v", setErr)
+	}
+	if rmErr := repo.Storer.RemoveReference(localRef); rmErr != nil {
+		t.Fatalf("RemoveReference local: %v", rmErr)
+	}
+
+	// Amend HEAD so attach treats this as an existing-checkpoint case.
+	runGitInDir(t, repoRoot, "commit", "--amend", "-m", "init\n\nEntire-Checkpoint: "+alicesCheckpoint.String())
+
+	sessionID := "bob-attempted-attach"
+	setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"hi"},"uuid":"u1"}
+`)
+
+	var out bytes.Buffer
+	err = runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, true)
+	if err == nil {
+		t.Fatal("expected attach to refuse when checkpoint is only in the remote-tracking ref")
+	}
+	if !strings.Contains(err.Error(), "missing from the local entire/checkpoints/v1 branch") {
+		t.Errorf("error should explain the local-branch gap; got: %v", err)
+	}
+
+	// Local branch must still not exist — attach should not have created a
+	// fresh orphan on refuse.
+	if _, refErr := repo.Reference(localRef, true); refErr == nil {
+		t.Error("local entire/checkpoints/v1 branch was created despite refuse; would clobber remote on push")
+	}
+
+	// Remote-tracking ref must still hold Alice's untouched data.
+	remoteRef, err := repo.Reference(remoteTrackingRef, true)
+	if err != nil {
+		t.Fatalf("remote-tracking ref missing: %v", err)
+	}
+	if remoteRef.Hash() != populated.Hash() {
+		t.Errorf("remote-tracking ref moved from %s to %s", populated.Hash(), remoteRef.Hash())
+	}
+}
+
 func TestAttach_PopulatesTokenUsage(t *testing.T) {
 	setupAttachTestRepo(t)
 
@@ -847,3 +1035,12 @@ func TestAttach_DiscoversExternalAgents(t *testing.T) {
 		t.Errorf("expected external agent %q in registry after attach, got: %v", agentName, lookupErr)
 	}
 }
+
+func runGitInDir(t *testing.T, dir string, args ...string) {
+	t.Helper()
+	cmd := exec.CommandContext(context.Background(), "git", args...)
+	cmd.Dir = dir
+	if out, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("git %v in %s: %v\n%s", args, dir, err, out)
+	}
+}