tools/bump: carry hand-pushed commits instead of pausing

Open PR with commits by others: rebase onto base, build on top, push
with lease. Rebase conflict pauses and comments once.

tools/bump/git.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 )
@@ -73,36 +74,96 @@ func commitAll(ctx context.Context, r *repo, message string) error {
 	return err
 }
 
-// startBranch rebuilds the working branch from the base every run. The branch
-// therefore never accumulates history, never conflicts, and the bot keeps no
-// state on disk between runs.
-func startBranch(ctx context.Context, r *repo, remote, base, branch string) error {
+var errRebaseConflict = errors.New("hand-pushed commits do not rebase cleanly")
+
+// startBranch rebuilds the working branch from the base every run, so it never
+// accumulates history and the bot keeps no state on disk between runs. The
+// exception is an open pull request holding commits someone pushed by hand:
+// rebuilding would erase them, so the branch is rebased onto the base and the
+// run builds on top. It returns the subjects of the commits it carried.
+func startBranch(ctx context.Context, r *repo, remote, base, branch string, carry bool) ([]string, error) {
 	if _, err := r.run(ctx, "git", "fetch", remote, base); err != nil { //nolint:noinlineerr
-		return err
+		return nil, err
 	}
 
-	_, err := r.run(ctx, "git", "checkout", "-B", branch, remote+"/"+base)
+	var hand []string
 
-	return err
+	if carry {
+		var err error
+
+		hand, err = handCommits(ctx, r, remote, base, branch)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	start := remote + "/" + base
+	if len(hand) > 0 {
+		start = remote + "/" + branch
+	}
+
+	if _, err := r.run(ctx, "git", "checkout", "-B", branch, start); err != nil { //nolint:noinlineerr
+		return nil, err
+	}
+
+	if len(hand) == 0 {
+		return nil, nil
+	}
+
+	if _, err := r.run(ctx, "git", "rebase", remote+"/"+base); err != nil { //nolint:noinlineerr
+		_, _ = r.run(ctx, "git", "rebase", "--abort")
+
+		return nil, fmt.Errorf("%w onto %s: %w", errRebaseConflict, base, err)
+	}
+
+	return hand, nil
 }
 
-// remoteBranchSHA is the tip of branch on remote, or "" when it does not exist.
-func remoteBranchSHA(ctx context.Context, r *repo, remote, branch string) string {
-	out, err := r.run(ctx, "git", "ls-remote", "--heads", remote, branch)
+// handCommits lists, oldest first, the subjects of commits on the remote branch
+// authored by anyone but the configured committer.
+func handCommits(ctx context.Context, r *repo, remote, base, branch string) ([]string, error) {
+	if _, err := r.run(ctx, "git", "fetch", remote, branch); err != nil { //nolint:noinlineerr
+		return nil, err
+	}
+
+	self, err := r.run(ctx, "git", "config", "user.email")
 	if err != nil {
-		return ""
+		return nil, err
 	}
 
-	fields := strings.Fields(out)
-	if len(fields) == 0 {
-		return ""
+	out, err := r.run(ctx, "git", "log", "--reverse", "--format=%ae%x00%s",
+		remote+"/"+base+".."+remote+"/"+branch)
+	if err != nil {
+		return nil, err
 	}
 
-	return fields[0]
+	return notAuthoredBy(out, strings.TrimSpace(self)), nil
 }
 
-func forcePush(ctx context.Context, r *repo, remote, branch string) error {
-	_, err := r.run(ctx, "git", "push", "--force", remote, "HEAD:refs/heads/"+branch)
+// notAuthoredBy filters `git log --format=%ae%x00%s` output to the subjects of
+// commits whose author is not email.
+func notAuthoredBy(log, email string) []string {
+	var subjects []string
+
+	for line := range strings.Lines(log) {
+		author, subject, _ := strings.Cut(strings.TrimRight(line, "\n"), "\x00")
+		if author != email {
+			subjects = append(subjects, subject)
+		}
+	}
+
+	return subjects
+}
+
+// forcePush replaces branch on remote. A non-empty lease is the tip the run
+// started from; the push is refused if anyone moved the branch since.
+func forcePush(ctx context.Context, r *repo, remote, branch, lease string) error {
+	force := "--force"
+	if lease != "" {
+		force = "--force-with-lease=refs/heads/" + branch + ":" + lease
+	}
+
+	_, err := r.run(ctx, "git", "push", force, remote, "HEAD:refs/heads/"+branch)
 	if err != nil {
 		return fmt.Errorf("pushing %s: %w", branch, err)
 	}

tools/bump/git_test.go

@@ -0,0 +1,27 @@
+package main
+
+import (
+	"slices"
+	"testing"
+)
+
+// The bot's own commits are rebuilt every run; only someone else's are carried.
+func TestNotAuthoredBy(t *testing.T) {
+	const bot = "41898282+github-actions[bot]@users.noreply.github.com"
+
+	log := bot + "\x00flake.lock: update nixpkgs\n" +
+		"dev@example.com\x00noise: fix deprecation\n" +
+		bot + "\x00go.mod: update dependencies\n" +
+		"dev@example.com\x00capver: regenerate\n"
+
+	got := notAuthoredBy(log, bot)
+
+	want := []string{"noise: fix deprecation", "capver: regenerate"}
+	if !slices.Equal(got, want) {
+		t.Errorf("notAuthoredBy = %q, want %q", got, want)
+	}
+
+	if got := notAuthoredBy("", bot); len(got) != 0 {
+		t.Errorf("empty log carried %q", got)
+	}
+}

tools/bump/main.go

@@ -19,6 +19,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"os"
@@ -112,7 +113,37 @@ func cmdRun(ctx context.Context) error {
 		return err
 	}
 
-	if err := startBranch(ctx, r, runCfg.Remote, runCfg.Base, runCfg.Branch); err != nil { //nolint:noinlineerr
+	// Looked up before the branch is built: an open pull request holding
+	// hand-pushed commits decides what the branch starts from.
+	var (
+		slug string
+		open *prState
+	)
+
+	if !noPR {
+		slug, err = currentSlug(ctx, r, runCfg.Repo)
+		if err != nil {
+			return err
+		}
+
+		pr, err := lookupPR(ctx, r, slug, runCfg.Branch)
+		if err != nil {
+			return err
+		}
+
+		if pr != nil && pr.State == "OPEN" && !pr.hasLabel(skipLabel) {
+			open = pr
+		}
+	}
+
+	carried, err := startBranch(ctx, r, runCfg.Remote, runCfg.Base, runCfg.Branch, open != nil)
+	if errors.Is(err, errRebaseConflict) {
+		_ = commentOnce(ctx, r, slug, open.Number,
+			"The commits pushed by hand here no longer rebase onto "+runCfg.Base+
+				", so the automation has paused. Rebase the branch, or merge or close this pull request.")
+	}
+
+	if err != nil {
 		return err
 	}
 
@@ -136,7 +167,7 @@ func cmdRun(ctx context.Context) error {
 		return err
 	}
 
-	body := renderBody(results, markerOf(results, tree, head), gate)
+	body := renderBody(results, markerOf(results, tree, head), gate, carried)
 
 	if err := writeStepSummary(body); err != nil { //nolint:noinlineerr
 		return err
@@ -154,19 +185,21 @@ func cmdRun(ctx context.Context) error {
 		return nil
 	}
 
-	slug, err := currentSlug(ctx, r, runCfg.Repo)
-	if err != nil {
-		return err
+	var lease string
+	if open != nil {
+		lease = open.HeadRefOid
 	}
 
 	return publish(ctx, r, publishOptions{
-		Slug:   slug,
-		Remote: runCfg.Remote,
-		Branch: runCfg.Branch,
-		Base:   runCfg.Base,
-		Title:  "all: bump pinned versions",
-		Gate:   gate,
-		Force:  runCfg.Force,
+		Slug:    slug,
+		Remote:  runCfg.Remote,
+		Branch:  runCfg.Branch,
+		Base:    runCfg.Base,
+		Title:   "all: bump pinned versions",
+		Gate:    gate,
+		Force:   runCfg.Force,
+		Lease:   lease,
+		Carried: carried,
 	}, results)
 }
 

tools/bump/pr.go

@@ -3,7 +3,6 @@ package main
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"log"
 	"strconv"
@@ -18,8 +17,6 @@ const skipLabel = "no-autoupdate"
 // same diff the next morning is how an automation earns itself an ignore rule.
 const closedCooldown = 7 * 24 * time.Hour
 
-var errHumanCommits = errors.New("branch has commits the bot did not push")
-
 type prState struct {
 	Number     int    `json:"number"`
 	State      string `json:"state"`
@@ -83,6 +80,10 @@ type publishOptions struct {
 	Title  string
 	Gate   string
 	Force  bool
+	// Lease is the branch tip the run started from, when it updates an open
+	// pull request; Carried lists hand-pushed commits it built on.
+	Lease   string
+	Carried []string
 }
 
 // publish force-pushes the rebuilt branch and puts it in front of a human:
@@ -99,7 +100,7 @@ func publish(ctx context.Context, r *repo, opt publishOptions, results []result)
 		return err
 	}
 
-	body := renderBody(results, markerOf(results, tree, head), opt.Gate)
+	body := renderBody(results, markerOf(results, tree, head), opt.Gate, opt.Carried)
 
 	pr, err := lookupPR(ctx, r, opt.Slug, opt.Branch)
 	if err != nil {
@@ -128,28 +129,18 @@ func publish(ctx context.Context, r *repo, opt publishOptions, results []result)
 		body = fmt.Sprintf("Previously closed unmerged: #%d\n\n%s", pr.Number, body)
 	}
 
-	if err := forcePush(ctx, r, opt.Remote, opt.Branch); err != nil { //nolint:noinlineerr
+	if err := forcePush(ctx, r, opt.Remote, opt.Branch, ""); err != nil { //nolint:noinlineerr
 		return err
 	}
 
 	return createPR(ctx, r, opt, body)
 }
 
-// updateOpen refreshes an existing pull request in place, but refuses to
-// force-push over work the bot did not author.
+// updateOpen refreshes an existing pull request in place. Hand-pushed commits
+// were carried when the branch was built; the lease keeps the push from
+// overwriting any that landed during the run.
 func updateOpen(ctx context.Context, r *repo, opt publishOptions, pr *prState, body string) error {
-	prev, hadMarker := parseMarker(pr.Body)
-
-	remoteTip := remoteBranchSHA(ctx, r, opt.Remote, opt.Branch)
-	if hadMarker && prev.Head != "" && remoteTip != "" && remoteTip != prev.Head {
-		_, _ = r.run(ctx, "gh", "pr", "comment", strconv.Itoa(pr.Number), "--repo", opt.Slug,
-			"--body", "This branch has commits the automation did not push, so it has paused. "+
-				"Merge or close this pull request to resume.")
-
-		return fmt.Errorf("%w: %s is at %s, expected %s", errHumanCommits, opt.Branch, remoteTip, prev.Head)
-	}
-
-	err := forcePush(ctx, r, opt.Remote, opt.Branch)
+	err := forcePush(ctx, r, opt.Remote, opt.Branch, opt.Lease)
 	if err != nil {
 		return err
 	}
@@ -183,3 +174,21 @@ func createPR(ctx context.Context, r *repo, opt publishOptions, body string) err
 
 	return nil
 }
+
+// commentOnce posts body unless the pull request already has it, so a
+// condition that persists across daily runs is reported once.
+func commentOnce(ctx context.Context, r *repo, slug string, number int, body string) error {
+	out, err := r.run(ctx, "gh", "pr", "view", strconv.Itoa(number), "--repo", slug,
+		"--json", "comments", "--jq", ".comments[].body")
+	if err != nil {
+		return err
+	}
+
+	if strings.Contains(out, body) {
+		return nil
+	}
+
+	_, err = r.run(ctx, "gh", "pr", "comment", strconv.Itoa(number), "--repo", slug, "--body", body)
+
+	return err
+}

tools/bump/report.go

@@ -8,7 +8,8 @@ import (
 
 // markerPrefix tags the machine-readable footer of a bot-authored pull request
 // body. It is the bot's only persistent state: everything else is rebuilt from
-// the base branch on every run.
+// the base branch on every run, or rebased onto it when carrying hand-pushed
+// commits.
 const markerPrefix = "<!-- versionbump:v1 "
 
 // marker records what a run produced, so the next run can tell "nothing new"
@@ -63,7 +64,7 @@ func markerOf(results []result, tree, head string) marker {
 // renderBody writes the pull request body. It leads with what landed and what
 // did not, because the point of the bot is that the reader can decide from the
 // body plus the checks without reproducing the run.
-func renderBody(results []result, m marker, gate string) string {
+func renderBody(results []result, m marker, gate string, carried []string) string {
 	var sb strings.Builder
 
 	sb.WriteString("Automated version bump.\n\n")
@@ -81,6 +82,14 @@ func renderBody(results []result, m marker, gate string) string {
 	writeDetails(&sb, results)
 	writeDropped(&sb, results)
 
+	if len(carried) > 0 {
+		sb.WriteString("\n### Carried\n\nPushed by hand, rebased onto the base, and built on:\n\n")
+
+		for _, s := range carried {
+			fmt.Fprintf(&sb, "- %s\n", s)
+		}
+	}
+
 	sb.WriteString("\n")
 	sb.WriteString(gateNote(gate))
 	sb.WriteString("\n")

tools/bump/report_test.go

@@ -26,7 +26,7 @@ func TestMarkerRoundTrip(t *testing.T) {
 	}
 
 	want := markerOf(results, "tree-sha", "head-sha")
-	body := renderBody(results, want, gateFull)
+	body := renderBody(results, want, gateFull, nil)
 
 	got, ok := parseMarker(body)
 	if !ok {