From bbf70fbda43a480eb432a00a3446d51dc001451d Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 05:21:56 +0200 Subject: [PATCH 1/2] Add the replay of issue 349: a merge acted on late superseded the merge after it (hq issue 349) Two catalogue merges acted on in reverse order left the fixed module planned at the commit without the fix. The replay uses only what the controller had before Listed among the store tests, so a check without a database cannot pass its skip. --- cmd/mesh-controller/replay349_test.go | 67 +++++++++++++++++++++++++++ testdata/store-tests | 1 + 2 files changed, 68 insertions(+) create mode 100644 cmd/mesh-controller/replay349_test.go diff --git a/cmd/mesh-controller/replay349_test.go b/cmd/mesh-controller/replay349_test.go new file mode 100644 index 00000000..69e9e4db --- /dev/null +++ b/cmd/mesh-controller/replay349_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "testing" + "time" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// Replay of novox/hq issue 349 (2026-10-09): two merges into the catalogue's main were acted on in reverse order. +// The later one (a082615b, a security fix to the forge's module) was planned first; the earlier one (8ff8197a, +// another module), handed over late, was planned after it, and its plan superseded the later merge's at tier 0 +// with "gitea planned there again": the forge's module was to be built for the commit without the fix. Written +// only with what the controller had before its fix (mesh-controller #179): a mesh, two registered modules of the +// catalogue, the merges as the forge announces them, the plans the inventory keeps — so it is laid over the +// commit before. +func TestReplay349(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + asksWithPaths(t) + for _, module := range []string{"app", "notes"} { + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: module, Version: "1"}, + inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/" + module, Ref: "main", + BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } + } + // Both made after the modules were last looked at, so neither is history. + made := time.Now().UTC().Add(time.Minute) + merge := func(commit, module string, at time.Time) link.SourceMoved { + return link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: commit, + MergedAt: at.Format(time.RFC3339Nano), Paths: []string{"modules/" + module + "/module.json"}, + ModuleDirs: []string{"modules/" + module}, ModuleDirsSaid: true} + } + const fix, earlier = "a082615bfix", "8ff8197aearlier" + // The later merge, the fix to notes, is acted on first; the earlier one, moving app, after it. + if err := (following{open: open}).SourceMoved(ctx, merge(fix, "notes", made.Add(time.Minute))); err != nil { + t.Fatal(err) + } + if err := (following{open: open}).SourceMoved(ctx, merge(earlier, "app", made)); err != nil { + t.Fatal(err) + } + + plans, err := inv.RecentPlans(ctx, 20) + if err != nil { + t.Fatal(err) + } + // A plan superseded hands its modules to the plan that took it over; the plans left standing are what the mesh + // builds. One of them builds the fixed module, and every one that does builds it at the commit holding the fix. + built := 0 + for _, p := range plans { + t.Logf("%s %s at %.8s: %v %s", p.ID, p.State, p.Commit, p.Tiers, p.Note) + if _, holds := p.Modules["notes"]; !holds || p.State == inventory.PlanSuperseded { + continue + } + built++ + if p.Commit != fix { + t.Errorf("%s plans the fixed module notes at %s, a commit without the fix %s: %s", p.ID, p.Commit, fix, p.Note) + } + } + if built == 0 { + t.Error("no plan left standing builds notes, the module the later merge fixed") + } +} diff --git a/testdata/store-tests b/testdata/store-tests index 1a2b6c6c..2c024822 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -41,3 +41,4 @@ ./cmd/mesh-controller TestPlanAndNodeListAnOptionalRequirementLeftUnbound ./cmd/mesh-controller TestReplay340 ./cmd/mesh-controller TestOnlyATrustedFilesNamedKeyIsRefusedThroughAVerb +./cmd/mesh-controller TestReplay349 From 5740874ce3b99b47c27403f33e6c6d2a334ad3d7 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 05:42:18 +0200 Subject: [PATCH 2/2] Drive the replay of issue 349 to where main decides (review of #290) Since ADR 0276 the earlier merge waits in a queued batch with no modules, and the replay skipped it, so its pass on main proved nothing. The replay now ends the later merge's walk and has another repository's merge heard, which cuts the batch, and fails on any plan of the two merges left standing with nothing planned. --- cmd/mesh-controller/replay349_test.go | 63 ++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/cmd/mesh-controller/replay349_test.go b/cmd/mesh-controller/replay349_test.go index 69e9e4db..fae683a2 100644 --- a/cmd/mesh-controller/replay349_test.go +++ b/cmd/mesh-controller/replay349_test.go @@ -15,7 +15,9 @@ import ( // with "gitea planned there again": the forge's module was to be built for the commit without the fix. Written // only with what the controller had before its fix (mesh-controller #179): a mesh, two registered modules of the // catalogue, the merges as the forge announces them, the plans the inventory keeps — so it is laid over the -// commit before. +// commit before. Since novox/hq ADR 0276 the earlier merge waits in a batch behind the later merge's walk and is +// planned only when that walk ends and the cutter looks again; the replay ends the walk and has a merge of another +// repository heard, which looks, and a plan of the two merges left standing with nothing planned fails it. func TestReplay349(t *testing.T) { open := aMesh(t) ctx := t.Context() @@ -28,6 +30,10 @@ func TestReplay349(t *testing.T) { t.Fatal(err) } } + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "other", Version: "1"}, + inventory.Source{Repository: "novox/other", Seat: "git", Ref: "main", BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } // Both made after the modules were last looked at, so neither is history. made := time.Now().UTC().Add(time.Minute) merge := func(commit, module string, at time.Time) link.SourceMoved { @@ -44,24 +50,57 @@ func TestReplay349(t *testing.T) { t.Fatal(err) } + // The later merge's plan builds and ends; then a merge of another repository comes. Whatever holds the earlier + // merge back while that plan runs (a batch queued behind it, novox/hq ADR 0276) is decided by now: every plan + // of the two merges left standing says what it builds. plans, err := inv.RecentPlans(ctx, 20) if err != nil { t.Fatal(err) } - // A plan superseded hands its modules to the plan that took it over; the plans left standing are what the mesh - // builds. One of them builds the fixed module, and every one that does builds it at the commit holding the fix. - built := 0 for _, p := range plans { - t.Logf("%s %s at %.8s: %v %s", p.ID, p.State, p.Commit, p.Tiers, p.Note) - if _, holds := p.Modules["notes"]; !holds || p.State == inventory.PlanSuperseded { - continue - } - built++ - if p.Commit != fix { - t.Errorf("%s plans the fixed module notes at %s, a commit without the fix %s: %s", p.ID, p.Commit, fix, p.Note) + if p.Commit == fix && p.Open() { + p.State = inventory.PlanDone + if err := inv.SavePlan(ctx, &p); err != nil { + t.Fatal(err) + } } } - if built == 0 { + if err := (following{open: open}).SourceMoved(ctx, link.SourceMoved{Owner: "novox", Repo: "other", Base: "main", + Commit: "c1other", MergedAt: made.Add(2 * time.Minute).Format(time.RFC3339Nano), + Paths: []string{"main.go"}}); err != nil { + t.Fatal(err) + } + if plans, err = inv.RecentPlans(ctx, 20); err != nil { + t.Fatal(err) + } + // A plan superseded hands its modules to the plan that took it over; the plans left standing are what the mesh + // builds. Each plan of the two merges left standing plans modules; one builds app, the earlier merge's module; + // one builds notes, the module the later merge fixed, and every one that does builds it at the commit holding + // the fix. + app, notes := 0, 0 + for _, p := range plans { + t.Logf("%s %s at %.8s: %v %s", p.ID, p.State, p.Commit, p.Tiers, p.Note) + if p.State == inventory.PlanSuperseded || (p.Commit != fix && p.Commit != earlier) { + continue + } + if len(p.Modules) == 0 { + t.Errorf("%s, of the merge at %s, is left standing and plans nothing: %s", p.ID, p.Commit, p.Note) + } + if _, holds := p.Modules["app"]; holds { + app++ + } + if _, holds := p.Modules["notes"]; holds { + notes++ + if p.Commit != fix { + t.Errorf("%s plans the fixed module notes at %s, a commit without the fix %s: %s", p.ID, p.Commit, fix, + p.Note) + } + } + } + if app == 0 { + t.Error("no plan left standing builds app, the module the earlier merge moved") + } + if notes == 0 { t.Error("no plan left standing builds notes, the module the later merge fixed") } }