Files
github__gh-stack/internal/git/gitops.go
Sameen Karim adbc3bd206 Stop reporting rebases that git refused to run as successes
`gh stack rebase` and `gh stack sync` could print a full success report
while leaving the stack exactly where it was:

    ✓ Trunk main fast-forwarded to 84bacb3
    ✓ Rebased phil/trd-mark-edge-drills onto main
    ✓ Rebased phil/implement-in-place-apis onto phil/trd-mark-edge-drills
    All branches in stack rebased locally with main

…after which `git merge-base --is-ancestor main HEAD` still reported that
main was not an ancestor.

The cause is in `tryAutoResolveRebase`, the shared error path for every
rebase: it returned nil whenever no rebase was in progress. That is only a
valid success signal *after* an auto-`--continue`. On the first check it
means `git rebase` exited non-zero without ever starting — a dirty working
tree, a branch checked out in another worktree, a missing local trunk, or a
stale rebase still in progress. All of those were silently swallowed and the
cascade carried on reporting success.

- Return a typed `*git.RebaseStartError` for a rebase that never started, and
  detect an already-running rebase up front so its leftover conflicts are not
  mistaken for this rebase's.
- Treat that error as fatal in `cascadeRebase` (and in `modify`) rather than a
  conflict, so no bogus rebase state is written and git's own message shows.
- Add the preflight checks `modify` already had — no rebase in progress, clean
  working tree — to `rebase` and `sync`, with `--autostash` to opt out of the
  clean-tree requirement by passing `--autostash` through to git.
- Verify after every cascade that each branch really does sit on top of its
  parent, and fail instead of printing the success summary. `sync` checks
  before pushing so an unrebased stack is never force-pushed.

Also fixes duplicate commits: the cascade passed the parent's current tip as
the `--onto` upstream with no staleness guard, so a parent amended, reordered,
or squash-merged out of band had its old commits replayed onto the child. The
merged-PR path guarded this but fell back to `merge-base(newBase, branch)`,
which replays squashed commits. Both paths now use the latest commit the child
genuinely contains.

Smaller fixes found alongside:

- `git rebase <option> --continue` is a usage error, so `--preserve-dates`
  made `rebase --continue` fail every time. git persists the option in the
  rebase state, so `--continue` alone honors it.
- `sync` never ensured the local trunk existed before rebasing onto it, and
  printed "Fetched latest changes" even when the fetch failed.
- A trunk whose remote branch is gone now says so instead of silently rebasing
  onto a stale local trunk (partial diagnostic for #225; migrating the trunk
  is a separate change).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f4355094-2f25-4532-a17d-2b88fbf68131
2026-07-26 14:30:42 -04:00

710 lines
20 KiB
Go

package git
import (
"context"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"time"
)
// RebaseOpts holds optional parameters for git rebase operations.
type RebaseOpts struct {
CommitterDateIsAuthorDate bool
// AutoStash passes --autostash to git, letting git stash any local
// changes before the rebase and reapply them afterwards.
AutoStash bool
}
// flags renders the options as git rebase command-line flags.
func (o RebaseOpts) flags() []string {
var args []string
if o.CommitterDateIsAuthorDate {
args = append(args, "--committer-date-is-author-date")
}
if o.AutoStash {
args = append(args, "--autostash")
}
return args
}
// Ops defines the interface for git operations used by commands.
// The package-level functions are the default production implementation.
// Tests can substitute a mock via SetOps().
type Ops interface {
GitDir() (string, error)
RootDir() (string, error)
CurrentBranch() (string, error)
BranchExists(name string) bool
CheckoutBranch(name string) error
Fetch(remote string) error
FetchBranches(remote string, branches []string) error
DefaultBranch() (string, error)
CreateBranch(name, base string) error
Push(remote string, branches []string, force, atomic bool) error
ResolveRemote(branch string) (string, error)
Rebase(base string, opts RebaseOpts) error
EnableRerere() error
IsRerereEnabled() (bool, error)
IsRerereDeclined() (bool, error)
SaveRerereDeclined() error
GetSavedRemote() (string, error)
SaveRemote(remote string) error
ClearRemote() error
RebaseOnto(newBase, oldBase, branch string, opts RebaseOpts) error
RebaseContinue(opts RebaseOpts) error
RebaseAbort() error
IsRebaseInProgress() bool
ConflictedFiles() ([]string, error)
FindConflictMarkers(filePath string) (*ConflictMarkerInfo, error)
IsAncestor(ancestor, descendant string) (bool, error)
RevParse(ref string) (string, error)
RevParseMulti(refs []string) ([]string, error)
MergeBase(a, b string) (string, error)
Log(ref string, maxCount int) ([]CommitInfo, error)
LogRange(base, head string) ([]CommitInfo, error)
DiffStatRange(base, head string) (additions, deletions int, err error)
DiffStatFiles(base, head string) ([]FileDiffStat, error)
DeleteBranch(name string, force bool) error
DeleteRemoteBranch(remote, branch string) error
DeleteTrackingRef(remote, branch string) error
ResetHard(ref string) error
SetUpstreamTracking(branch, remote string) error
MergeFF(target string) error
UpdateBranchRef(branch, sha string) error
StageAll() error
StageTracked() error
HasStagedChanges() bool
Commit(message string) (string, error)
CommitInteractive() (string, error)
ValidateRefName(name string) error
RenameBranch(oldName, newName string) error
CherryPick(commits []string) error
CherryPickQuit() error
CherryPickAbort() error
CherryPickContinue() error
IsCherryPickInProgress() bool
HasUncommittedChanges() (bool, error)
LogMerges(base, head string) ([]CommitInfo, error)
}
// defaultOps implements Ops by delegating to the real git client and helpers.
type defaultOps struct{}
var _ Ops = (*defaultOps)(nil)
// ops is the current implementation. Tests replace this via SetOps().
var ops Ops = &defaultOps{}
// SetOps replaces the git operations implementation. Returns a restore function.
func SetOps(o Ops) func() {
old := ops
ops = o
return func() { ops = old }
}
// CurrentOps returns the current Ops implementation.
func CurrentOps() Ops {
return ops
}
// --- defaultOps method implementations ---
func (d *defaultOps) GitDir() (string, error) {
return client.GitDir(context.Background())
}
func (d *defaultOps) RootDir() (string, error) {
return run("rev-parse", "--show-toplevel")
}
func (d *defaultOps) CurrentBranch() (string, error) {
return client.CurrentBranch(context.Background())
}
func (d *defaultOps) BranchExists(name string) bool {
return client.HasLocalBranch(context.Background(), name)
}
func (d *defaultOps) CheckoutBranch(name string) error {
return client.CheckoutBranch(context.Background(), name)
}
func (d *defaultOps) Fetch(remote string) error {
return client.Fetch(context.Background(), remote, "")
}
func (d *defaultOps) FetchBranches(remote string, branches []string) error {
if len(branches) == 0 {
return nil
}
// Build explicit refspecs that create/update tracking refs for every
// branch, regardless of whether a tracking ref already exists.
// The + prefix allows non-fast-forward tracking-ref updates.
refspecs := make([]string, len(branches))
for i, b := range branches {
refspecs[i] = fmt.Sprintf("+refs/heads/%s:refs/remotes/%s/%s", b, remote, b)
}
// Fast path: fetch all branches in a single call.
args := []string{"fetch", remote}
args = append(args, refspecs...)
if err := runSilent(args...); err == nil {
return nil
}
// Fallback: one branch may be absent on the remote or deleted since
// the last fetch. Fetch individually so one missing branch doesn't
// block the rest. A missing ref is expected and tolerated; any other
// failure (no network, bad credentials, unknown remote) means the fetch
// itself did not work and callers must not report success.
var fetchErr error
for _, rs := range refspecs {
err := runSilent("fetch", remote, rs)
if err == nil || isMissingRemoteRefError(err) {
continue
}
if fetchErr == nil {
fetchErr = fmt.Errorf("fetching from %s: %w", remote, err)
}
}
return fetchErr
}
// isMissingRemoteRefError reports whether a fetch failed only because the
// requested ref does not exist on the remote — the normal case for a local
// branch that has never been pushed, or one whose remote branch was deleted.
func isMissingRemoteRefError(err error) bool {
return err != nil && strings.Contains(err.Error(), "couldn't find remote ref")
}
func (d *defaultOps) DefaultBranch() (string, error) {
ref, err := run("symbolic-ref", "refs/remotes/origin/HEAD")
if err != nil {
for _, name := range []string{"main", "master"} {
if BranchExists(name) {
return name, nil
}
}
return "", err
}
return strings.TrimPrefix(ref, "refs/remotes/origin/"), nil
}
func (d *defaultOps) CreateBranch(name, base string) error {
return runSilent("branch", name, base)
}
func (d *defaultOps) Push(remote string, branches []string, force, atomic bool) error {
args := []string{"push", remote}
if force {
// Build explicit per-branch leases and refspecs. This removes
// dependence on push.default / upstream configuration and
// ensures correct lease values for branches whose tracking ref
// was missing before the preceding FetchBranches call.
for _, b := range branches {
trackingRef := fmt.Sprintf("refs/remotes/%s/%s", remote, b)
sha, err := run("rev-parse", "--verify", "--quiet", trackingRef)
if err == nil && sha != "" {
// Tracking ref exists: lease against the known SHA.
args = append(args, fmt.Sprintf("--force-with-lease=refs/heads/%s:%s", b, sha))
} else {
// No tracking ref: branch is absent on remote (never
// pushed). Empty expected value means "must not exist".
args = append(args, fmt.Sprintf("--force-with-lease=refs/heads/%s:", b))
}
}
}
if atomic {
args = append(args, "--atomic")
}
// Fully-qualified refspecs: refs/heads/<local>:refs/heads/<remote>.
// Qualifying the source (not a bare branch name) ensures a branch name is
// never reinterpreted as refspec syntax — e.g. a leading "+" is part of the
// ref, not a force modifier. This form is identical whether or not the push
// is forced; force is supplied out-of-band by the --force-with-lease flags
// built above.
for _, b := range branches {
args = append(args, fmt.Sprintf("refs/heads/%s:refs/heads/%s", b, b))
}
return runSilent(args...)
}
// ResolveRemote determines the remote for pushing a branch. It checks git
// config keys in priority order (branch.<name>.pushRemote, remote.pushDefault,
// branch.<name>.remote), then checks the gh-stack.remote saved preference,
// then falls back to listing all remotes. If exactly one remote exists it is
// returned. If multiple exist, ErrMultipleRemotes is returned with the list
// attached. If none exist, a plain error is returned.
func (d *defaultOps) ResolveRemote(branch string) (string, error) {
candidates := []string{
"branch." + branch + ".pushRemote",
"remote.pushDefault",
"branch." + branch + ".remote",
}
for _, key := range candidates {
out, err := run("config", "--get", key)
if err == nil && out != "" {
return out, nil
}
}
// Check gh-stack saved remote preference.
if saved, err := d.GetSavedRemote(); err == nil && saved != "" {
return saved, nil
}
out, err := run("remote")
if err != nil {
return "", fmt.Errorf("could not list remotes: %w", err)
}
remotes := strings.Fields(strings.TrimSpace(out))
if len(remotes) == 1 {
return remotes[0], nil
}
if len(remotes) > 1 {
return "", &ErrMultipleRemotes{Remotes: remotes}
}
return "", fmt.Errorf("no remotes configured")
}
func (d *defaultOps) Rebase(base string, opts RebaseOpts) error {
args := []string{"rebase"}
args = append(args, opts.flags()...)
args = append(args, base)
return runRebaseCommand(args, opts)
}
func (d *defaultOps) EnableRerere() error {
if err := runSilent("config", "rerere.enabled", "true"); err != nil {
return err
}
return runSilent("config", "rerere.autoupdate", "true")
}
func (d *defaultOps) IsRerereEnabled() (bool, error) {
out, err := run("config", "--get", "rerere.enabled")
if err != nil {
// Missing key — not enabled.
return false, nil
}
return strings.EqualFold(strings.TrimSpace(out), "true"), nil
}
func (d *defaultOps) IsRerereDeclined() (bool, error) {
out, err := run("config", "--get", "gh-stack.rerere-declined")
if err != nil {
return false, nil
}
return strings.EqualFold(strings.TrimSpace(out), "true"), nil
}
func (d *defaultOps) SaveRerereDeclined() error {
return runSilent("config", "gh-stack.rerere-declined", "true")
}
func (d *defaultOps) GetSavedRemote() (string, error) {
out, err := run("config", "--get", "gh-stack.remote")
if err != nil {
return "", err
}
return out, nil
}
func (d *defaultOps) SaveRemote(remote string) error {
return runSilent("config", "gh-stack.remote", remote)
}
func (d *defaultOps) ClearRemote() error {
return runSilent("config", "--unset", "gh-stack.remote")
}
func (d *defaultOps) RebaseOnto(newBase, oldBase, branch string, opts RebaseOpts) error {
args := []string{"rebase"}
args = append(args, opts.flags()...)
args = append(args, "--onto", newBase, oldBase, branch)
return runRebaseCommand(args, opts)
}
func (d *defaultOps) RebaseContinue(opts RebaseOpts) error {
err := rebaseContinueOnce()
if err == nil {
return nil
}
return tryAutoResolveRebase(err, opts)
}
func (d *defaultOps) RebaseAbort() error {
return runSilent("rebase", "--abort")
}
func (d *defaultOps) IsRebaseInProgress() bool {
gitDir, err := GitDir()
if err != nil {
return false
}
for _, dir := range []string{"rebase-merge", "rebase-apply"} {
rebasePath := filepath.Join(gitDir, dir)
if info, err := os.Stat(rebasePath); err == nil && info.IsDir() {
return true
}
}
return false
}
func (d *defaultOps) ConflictedFiles() ([]string, error) {
output, err := run("diff", "--name-only", "--diff-filter=U")
if err != nil {
return nil, err
}
if output == "" {
return nil, nil
}
return strings.Split(output, "\n"), nil
}
func (d *defaultOps) FindConflictMarkers(filePath string) (*ConflictMarkerInfo, error) {
output, err := run("diff", "--check", "--", filePath)
if output == "" && err != nil {
return nil, err
}
info := &ConflictMarkerInfo{File: filePath}
var currentSection *ConflictSection
for _, line := range strings.Split(output, "\n") {
line = strings.TrimSpace(line)
if line == "" {
continue
}
parts := strings.SplitN(line, ":", 3)
if len(parts) < 3 {
continue
}
lineNo, parseErr := strconv.Atoi(strings.TrimSpace(parts[1]))
if parseErr != nil {
continue
}
marker := strings.TrimSpace(parts[2])
if strings.Contains(marker, "leftover conflict marker") {
if currentSection == nil || currentSection.EndLine != 0 {
currentSection = &ConflictSection{StartLine: lineNo}
info.Sections = append(info.Sections, *currentSection)
}
info.Sections[len(info.Sections)-1].EndLine = lineNo
}
}
return info, nil
}
func (d *defaultOps) IsAncestor(ancestor, descendant string) (bool, error) {
err := runSilent("merge-base", "--is-ancestor", ancestor, descendant)
if err == nil {
return true, nil
}
var exitErr *exec.ExitError
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
return false, nil
}
return false, err
}
func (d *defaultOps) RevParse(ref string) (string, error) {
return run("rev-parse", ref)
}
func (d *defaultOps) RevParseMulti(refs []string) ([]string, error) {
if len(refs) == 0 {
return nil, nil
}
args := append([]string{"rev-parse"}, refs...)
out, err := run(args...)
if err != nil {
return nil, err
}
shas := strings.Split(out, "\n")
if len(shas) != len(refs) {
return nil, fmt.Errorf("rev-parse returned %d SHAs for %d refs", len(shas), len(refs))
}
return shas, nil
}
func (d *defaultOps) MergeBase(a, b string) (string, error) {
return run("merge-base", a, b)
}
func (d *defaultOps) Log(ref string, maxCount int) ([]CommitInfo, error) {
format := "%H\t%s\t%at"
output, err := run("log", ref, "--format="+format, "-n", strconv.Itoa(maxCount))
if err != nil {
return nil, err
}
if output == "" {
return nil, nil
}
var commits []CommitInfo
for _, line := range strings.Split(output, "\n") {
parts := strings.SplitN(line, "\t", 3)
if len(parts) < 3 {
continue
}
ts, _ := strconv.ParseInt(parts[2], 10, 64)
commits = append(commits, CommitInfo{
SHA: parts[0],
Subject: parts[1],
Time: time.Unix(ts, 0),
})
}
return commits, nil
}
func (d *defaultOps) LogRange(base, head string) ([]CommitInfo, error) {
format := "%H%x01%B%x01%at%x00"
rangeSpec := base + ".." + head
output, err := run("log", rangeSpec, "--format="+format)
if err != nil {
return nil, err
}
if output == "" {
return nil, nil
}
var commits []CommitInfo
for _, record := range strings.Split(output, "\x00") {
record = strings.TrimSpace(record)
if record == "" {
continue
}
parts := strings.SplitN(record, "\x01", 3)
if len(parts) < 3 {
continue
}
ts, _ := strconv.ParseInt(strings.TrimSpace(parts[2]), 10, 64)
subject, body := splitCommitMessage(parts[1])
commits = append(commits, CommitInfo{
SHA: parts[0],
Subject: subject,
Body: body,
Time: time.Unix(ts, 0),
})
}
return commits, nil
}
// splitCommitMessage splits a full commit message into subject (first line)
// and body (remaining lines with leading/trailing blank lines trimmed).
func splitCommitMessage(msg string) (subject, body string) {
msg = strings.TrimSpace(msg)
if i := strings.IndexByte(msg, '\n'); i >= 0 {
subject = msg[:i]
body = strings.TrimSpace(msg[i+1:])
} else {
subject = msg
}
return
}
func (d *defaultOps) DiffStatRange(base, head string) (additions, deletions int, err error) {
output, err := run("diff", "--numstat", base+".."+head)
if err != nil {
return 0, 0, err
}
if output == "" {
return 0, 0, nil
}
for _, line := range strings.Split(output, "\n") {
parts := strings.Fields(line)
if len(parts) < 2 {
continue
}
if parts[0] == "-" {
continue
}
a, _ := strconv.Atoi(parts[0])
d, _ := strconv.Atoi(parts[1])
additions += a
deletions += d
}
return additions, deletions, nil
}
func (d *defaultOps) DiffStatFiles(base, head string) ([]FileDiffStat, error) {
output, err := run("diff", "--numstat", base+".."+head)
if err != nil {
return nil, err
}
if output == "" {
return nil, nil
}
var files []FileDiffStat
for _, line := range strings.Split(output, "\n") {
parts := strings.Fields(line)
if len(parts) < 3 {
continue
}
a, _ := strconv.Atoi(parts[0])
d, _ := strconv.Atoi(parts[1])
files = append(files, FileDiffStat{
Path: parts[2],
Additions: a,
Deletions: d,
})
}
return files, nil
}
func (d *defaultOps) DeleteBranch(name string, force bool) error {
flag := "-d"
if force {
flag = "-D"
}
return runSilent("branch", flag, name)
}
func (d *defaultOps) DeleteRemoteBranch(remote, branch string) error {
// Fully-qualify the ref so a branch name is never reinterpreted as
// refspec syntax.
return runSilent("push", remote, "--delete", "refs/heads/"+branch)
}
func (d *defaultOps) DeleteTrackingRef(remote, branch string) error {
return runSilent("branch", "-dr", remote+"/"+branch)
}
func (d *defaultOps) ResetHard(ref string) error {
return runSilent("reset", "--hard", ref)
}
func (d *defaultOps) SetUpstreamTracking(branch, remote string) error {
return runSilent("branch", "--set-upstream-to="+remote+"/"+branch, branch)
}
func (d *defaultOps) MergeFF(target string) error {
return runSilent("merge", "--ff-only", target)
}
func (d *defaultOps) UpdateBranchRef(branch, sha string) error {
return runSilent("branch", "-f", branch, sha)
}
func (d *defaultOps) StageAll() error {
return runSilent("add", "-A")
}
func (d *defaultOps) StageTracked() error {
return runSilent("add", "-u")
}
func (d *defaultOps) HasStagedChanges() bool {
err := runSilent("diff", "--cached", "--quiet")
return err != nil
}
func (d *defaultOps) Commit(message string) (string, error) {
if err := runSilent("commit", "-m", message); err != nil {
return "", err
}
return run("rev-parse", "HEAD")
}
// CommitInteractive launches the user's editor for the commit message.
func (d *defaultOps) CommitInteractive() (string, error) {
if err := runInteractive("commit"); err != nil {
return "", err
}
return run("rev-parse", "HEAD")
}
func (d *defaultOps) ValidateRefName(name string) error {
_, err := run("check-ref-format", "--branch", name)
return err
}
func (d *defaultOps) RenameBranch(oldName, newName string) error {
return runSilent("branch", "-m", oldName, newName)
}
func (d *defaultOps) CherryPick(commits []string) error {
args := append([]string{"cherry-pick"}, commits...)
return runSilent(args...)
}
// CherryPickQuit clears the in-progress cherry-pick sequencer state without
// touching the working tree or index (git cherry-pick --quit). Used to clear
// any stale sequencer state before starting a fresh cherry-pick.
func (d *defaultOps) CherryPickQuit() error {
return runSilent("cherry-pick", "--quit")
}
// CherryPickAbort cancels an in-progress cherry-pick and restores the working
// tree and index to the state before the cherry-pick began
// (git cherry-pick --abort). Errors if no cherry-pick is in progress, so
// callers should gate this with IsCherryPickInProgress.
func (d *defaultOps) CherryPickAbort() error {
return runSilent("cherry-pick", "--abort")
}
func (d *defaultOps) CherryPickContinue() error {
cmd := exec.Command("git", "cherry-pick", "--continue")
cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
return cmd.Run()
}
// IsCherryPickInProgress reports whether a cherry-pick is currently in progress
// by checking for the CHERRY_PICK_HEAD marker in the git directory.
func (d *defaultOps) IsCherryPickInProgress() bool {
gitDir, err := GitDir()
if err != nil {
return false
}
if _, err := os.Stat(filepath.Join(gitDir, "CHERRY_PICK_HEAD")); err == nil {
return true
}
return false
}
func (d *defaultOps) HasUncommittedChanges() (bool, error) {
out, err := run("status", "--porcelain")
if err != nil {
return false, err
}
return out != "", nil
}
func (d *defaultOps) LogMerges(base, head string) ([]CommitInfo, error) {
format := "%H%x01%B%x01%at%x00"
rangeSpec := base + ".." + head
output, err := run("log", "--merges", rangeSpec, "--format="+format)
if err != nil {
return nil, err
}
if output == "" {
return nil, nil
}
var commits []CommitInfo
for _, record := range strings.Split(output, "\x00") {
record = strings.TrimSpace(record)
if record == "" {
continue
}
parts := strings.SplitN(record, "\x01", 3)
if len(parts) < 3 {
continue
}
ts, _ := strconv.ParseInt(strings.TrimSpace(parts[2]), 10, 64)
subject, body := splitCommitMessage(parts[1])
commits = append(commits, CommitInfo{
SHA: parts[0],
Subject: subject,
Body: body,
Time: time.Unix(ts, 0),
})
}
return commits, nil
}