diff --git a/cmd/entire/cli/resume.go b/cmd/entire/cli/resume.go index f2c9000dfa..1b84657517 100644 --- a/cmd/entire/cli/resume.go +++ b/cmd/entire/cli/resume.go @@ -845,12 +845,13 @@ func checkRemoteMetadata(ctx context.Context, w, errW io.Writer, checkpointID id return nil } +// promoteRemoteTrackingMetadataBranch advances the local entire/checkpoints/v1 +// ref to match origin's remote-tracking ref. Without this, callers reading +// checkpoint metadata via the local ref miss checkpoints already fetched into +// refs/remotes/origin/...: the committed-checkpoint store only falls back to +// origin/... when the local ref is *missing*, not when it's behind. func promoteRemoteTrackingMetadataBranch(ctx context.Context, repo *git.Repository) { localRefName := plumbing.NewBranchReferenceName(paths.MetadataBranchName) - if _, err := repo.Reference(localRefName, true); err == nil { - return - } - remoteRef, err := repo.Reference(plumbing.NewRemoteReferenceName("origin", paths.MetadataBranchName), true) if err != nil { return diff --git a/cmd/entire/cli/resume_test.go b/cmd/entire/cli/resume_test.go index 453fb09296..dd8f9d09f1 100644 --- a/cmd/entire/cli/resume_test.go +++ b/cmd/entire/cli/resume_test.go @@ -921,6 +921,120 @@ func TestCheckRemoteMetadata_CheckpointNotOnRemote(t *testing.T) { } } +// makeLocalMetadataBranchStale advances origin/entire/checkpoints/v1 to the +// current local hash and rewinds the local ref back to baseHash, leaving the +// local metadata branch behind its remote-tracking counterpart. Returns the +// hash the remote-tracking ref now points at. +func makeLocalMetadataBranchStale(t *testing.T, repo *git.Repository, baseHash plumbing.Hash) plumbing.Hash { + t.Helper() + localRefName := plumbing.NewBranchReferenceName(paths.MetadataBranchName) + current, err := repo.Reference(localRefName, true) + if err != nil { + t.Fatalf("read advanced metadata branch ref: %v", err) + } + if current.Hash() == baseHash { + t.Fatalf("makeLocalMetadataBranchStale: local ref must have advanced past baseHash before calling") + } + remoteRefName := plumbing.NewRemoteReferenceName("origin", paths.MetadataBranchName) + if err := repo.Storer.SetReference(plumbing.NewHashReference(remoteRefName, current.Hash())); err != nil { + t.Fatalf("set remote-tracking ref: %v", err) + } + if err := repo.Storer.SetReference(plumbing.NewHashReference(localRefName, baseHash)); err != nil { + t.Fatalf("rewind local metadata branch: %v", err) + } + return current.Hash() +} + +// readMetadataBranchHash returns the current hash of refs/heads/entire/checkpoints/v1. +func readMetadataBranchHash(t *testing.T, repo *git.Repository) plumbing.Hash { + t.Helper() + ref, err := repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) + if err != nil { + t.Fatalf("read metadata branch ref: %v", err) + } + return ref.Hash() +} + +// Before the fix, promoteRemoteTrackingMetadataBranch returned early whenever +// the local ref existed, even when it was behind the remote-tracking ref — +// so downstream metadata readers using the local ref missed checkpoints +// already fetched into refs/remotes/origin/... +func TestPromoteRemoteTrackingMetadataBranch_FastForwardsStaleLocal(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + repo, _, _ := setupResumeTestRepo(t, tmpDir, false) + + initialHash := readMetadataBranchHash(t, repo) + _ = createCheckpointOnMetadataBranch(t, repo, "2025-01-01-test-session-uuid") + descendantHash := makeLocalMetadataBranchStale(t, repo, initialHash) + + promoteRemoteTrackingMetadataBranch(context.Background(), repo) + + if got := readMetadataBranchHash(t, repo); got != descendantHash { + t.Errorf("local should be fast-forwarded to remote-tracking ref: got %s, want %s", got, descendantHash) + } +} + +// End-to-end coverage for the same bug: when a fresh checkpoint has been +// pushed to origin but the user's local entire/checkpoints/v1 ref is behind, +// `entire resume` previously printed "session log not available" because the +// committed-checkpoint reader only falls back to origin/... when the local +// ref is missing entirely. +func TestResumeFromCurrentBranch_FastForwardsStaleLocalMetadata(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + t.Setenv("ENTIRE_TEST_CLAUDE_PROJECT_DIR", filepath.Join(tmpDir, "claude-projects")) + + repo, w, _ := setupResumeTestRepo(t, tmpDir, false) + initialHash := readMetadataBranchHash(t, repo) + + ctx := context.Background() + cpID := id.MustCheckpointID("abc123def456") + rawTranscript := []byte(`{"type":"user","message":{"content":[{"type":"text","text":"hi"}]}}` + "\n") + + // Agent must be set so RestoreLogsOnly can resolve a session-write target. + v1Store := checkpoint.NewGitStore(repo) + if err := v1Store.WriteCommitted(ctx, checkpoint.WriteCommittedOptions{ + CheckpointID: cpID, + SessionID: "2025-01-01-test-session-uuid", + Strategy: "manual-commit", + Transcript: redact.AlreadyRedacted(rawTranscript), + Agent: agent.AgentTypeClaudeCode, + AuthorName: "Test", + AuthorEmail: "test@example.com", + }); err != nil { + t.Fatalf("WriteCommitted: %v", err) + } + + _ = makeLocalMetadataBranchStale(t, repo, initialHash) + + featureFile := filepath.Join(tmpDir, "feature.txt") + if err := os.WriteFile(featureFile, []byte("feature content"), 0o644); err != nil { + t.Fatalf("write feature file: %v", err) + } + if _, err := w.Add("feature.txt"); err != nil { + t.Fatalf("add feature file: %v", err) + } + if _, err := w.Commit("Add feature\n\nEntire-Checkpoint: "+cpID.String(), &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com"}, + }); err != nil { + t.Fatalf("Commit: %v", err) + } + + var stdout, stderr bytes.Buffer + if err := resumeFromCurrentBranch(ctx, &stdout, &stderr, "master", true); err != nil { + t.Fatalf("resumeFromCurrentBranch error: %v\nstdout: %s\nstderr: %s", + err, stdout.String(), stderr.String()) + } + + combined := stdout.String() + stderr.String() + if strings.Contains(combined, "session log not available") { + t.Errorf("resume reported missing log even though origin has the checkpoint metadata:\n%s", combined) + } +} + func TestResumeFromCurrentBranch_NoMetadataAvailable(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) diff --git a/cmd/entire/cli/strategy/checkpoint_remote_test.go b/cmd/entire/cli/strategy/checkpoint_remote_test.go index 08e815cb3a..4bdd6fceaf 100644 --- a/cmd/entire/cli/strategy/checkpoint_remote_test.go +++ b/cmd/entire/cli/strategy/checkpoint_remote_test.go @@ -918,10 +918,12 @@ func TestFetchV2MainFromURL_UpdatesExistingRef(t *testing.T) { require.NoError(t, err) hash1 := strings.TrimSpace(string(hash1Out)) - // Add a second commit on the remote's v2 ref - createV2MainRef(ctx, t, remoteDir) // Creates a new orphan commit, updating the ref + // Advance the remote's v2 /main with a child commit so the new tip is a + // strict descendant of hash1 (the realistic CLI flow — condensation + // appends a new checkpoint commit on top of the previous tip). + advanceV2MainOnTop(ctx, t, remoteDir, hash1) - // Fetch again — should update + // Fetch again — should fast-forward require.NoError(t, FetchV2MainFromURL(ctx, remoteDir)) hashCmd = exec.CommandContext(ctx, "git", "rev-parse", paths.V2MainRefName) diff --git a/cmd/entire/cli/strategy/common.go b/cmd/entire/cli/strategy/common.go index ce49de7d1e..104a138427 100644 --- a/cmd/entire/cli/strategy/common.go +++ b/cmd/entire/cli/strategy/common.go @@ -126,13 +126,24 @@ func PromoteTmpRefSafely(ctx context.Context, tmpRefName, destRefName plumbing.R return nil } -// SafelyAdvanceLocalRef updates localRefName to point at targetHash, except -// when the existing local ref is already at or ahead of targetHash. In that -// case it leaves the local ref unchanged to avoid rewinding locally-ahead -// work. Otherwise (local missing, behind, or diverged) it updates the ref to -// targetHash. +// SafelyAdvanceLocalRef updates localRefName to point at targetHash only when +// the move is non-destructive: // -// The ancestry check walks from the local ref (which has full history), so +// - local missing → create at target +// - local == target → no-op +// - local at or ahead of target (target is an ancestor of local) → no-op +// - local strictly behind target (local is an ancestor of target) → fast-forward +// - diverged or unrelated history (neither ref is an ancestor of the other) +// → no-op, logged at debug level +// +// The diverged case is the dangerous one this guard exists for. The CLI +// maintains orphan-style refs (entire/checkpoints/v1, the V2 main ref) where +// unpushed local commits encode user work — a fetch that landed sibling +// commits from another machine must not silently rewind that work. In the +// diverged case readers fall through to the remote-tracking tree or to v1, +// so resume keeps working while local-only commits stay reachable. +// +// The ancestry checks walk from the local ref (which has full history), so // callers that fetched with --depth=1 do not break the check. func SafelyAdvanceLocalRef(ctx context.Context, repo *git.Repository, localRefName plumbing.ReferenceName, targetHash plumbing.Hash) error { currentLocal, localErr := repo.Reference(localRefName, true) @@ -143,6 +154,14 @@ func SafelyAdvanceLocalRef(ctx context.Context, repo *git.Repository, localRefNa if IsAncestorOf(ctx, repo, targetHash, currentLocal.Hash()) { return nil } + if !IsAncestorOf(ctx, repo, currentLocal.Hash(), targetHash) { + logging.Debug(ctx, "skipping advance: local ref has diverged from target", + slog.String("ref", string(localRefName)), + slog.String("local", currentLocal.Hash().String()), + slog.String("target", targetHash.String()), + ) + return nil + } } newRef := plumbing.NewHashReference(localRefName, targetHash) diff --git a/cmd/entire/cli/strategy/safely_advance_local_ref_test.go b/cmd/entire/cli/strategy/safely_advance_local_ref_test.go new file mode 100644 index 0000000000..57d650a815 --- /dev/null +++ b/cmd/entire/cli/strategy/safely_advance_local_ref_test.go @@ -0,0 +1,183 @@ +package strategy + +import ( + "context" + "testing" + "time" + + "github.com/entireio/cli/cmd/entire/cli/testutil" + + "github.com/go-git/go-git/v6" + "github.com/go-git/go-git/v6/plumbing" + "github.com/go-git/go-git/v6/plumbing/object" +) + +const safelyAdvanceTestRef plumbing.ReferenceName = "refs/heads/safely-advance-test" + +// newSafelyAdvanceTestRepo opens an empty git repository for ref-manipulation +// tests. It uses testutil.InitRepo so author/GPG config matches the rest of +// the suite, but the tests themselves operate purely via plumbing — no +// worktree state needed. +func newSafelyAdvanceTestRepo(t *testing.T) *git.Repository { + t.Helper() + dir := t.TempDir() + testutil.InitRepo(t, dir) + repo, err := git.PlainOpen(dir) + if err != nil { + t.Fatalf("PlainOpen: %v", err) + } + return repo +} + +// makeEmptyTreeCommit writes a commit with an empty tree and the given parents +// and message. Different messages produce different hashes, so callers can +// create distinct commits (including divergent siblings) without touching +// files. +func makeEmptyTreeCommit(t *testing.T, repo *git.Repository, parents []plumbing.Hash, msg string) plumbing.Hash { + t.Helper() + emptyTree := object.Tree{} + emptyTreeObj := repo.Storer.NewEncodedObject() + if err := emptyTree.Encode(emptyTreeObj); err != nil { + t.Fatalf("encode empty tree: %v", err) + } + treeHash, err := repo.Storer.SetEncodedObject(emptyTreeObj) + if err != nil { + t.Fatalf("store empty tree: %v", err) + } + sig := object.Signature{Name: "T", Email: "t@example.com", When: time.Unix(0, 0).UTC()} + commit := &object.Commit{ + TreeHash: treeHash, + Message: msg, + Author: sig, + Committer: sig, + ParentHashes: parents, + } + obj := repo.Storer.NewEncodedObject() + if err := commit.Encode(obj); err != nil { + t.Fatalf("encode commit %q: %v", msg, err) + } + hash, err := repo.Storer.SetEncodedObject(obj) + if err != nil { + t.Fatalf("store commit %q: %v", msg, err) + } + return hash +} + +// forceSetTestRef points safelyAdvanceTestRef at hash unconditionally — +// tests deliberately rewind/diverge the ref to set up the scenarios under +// test. +func forceSetTestRef(t *testing.T, repo *git.Repository, hash plumbing.Hash) { + t.Helper() + if err := repo.Storer.SetReference(plumbing.NewHashReference(safelyAdvanceTestRef, hash)); err != nil { + t.Fatalf("SetReference %s: %v", safelyAdvanceTestRef, err) + } +} + +// readTestRef reads safelyAdvanceTestRef and fatals if it is missing. Used +// after a call to SafelyAdvanceLocalRef to inspect the resulting ref state. +func readTestRef(t *testing.T, repo *git.Repository) plumbing.Hash { + t.Helper() + ref, err := repo.Reference(safelyAdvanceTestRef, true) + if err != nil { + t.Fatalf("read %s: %v", safelyAdvanceTestRef, err) + } + return ref.Hash() +} + +func TestSafelyAdvanceLocalRef_LocalMissing_SetsToTarget(t *testing.T) { + t.Parallel() + repo := newSafelyAdvanceTestRepo(t) + a := makeEmptyTreeCommit(t, repo, nil, "A") + + if err := SafelyAdvanceLocalRef(context.Background(), repo, safelyAdvanceTestRef, a); err != nil { + t.Fatalf("SafelyAdvanceLocalRef: %v", err) + } + if got := readTestRef(t, repo); got != a { + t.Errorf("local ref = %s, want %s", got, a) + } +} + +func TestSafelyAdvanceLocalRef_LocalEqualsTarget_NoOp(t *testing.T) { + t.Parallel() + repo := newSafelyAdvanceTestRepo(t) + a := makeEmptyTreeCommit(t, repo, nil, "A") + forceSetTestRef(t, repo, a) + + if err := SafelyAdvanceLocalRef(context.Background(), repo, safelyAdvanceTestRef, a); err != nil { + t.Fatalf("SafelyAdvanceLocalRef: %v", err) + } + if got := readTestRef(t, repo); got != a { + t.Errorf("local ref = %s, want %s (unchanged)", got, a) + } +} + +func TestSafelyAdvanceLocalRef_LocalAhead_NoOp(t *testing.T) { + t.Parallel() + repo := newSafelyAdvanceTestRepo(t) + a := makeEmptyTreeCommit(t, repo, nil, "A") + b := makeEmptyTreeCommit(t, repo, []plumbing.Hash{a}, "B") + forceSetTestRef(t, repo, b) + + if err := SafelyAdvanceLocalRef(context.Background(), repo, safelyAdvanceTestRef, a); err != nil { + t.Fatalf("SafelyAdvanceLocalRef: %v", err) + } + if got := readTestRef(t, repo); got != b { + t.Errorf("locally-ahead ref must not rewind: got %s, want %s", got, b) + } +} + +func TestSafelyAdvanceLocalRef_LocalBehind_FastForwards(t *testing.T) { + t.Parallel() + repo := newSafelyAdvanceTestRepo(t) + a := makeEmptyTreeCommit(t, repo, nil, "A") + b := makeEmptyTreeCommit(t, repo, []plumbing.Hash{a}, "B") + forceSetTestRef(t, repo, a) + + if err := SafelyAdvanceLocalRef(context.Background(), repo, safelyAdvanceTestRef, b); err != nil { + t.Fatalf("SafelyAdvanceLocalRef: %v", err) + } + if got := readTestRef(t, repo); got != b { + t.Errorf("local ref should have fast-forwarded: got %s, want %s", got, b) + } +} + +// Before this fix, SafelyAdvanceLocalRef overwrote any local ref whose history +// the target couldn't reach. That destroyed unpushed local commits on orphan +// refs the CLI maintains (entire/checkpoints/v1, the V2 main ref) whenever a +// fetch landed sibling commits — e.g. checkpoint metadata produced on another +// machine. The objects survived in the loose-objects pool until git gc, but +// the branch ref no longer pointed at them. +func TestSafelyAdvanceLocalRef_Diverged_PreservesLocal(t *testing.T) { + t.Parallel() + repo := newSafelyAdvanceTestRepo(t) + base := makeEmptyTreeCommit(t, repo, nil, "base") + localTip := makeEmptyTreeCommit(t, repo, []plumbing.Hash{base}, "local-only-work") + targetTip := makeEmptyTreeCommit(t, repo, []plumbing.Hash{base}, "remote-only-work") + forceSetTestRef(t, repo, localTip) + + if err := SafelyAdvanceLocalRef(context.Background(), repo, safelyAdvanceTestRef, targetTip); err != nil { + t.Fatalf("SafelyAdvanceLocalRef: %v", err) + } + if got := readTestRef(t, repo); got != localTip { + t.Errorf("diverged local ref must be preserved: got %s, want %s (overwrote to target %s?)", + got, localTip, targetTip) + } +} + +// Unrelated histories (no common ancestor at all) are a strict subset of +// "diverged" — neither ref is an ancestor of the other. Confirms the +// protection extends to that case. +func TestSafelyAdvanceLocalRef_UnrelatedHistory_PreservesLocal(t *testing.T) { + t.Parallel() + repo := newSafelyAdvanceTestRepo(t) + localOnly := makeEmptyTreeCommit(t, repo, nil, "local-orphan") + targetOnly := makeEmptyTreeCommit(t, repo, nil, "target-orphan") + forceSetTestRef(t, repo, localOnly) + + if err := SafelyAdvanceLocalRef(context.Background(), repo, safelyAdvanceTestRef, targetOnly); err != nil { + t.Fatalf("SafelyAdvanceLocalRef: %v", err) + } + if got := readTestRef(t, repo); got != localOnly { + t.Errorf("unrelated-history local ref must be preserved: got %s, want %s", got, localOnly) + } +}