From 16ffab275b2f00c5bbc482706751d7fe3922c94f Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Tue, 14 Apr 2026 03:34:11 -0400 Subject: [PATCH] git: add RefIsAncestor, FetchRemote, branch listers; fix CreateFireBranch ref Remove PushKnownBranches from the harness package so git-fire can own push-known policy. Export FetchRemote, ListLocalBranches, ListRemoteBranches, and RefIsAncestor for composition. CreateFireBranch now runs 'git branch ' so backups point at the requested commit instead of current HEAD (required for non-checkout branches). Tests: replace PushKnownBranches coverage with primitives + list-branch smoke. Made-with: Cursor --- git/operations.go | 77 ++++++++++-------------- git/operations_test.go | 133 ++++++++--------------------------------- 2 files changed, 58 insertions(+), 152 deletions(-) diff --git a/git/operations.go b/git/operations.go index 190355f..ff85242 100644 --- a/git/operations.go +++ b/git/operations.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "os" "os/exec" "strings" "time" @@ -143,6 +142,32 @@ func getMergeBaseSHA(repoPath, leftRef, rightRef string) (string, bool, error) { return strings.TrimSpace(string(output)), true, nil } +// RefIsAncestor reports whether ancestorRef is an ancestor of descendantRef +// (git merge-base --is-ancestor). +func RefIsAncestor(repoPath, ancestorRef, descendantRef string) (bool, error) { + cmd := exec.Command("git", "merge-base", "--is-ancestor", ancestorRef, descendantRef) + cmd.Dir = repoPath + err := cmd.Run() + if err == nil { + return true, nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return false, nil + } + return false, fmt.Errorf("git merge-base --is-ancestor %s %s failed: %w", ancestorRef, descendantRef, err) +} + +// FetchRemote runs git fetch for a single remote name. +func FetchRemote(repoPath, remote string) error { + cmd := exec.Command("git", "fetch", remote) + cmd.Dir = repoPath + if output, err := cmd.CombinedOutput(); err != nil { + return commandError("git fetch", err, output) + } + return nil +} + // CreateFireBranch creates a new fire backup branch. func CreateFireBranch(repoPath, originalBranch, localSHA string) (string, error) { timestamp := time.Now().Format("20060102-150405") @@ -152,7 +177,9 @@ func CreateFireBranch(repoPath, originalBranch, localSHA string) (string, error) } branchName := fmt.Sprintf("git-fire-backup-%s-%s-%s", originalBranch, timestamp, shortSHA) - cmd := exec.Command("git", "branch", branchName) + // Point the backup ref at localSHA — never at current HEAD. Callers may be on + // a different branch while backing up a diverged or inactive local branch. + cmd := exec.Command("git", "branch", branchName, localSHA) cmd.Dir = repoPath if output, err := cmd.CombinedOutput(); err != nil { return "", commandError("git branch "+branchName, err, output) @@ -184,47 +211,8 @@ func PushAllBranches(repoPath, remote string) error { return nil } -// PushKnownBranches pushes only branches that exist on the remote. -func PushKnownBranches(repoPath, remote string) error { - cmd := exec.Command("git", "fetch", remote) - cmd.Dir = repoPath - if output, err := cmd.CombinedOutput(); err != nil { - return commandError("git fetch", err, output) - } - - remoteBranches, err := getRemoteBranches(repoPath, remote) - if err != nil { - return fmt.Errorf("failed to get remote branches: %w", err) - } - localBranches, err := getLocalBranches(repoPath) - if err != nil { - return fmt.Errorf("failed to get local branches: %w", err) - } - - var errs []error - for _, localBranch := range localBranches { - exists := false - for _, remoteBranch := range remoteBranches { - if remoteBranch == localBranch { - exists = true - break - } - } - - if exists { - if err := PushBranch(repoPath, remote, localBranch); err != nil { - errs = append(errs, fmt.Errorf("branch %s: %w", localBranch, err)) - } - continue - } - - fmt.Fprintf(os.Stderr, "warning: branch '%s' has no remote tracking ref — not backed up\n", localBranch) - } - - return errors.Join(errs...) -} - -func getRemoteBranches(repoPath, remote string) ([]string, error) { +// ListRemoteBranches returns short branch names under remote/ (excluding HEAD). +func ListRemoteBranches(repoPath, remote string) ([]string, error) { cmd := exec.Command("git", "branch", "-r", "--format=%(refname:short)") cmd.Dir = repoPath output, err := cmd.Output() @@ -250,7 +238,8 @@ func getRemoteBranches(repoPath, remote string) ([]string, error) { return branches, nil } -func getLocalBranches(repoPath string) ([]string, error) { +// ListLocalBranches returns local branch short names. +func ListLocalBranches(repoPath string) ([]string, error) { cmd := exec.Command("git", "branch", "--format=%(refname:short)") cmd.Dir = repoPath output, err := cmd.Output() diff --git a/git/operations_test.go b/git/operations_test.go index 2d4cea0..a9e96bf 100644 --- a/git/operations_test.go +++ b/git/operations_test.go @@ -1,7 +1,6 @@ package git import ( - "bytes" "os" "os/exec" "path/filepath" @@ -1119,131 +1118,49 @@ func TestPushAllBranches_InvalidRemote(t *testing.T) { } } -func TestPushKnownBranches(t *testing.T) { +func TestListLocalAndRemoteBranches(t *testing.T) { remote := testutil.CreateBareRemote(t, "origin") repo := testutil.CreateTestRepo(t, testutil.RepoOptions{ Name: "test-repo", Remotes: map[string]string{"origin": remote}, }) - - // Push once to establish the remote branch if err := PushAllBranches(repo, "origin"); err != nil { - t.Fatalf("initial push: %v", err) + t.Fatalf("push: %v", err) } - - // Add a new commit so we can verify PushKnownBranches actually moves the remote ref - testutil.RunGitCmd(t, repo, "commit", "--allow-empty", "-m", "second commit") - localSHA := testutil.GetCurrentSHA(t, repo) - - if err := PushKnownBranches(repo, "origin"); err != nil { - t.Errorf("PushKnownBranches() error = %v", err) + if err := FetchRemote(repo, "origin"); err != nil { + t.Fatal(err) } - - // Confirm the remote ref was updated to the new commit - remoteSHA := testutil.GetCurrentSHA(t, remote) - if remoteSHA != localSHA { - t.Errorf("remote SHA = %s, want %s — branch was not pushed", remoteSHA, localSHA) + locals, err := ListLocalBranches(repo) + if err != nil || len(locals) == 0 { + t.Fatalf("ListLocalBranches: %v %#v", err, locals) } -} - -func TestPushKnownBranches_NoRemoteBranches(t *testing.T) { - remote := testutil.CreateBareRemote(t, "origin") - repo := testutil.CreateTestRepo(t, testutil.RepoOptions{ - Name: "test-repo", - Remotes: map[string]string{"origin": remote}, - }) - - // No branches pushed yet — PushKnownBranches should succeed (nothing to do) - if err := PushKnownBranches(repo, "origin"); err != nil { - t.Errorf("PushKnownBranches() with no remote branches: error = %v", err) + remotes, err := ListRemoteBranches(repo, "origin") + if err != nil || len(remotes) == 0 { + t.Fatalf("ListRemoteBranches: %v %#v", err, remotes) } } -func TestPushKnownBranches_WarnsForLocalOnlyBranch(t *testing.T) { +func TestRefIsAncestor(t *testing.T) { remote := testutil.CreateBareRemote(t, "origin") - repo := testutil.CreateTestRepo(t, testutil.RepoOptions{ - Name: "test-repo", + local := testutil.CreateTestRepo(t, testutil.RepoOptions{ + Name: "local", Remotes: map[string]string{"origin": remote}, + Files: map[string]string{"a.txt": "1"}, }) - - // Establish the default branch on remote. - currentBranch, err := GetCurrentBranch(repo) - if err != nil { - t.Fatalf("GetCurrentBranch() error = %v", err) - } - if err := PushBranch(repo, "origin", currentBranch); err != nil { - t.Fatalf("initial push failed: %v", err) - } - - // Create local-only branch that does not exist on remote. - testutil.RunGitCmd(t, repo, "checkout", "-b", "feature-local-only") - testutil.RunGitCmd(t, repo, "commit", "--allow-empty", "-m", "feature work") - testutil.RunGitCmd(t, repo, "checkout", "-") - - origStderr := os.Stderr - r, w, err := os.Pipe() + main, err := GetCurrentBranch(local) if err != nil { - t.Fatalf("pipe: %v", err) - } - os.Stderr = w - - callErr := PushKnownBranches(repo, "origin") - - w.Close() - os.Stderr = origStderr - - if callErr != nil { - t.Fatalf("PushKnownBranches() error = %v", callErr) - } - - var stderr bytes.Buffer - if _, err := stderr.ReadFrom(r); err != nil { - t.Fatalf("read stderr: %v", err) - } - if !strings.Contains(stderr.String(), "feature-local-only") { - t.Fatalf("expected warning for local-only branch, got: %q", stderr.String()) - } -} - -func TestPushKnownBranches_ContinuesOnError(t *testing.T) { - remote := testutil.CreateBareRemote(t, "origin") - repo := testutil.CreateTestRepo(t, testutil.RepoOptions{ - Name: "test-repo", - Remotes: map[string]string{"origin": remote}, - Branches: []string{"accept-branch", "reject-branch"}, - }) - - // Push all branches to establish them on the remote - if err := PushAllBranches(repo, "origin"); err != nil { - t.Fatalf("initial push: %v", err) + t.Fatal(err) } - - // accept-branch: add a new commit (fast-forward friendly) - testutil.RunGitCmd(t, repo, "checkout", "accept-branch") - testutil.RunGitCmd(t, repo, "commit", "--allow-empty", "-m", "extra commit on accept-branch") - acceptLocalSHA := testutil.GetCurrentSHA(t, repo) - - // reject-branch: push an extra commit to the remote, then rewind local and - // add a different commit — local and remote now have diverged histories. - testutil.RunGitCmd(t, repo, "checkout", "reject-branch") - testutil.RunGitCmd(t, repo, "commit", "--allow-empty", "-m", "commit pushed to remote") - testutil.RunGitCmd(t, repo, "push", "origin", "reject-branch") // advance remote - testutil.RunGitCmd(t, repo, "reset", "--hard", "HEAD^") // rewind local - testutil.RunGitCmd(t, repo, "commit", "--allow-empty", "-m", "diverged local commit") - - // Return to the default branch before calling PushKnownBranches - testutil.RunGitCmd(t, repo, "checkout", "-") - - err := PushKnownBranches(repo, "origin") - if err == nil { - t.Error("expected error: reject-branch should fail with non-fast-forward rejection") + testutil.RunGitCmd(t, local, "push", "-u", "origin", main) + testutil.RunGitCmd(t, local, "commit", "--allow-empty", "-m", "second") + // Intentionally do not push again: local main is strictly ahead of origin/main. + yes, err := RefIsAncestor(local, "origin/"+main, main) + if err != nil || !yes { + t.Fatalf("origin/%s should be ancestor of local %s: %v %v", main, main, yes, err) } - - // accept-branch must have been pushed despite reject-branch failing - acceptRemoteSHA := getBareBranchSHA(t, remote, "accept-branch") - if acceptRemoteSHA != acceptLocalSHA { - t.Errorf("accept-branch remote SHA = %s, want %s — branch was not pushed after sibling failure", - acceptRemoteSHA, acceptLocalSHA) + no, err := RefIsAncestor(local, main, "origin/"+main) + if err != nil || no { + t.Fatalf("local should not be ancestor of origin when ahead: %v %v", no, err) } }