Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/entire/cli/strategy/checkpoint_remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 25 additions & 6 deletions cmd/entire/cli/strategy/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)),
Comment on lines 146 to +159
slog.String("local", currentLocal.Hash().String()),
slog.String("target", targetHash.String()),
)
return nil
}
}

newRef := plumbing.NewHashReference(localRefName, targetHash)
Expand Down
183 changes: 183 additions & 0 deletions cmd/entire/cli/strategy/safely_advance_local_ref_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
Loading