fix(repository): replace pull with fetch+reset to avoid go-git non-fast-forward bug (#863)

* fix(repository): replace Pull with Fetch+Reset to avoid go-git non-fast-forward bug

The go-git library's Pull() function has a known issue (#358) where it
returns 'non-fast-forward update' errors even when a fast-forward merge
is possible. This happens when local branches don't have proper upstream
tracking configured.

This commit replaces Pull() with an explicit Fetch() + Hard Reset approach:
1. Fetch latest changes from remote with Force=true
2. Get the remote reference for the target branch
3. Hard reset the worktree to the remote ref
4. Update the local branch reference

This approach:
- Avoids the go-git Pull() bug entirely
- Is more explicit about what we want (always match remote)
- Handles force-push scenarios correctly
- Works regardless of tracking configuration

Fixes: go-git/go-git#358

* fix: incorrect Pull target on non-default branches

---------

Co-authored-by: Obinna Odirionye <odirionye@gmail.com>
Co-authored-by: Daniel Ciaglia <daniel@sigterm.de>
Co-authored-by: Luca Corrieri <luca.corrieri@theodo.com>

Author: 3 people

internal/repository/providers/standard/repository.go

@@ -128,25 +128,39 @@ func (p *GitProvider) Bundle(ref string) ([]byte, error) {
 		return nil, fmt.Errorf("failed to checkout branch %s: %w", reference.Name(), err)
 	}
 
-	// Pull latest changes
-	pullOpts := &git.PullOptions{
+	// Fetch latest changes from remote (instead of Pull which has issues with go-git)
+	// See: https://github.com/go-git/go-git/issues/358
+	log.Infof("fetching latest changes for repo %s", p.RepoURL)
+	err = p.gitRepository.Fetch(&git.FetchOptions{
 		Auth:       p.AuthMethod,
 		RemoteName: remote,
+		Force:      true,
+	})
+	if err != nil && err != git.NoErrAlreadyUpToDate {
+		log.Warnf("failed to fetch: %v", err)
 	}
 
-	log.Infof("pulling latest changes for repo %s on branch %s", p.RepoURL, reference.Name())
-	err = worktree.Pull(pullOpts)
+	// Get the latest remote ref and hard reset to it
+	remoteRefName := getRemoteReferenceName(ref)
+	remoteRef, err := p.gitRepository.Reference(remoteRefName, true)
 	if err != nil {
-		if err == git.NoErrAlreadyUpToDate {
-			log.Info("repository is already up-to-date")
-		} else {
-			log.Warnf("failed to pull latest changes for ref %s: %v, deleting local repository", reference.Name(), err)
-			p.gitRepository = nil
-			if removeErr := os.RemoveAll(p.repositoryPath); removeErr != nil {
-				return nil, fmt.Errorf("failed to remove repository at %s: %w", p.repositoryPath, removeErr)
-			}
-			return nil, fmt.Errorf("failed to pull latest changes for ref %s: %w, likely because of force-push, next run will re-clone", reference.Name(), err)
-		}
+		return nil, fmt.Errorf("failed to get remote reference %s: %w", remoteRefName, err)
+	}
+
+	log.Infof("resetting to remote ref %s (%s)", remoteRefName, remoteRef.Hash().String())
+	err = worktree.Reset(&git.ResetOptions{
+		Commit: remoteRef.Hash(),
+		Mode:   git.HardReset,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to reset to remote ref %s: %w", remoteRefName, err)
+	}
+
+	// Update local branch reference to match remote
+	reference = plumbing.NewHashReference(localRefName, remoteRef.Hash())
+	err = p.gitRepository.Storer.SetReference(reference)
+	if err != nil {
+		return nil, fmt.Errorf("failed to update local branch reference: %w", err)
 	}
 
 	// Create git bundle

internal/repository/providers/standard/repository_test.go

@@ -0,0 +1,208 @@
+package standard
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/plumbing"
+	"github.com/go-git/go-git/v5/plumbing/object"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func testSig() *object.Signature {
+	return &object.Signature{Name: "test", Email: "test@example.com", When: time.Now()}
+}
+
+// TestBundle_NonDefaultBranch_FastForwards is a regression test for the
+// "non-fast-forward update" error when Bundle() is called on a non-default
+// branch whose Pull was resolved to the wrong remote ref.
+//
+// Root cause: Pull() without ReferenceName falls back to the remote's HEAD
+// (the default branch). When the default branch has commits not in the
+// non-default branch, this is a non-fast-forward relative to the local branch.
+//
+// Fix: always pass ReferenceName in PullOptions so go-git targets the correct
+// remote branch rather than the remote's default branch.
+func TestBundle_NonDefaultBranch_FastForwards(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git CLI not available")
+	}
+
+	// Set up a "remote" repo with diverged master and feature branches:
+	//   master:  A → B  (B has a master-only file)
+	//   feature: A → C  (C has a feature-only file; diverges from master at A)
+	remoteDir := t.TempDir()
+	remoteRepo, err := git.PlainInit(remoteDir, false)
+	require.NoError(t, err)
+
+	wt, err := remoteRepo.Worktree()
+	require.NoError(t, err)
+	sig := testSig()
+
+	// Commit A — initial commit on master
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "README.md"), []byte("initial"), 0644))
+	_, err = wt.Add("README.md")
+	require.NoError(t, err)
+	commitA, err := wt.Commit("initial commit", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+
+	// Create feature branch pointing at A (before master advances)
+	require.NoError(t, remoteRepo.Storer.SetReference(
+		plumbing.NewHashReference("refs/heads/feature", commitA),
+	))
+
+	// Commit B on master (master = A→B, feature = A)
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "master.txt"), []byte("master-only"), 0644))
+	_, err = wt.Add("master.txt")
+	require.NoError(t, err)
+	_, err = wt.Commit("advance master", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+
+	// Commit C on feature (feature = A→C, diverged from master)
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/feature"}))
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "feature.txt"), []byte("feature-v1"), 0644))
+	_, err = wt.Add("feature.txt")
+	require.NoError(t, err)
+	_, err = wt.Commit("feature commit", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+
+	// Leave remote worktree on master so the default branch is correct
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/master"}))
+
+	// Clone the remote
+	localDir := t.TempDir()
+	cloned, err := git.PlainClone(localDir, false, &git.CloneOptions{URL: remoteDir})
+	require.NoError(t, err)
+
+	// Build a GitProvider with the cloned repo injected directly (bypasses
+	// the clone() call and the hardcoded repositoryDir constant).
+	workingDir := t.TempDir()
+	p := &GitProvider{
+		RepoURL:        remoteDir,
+		gitRepository:  cloned,
+		repositoryPath: localDir,
+		workingDir:     workingDir,
+	}
+
+	// First Bundle("feature") call: local feature branch doesn't exist yet.
+	// Bundle() creates it from origin/feature (C) and pulls (already up-to-date).
+	_, err = p.Bundle("feature")
+	require.NoError(t, err, "first Bundle() on non-default branch should succeed")
+
+	// Advance feature on remote: commit D (feature = A→C→D)
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/feature"}))
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "feature.txt"), []byte("feature-v2"), 0644))
+	_, err = wt.Add("feature.txt")
+	require.NoError(t, err)
+	_, err = wt.Commit("advance feature", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/master"}))
+
+	// Second Bundle("feature") call: local feature is at C, remote feature is
+	// at D (descendant of C), remote master is at B (not a descendant of C).
+	// Without the fix, Pull targets origin/master (B) → non-fast-forward update.
+	// With the fix, Pull targets origin/feature (D) → fast-forward C→D.
+	_, err = p.Bundle("feature")
+	assert.NoError(t, err, "second Bundle() should fast-forward feature to new remote tip, not fail with non-fast-forward update")
+}
+
+// TestBundle_NonDefaultBranch_DirectDescendant covers the scenario from the
+// original bug report: a non-default branch that is a direct descendant of the
+// default branch (feature = master + extra commits on top).
+//
+// The error in this case is non-obvious: go-git's Pull without ReferenceName
+// targets origin/master (B) and tries to fast-forward local feature (C) to B.
+// Since B is an ancestor of C (not a descendant), this is a non-fast-forward
+// update — go-git refuses to move the branch pointer backwards.
+//
+// With the fix (ReferenceName set), Pull correctly targets origin/feature (D)
+// and fast-forwards C → D.
+func TestBundle_NonDefaultBranch_DirectDescendant(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git CLI not available")
+	}
+
+	// Set up remote with feature as a direct descendant of master:
+	//   master:  A → B
+	//   feature: A → B → C  (ahead of master, contains all master commits)
+	remoteDir := t.TempDir()
+	remoteRepo, err := git.PlainInit(remoteDir, false)
+	require.NoError(t, err)
+
+	wt, err := remoteRepo.Worktree()
+	require.NoError(t, err)
+	sig := testSig()
+
+	// Commit A on master
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "README.md"), []byte("initial"), 0644))
+	_, err = wt.Add("README.md")
+	require.NoError(t, err)
+	_, err = wt.Commit("initial commit", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+
+	// Commit B on master
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "base.txt"), []byte("base"), 0644))
+	_, err = wt.Add("base.txt")
+	require.NoError(t, err)
+	commitB, err := wt.Commit("base commit", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+
+	// Create feature branch at B (direct descendant starting point)
+	require.NoError(t, remoteRepo.Storer.SetReference(
+		plumbing.NewHashReference("refs/heads/feature", commitB),
+	))
+
+	// Commit C on feature (feature = A→B→C, master = A→B)
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/feature"}))
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "feature.txt"), []byte("feature-v1"), 0644))
+	_, err = wt.Add("feature.txt")
+	require.NoError(t, err)
+	commitC, err := wt.Commit("feature commit", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/master"}))
+
+	// Clone the remote
+	localDir := t.TempDir()
+	cloned, err := git.PlainClone(localDir, false, &git.CloneOptions{URL: remoteDir})
+	require.NoError(t, err)
+
+	workingDir := t.TempDir()
+	p := &GitProvider{
+		RepoURL:        remoteDir,
+		gitRepository:  cloned,
+		repositoryPath: localDir,
+		workingDir:     workingDir,
+	}
+
+	// First Bundle("feature"): creates local feature = C, pulls (already up-to-date)
+	_, err = p.Bundle("feature")
+	require.NoError(t, err, "first Bundle() on direct-descendant branch should succeed")
+
+	// Advance feature on remote: commit D (feature = A→B→C→D)
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/feature"}))
+	require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "feature.txt"), []byte("feature-v2"), 0644))
+	_, err = wt.Add("feature.txt")
+	require.NoError(t, err)
+	commitD, err := wt.Commit("advance feature", &git.CommitOptions{Author: sig})
+	require.NoError(t, err)
+	require.NoError(t, wt.Checkout(&git.CheckoutOptions{Branch: "refs/heads/master"}))
+
+	// Second Bundle("feature"): local feature is at C, remote feature is at D.
+	// master is still at B (ancestor of both C and D) — no divergence.
+	// Without the fix, Pull targets origin/master (B), which is an ancestor of
+	// C, so it returns "already up-to-date" and the bundle is built from C
+	// (stale). With the fix, Pull targets origin/feature (D) and fast-forwards.
+	_, err = p.Bundle("feature")
+	require.NoError(t, err, "second Bundle() on direct-descendant branch should succeed")
+
+	// Verify the local feature branch was advanced to D, not left at C.
+	ref, err := cloned.Reference(plumbing.NewBranchReferenceName("feature"), true)
+	require.NoError(t, err)
+	assert.Equal(t, commitD, ref.Hash(),
+		"feature branch should point to the new remote tip D after pull, not the stale C (%s)", commitC)
+}