From 1b780eae3a87f1aad46c83edba7b985e7daff90d Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 11:41:05 +0200 Subject: [PATCH] Put back on a failed gate only a build of the module's own repository A build refused for its repository is still recorded, and a fork carries the commit the module was registered at: the rollback's search for the previous build would have found it and registered it by the back door. --- cmd/mesh-controller/build_source_test.go | 36 ++++++++++++++++++++++++ internal/inventory/gate.go | 15 ++++++++++ 2 files changed, 51 insertions(+) diff --git a/cmd/mesh-controller/build_source_test.go b/cmd/mesh-controller/build_source_test.go index b49ba7bf..6a9301fc 100644 --- a/cmd/mesh-controller/build_source_test.go +++ b/cmd/mesh-controller/build_source_test.go @@ -184,3 +184,39 @@ func asTheOperator(t *testing.T, inv *inventory.Inventory, b link.BuildResult) l } return b } + +// A rollback puts back only a build of the module's own repository: an agent's build of the module's name, +// recorded and refused, at the very commit the machine ran before (a fork carries it), is never registered by +// the back door of a failed gate. +func TestARollbackNeverPutsBackABuildFromAnotherRepository(t *testing.T) { + open := theCatalogue(t) + ctx := t.Context() + inv := open.inventory + fork := onTrunk("build-1791500000000000000", "agent/mesh-catalog", "git", "modules/sudo", + map[string]any{"module": "sudo", "version": "evil"}) + fork.Commit = "c0ffee0123456789" // the commit sudo was registered at + keptAsked(t, inv, fork.ID, "agent/mesh-catalog", false) + if _, _, err := takeIn(ctx, inv, fork); !errors.Is(err, errNotItsSource) { + t.Fatalf("the fork's build was taken in: %v", err) + } + failed := onTrunk("build-1791600000000000000", "novox/mesh-catalog", "git", "modules/sudo", + map[string]any{"module": "sudo", "version": "2"}) + failed.Commit = "badbadbad0123456" + if _, _, err := takeIn(ctx, inv, failed); err != nil { + t.Fatal(err) + } + record, _, err := inv.BuildByID(ctx, failed.ID) + if err != nil { + t.Fatal(err) + } + previous, found, err := inv.PreviousBuild(ctx, "sudo", "c0ffee0123456789", record) + if err != nil { + t.Fatal(err) + } + if found && previous.ID == fork.ID { + t.Fatalf("a rollback would put back the fork's build %s", previous.ID) + } + if !found || previous.ID != "build-sudo" { + t.Fatalf("a rollback puts back %q (found %v), want the registered build-sudo", previous.ID, found) + } +} diff --git a/internal/inventory/gate.go b/internal/inventory/gate.go index 58bc4e80..7baa82e7 100644 --- a/internal/inventory/gate.go +++ b/internal/inventory/gate.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "time" "github.com/jackc/pgx/v5" @@ -169,6 +170,12 @@ func (i *Inventory) PreviousBuild(ctx context.Context, module, commit string, fa if b.ID == failed.ID || (commit != "" && b.Commit != commit) { continue } + // Only a build of the repository the failed build was made from — the module's, since its take-in + // registered it (novox/hq ADR 0266): a build of the module's name from another repository is recorded + // and was never registered, and putting it back would register it now. + if failed.Repository != "" && !sameRepositoryAs(b.Repository, failed.Repository) { + continue + } if !failed.AskedOrAt().IsZero() && !b.AskedOrAt().Before(failed.AskedOrAt()) { continue } @@ -190,6 +197,14 @@ func (i *Inventory) PreviousBuild(ctx context.Context, module, commit string, fa return Build{}, false, nil } +// sameRepositoryAs says two recorded repositories are one, however their case or `.git` is spelled. +func sameRepositoryAs(a, b string) bool { + trim := func(s string) string { + return strings.TrimSuffix(strings.TrimRight(strings.ToLower(strings.TrimSpace(s)), "/"), ".git") + } + return trim(a) == trim(b) +} + // RestoreModule puts a module's registered build back to an earlier one: its manifest, the commit it // was built from, and when it was asked — as now, so the build that failed its gate, asked before, can // never register over it again (issue 219's order). The source's head is left where the merge moved it: