From 731b0b963c0601b9d06164a8aa23745646970aab Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@dalby.cc>
Date: Wed, 23 Sep 2026 07:28:08 +0000
Subject: [PATCH] tools/bump: refuse a batch that moves a requirement backwards

Diffing go.mod catches a downgrade wherever selection produced it;
reading what go get printed only catches what go get did itself. The
lockstep partners are exempt, since repin lowers them on purpose.
---
 tools/bump/bisect.go        | 17 +++++++++-
 tools/bump/gomod.go         | 38 ++++++++++++++++++++++
 tools/bump/modproxy_test.go | 63 +++++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/tools/bump/bisect.go b/tools/bump/bisect.go
index e5ccbd99..cc545bb8 100644
--- a/tools/bump/bisect.go
+++ b/tools/bump/bisect.go
@@ -68,6 +68,11 @@ func applyAtoms(ctx context.Context, r *repo, atoms []atom) ([]string, []dropped
 // applySet applies every atom, settles go.mod and asks the atom gate whether
 // the result is viable.
 func applySet(ctx context.Context, r *repo, atoms []atom) ([]string, error) {
+	before, err := parseGoMod(r)
+	if err != nil {
+		return nil, err
+	}
+
 	summaries := make([]string, 0, len(atoms))
 
 	for _, a := range atoms {
@@ -83,7 +88,17 @@ func applySet(ctx context.Context, r *repo, atoms []atom) ([]string, error) {
 		}
 	}
 
-	err := settle(ctx, r)
+	err = settle(ctx, r)
+	if err != nil {
+		return nil, err
+	}
+
+	after, err := parseGoMod(r)
+	if err != nil {
+		return nil, err
+	}
+
+	err = checkDowngrades(before, after)
 	if err != nil {
 		return nil, err
 	}
diff --git a/tools/bump/gomod.go b/tools/bump/gomod.go
index 20fd2438..1bec7acc 100644
--- a/tools/bump/gomod.go
+++ b/tools/bump/gomod.go
@@ -378,6 +378,44 @@ func noteAttached(f *modfile.File, module, needle string) bool {
 	return false
 }
 
+var errDowngrade = errors.New("upgrade moved a requirement backwards")
+
+// checkDowngrades refuses a set that lowered any requirement. An upgrade run
+// that moves a version backwards means selection resolved something nobody
+// asked for, and unwinding that is a human's rollback commit, not an
+// unattended bump. Diffing go.mod catches it wherever it came from; reading
+// what go get printed only catches what go get did itself.
+//
+// The lockstep partners are exempt because repin lowers them on purpose, back
+// to the version their owner requires.
+func checkDowngrades(before, after *modfile.File) error {
+	exempt := map[string]bool{modGvisor: true, modLibc: true}
+
+	was := make(map[string]string, len(before.Require))
+	for _, req := range before.Require {
+		was[req.Mod.Path] = req.Mod.Version
+	}
+
+	var lowered []string
+
+	for _, req := range after.Require {
+		old, ok := was[req.Mod.Path]
+		if !ok || exempt[req.Mod.Path] {
+			continue
+		}
+
+		if semver.Compare(req.Mod.Version, old) < 0 {
+			lowered = append(lowered, fmt.Sprintf("%s %s -> %s", req.Mod.Path, old, req.Mod.Version))
+		}
+	}
+
+	if len(lowered) > 0 {
+		return fmt.Errorf("%w: %s", errDowngrade, strings.Join(lowered, ", "))
+	}
+
+	return nil
+}
+
 var errToolchainAhead = errors.New("dependencies require a newer Go than the devShell provides")
 
 // checkToolchain catches a dependency that dragged go.mod's go directive above
diff --git a/tools/bump/modproxy_test.go b/tools/bump/modproxy_test.go
index e8ec2f87..086badd9 100644
--- a/tools/bump/modproxy_test.go
+++ b/tools/bump/modproxy_test.go
@@ -9,6 +9,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"golang.org/x/mod/modfile"
 )
 
 // fakeProxy serves the three module proxy endpoints the resolver reads. The
@@ -274,3 +276,64 @@ func TestDescribeChange(t *testing.T) {
 		t.Errorf("describeChange() without a link = %q", plain)
 	}
 }
+
+func TestCheckDowngrades(t *testing.T) {
+	parse := func(t *testing.T, requires string) *modfile.File {
+		t.Helper()
+
+		f, err := modfile.Parse("go.mod", []byte("module example.com/app\n\ngo 1.24\n\nrequire (\n"+requires+")\n"), nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		return f
+	}
+
+	tests := []struct {
+		name    string
+		before  string
+		after   string
+		wantErr bool
+	}{
+		{
+			name:   "upgrade",
+			before: "\tgithub.com/a/b v1.0.0\n",
+			after:  "\tgithub.com/a/b v1.1.0\n",
+		},
+		{
+			name:    "downgrade",
+			before:  "\tgithub.com/a/b v1.2.0\n",
+			after:   "\tgithub.com/a/b v1.1.0\n",
+			wantErr: true,
+		},
+		{
+			// repin lowers these on purpose, back to what their owner requires.
+			name:   "lockstep partners may move backwards",
+			before: "\t" + modGvisor + " v0.0.0-20260301000000-aaaaaaaaaaaa\n\t" + modLibc + " v1.76.0\n",
+			after:  "\t" + modGvisor + " v0.0.0-20260101000000-bbbbbbbbbbbb\n\t" + modLibc + " v1.75.6\n",
+		},
+		{
+			name:   "a new requirement is not a downgrade",
+			before: "\tgithub.com/a/b v1.0.0\n",
+			after:  "\tgithub.com/a/b v1.0.0\n\tgithub.com/c/d v0.1.0\n",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := checkDowngrades(parse(t, tt.before), parse(t, tt.after))
+
+			if tt.wantErr {
+				if !errors.Is(err, errDowngrade) {
+					t.Fatalf("checkDowngrades() error = %v, want %v", err, errDowngrade)
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Errorf("checkDowngrades() error = %v", err)
+			}
+		})
+	}
+}