From ea09f074db01f4d39eeb500bd9fbbe20bcda9497 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 7 Oct 2026 22:31:30 +0200 Subject: [PATCH] Build and register a new module when its merge lands (hq issue 300) The delivery plan said a merge adding a module builds it, sent nowhere; the merge built nothing, so the module was never registered and assign refused it. The merge now asks for the build of every directory it adds, outside the plan, and the take-in registers it like a hand build. --- .../new_module_on_merge_test.go | 94 +++++++++++++++++++ cmd/mesh-controller/upgrades.go | 49 +++++++++- 2 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 cmd/mesh-controller/new_module_on_merge_test.go diff --git a/cmd/mesh-controller/new_module_on_merge_test.go b/cmd/mesh-controller/new_module_on_merge_test.go new file mode 100644 index 00000000..f5080df1 --- /dev/null +++ b/cmd/mesh-controller/new_module_on_merge_test.go @@ -0,0 +1,94 @@ +package main + +import ( + "context" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// asksWithPaths records every build a merge asks for, as repository, path and ref. +func asksWithPaths(t *testing.T) *[][3]string { + t.Helper() + var asked [][3]string + was := askABuild + askABuild = func(_ context.Context, source buildSource, path, ref string) (string, error) { + asked = append(asked, [3]string{source.Repository, path, ref}) + return "b-" + path, nil + } + t.Cleanup(func() { askABuild = was }) + return &asked +} + +// The live case of 2026-10-07 (novox/hq issue 300), replayed: a merge of the catalogue adds one module's +// directory and touches nothing else the mesh holds. Its delivery plan said "builds new: …, sent nowhere"; +// the merge said it changed nothing any module the mesh holds is built from, and built nothing, so the +// module was never registered and `assign` refused it. The merge asks for its build, at the branch merged +// into, from the repository as the mesh spells it — and opens no plan, since nothing it holds moved. +func TestAMergeBuildsTheNewModuleItAdds(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + asked := asksWithPaths(t) + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "networkmanager", Version: "1"}, + inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/networkmanager", Ref: "main", + BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } + m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "3da80a4b00", + Paths: []string{"modules/systemd-resolved/module.json", "modules/systemd-resolved/cmd/main.go"}, + ModuleDirs: []string{"modules/systemd-resolved"}, ModuleDirsSaid: true} + if err := (following{open: open}).SourceMoved(ctx, m); err != nil { + t.Fatal(err) + } + want := [3]string{"novox/mesh-catalog", "modules/systemd-resolved", "main"} + if len(*asked) != 1 || (*asked)[0] != want { + t.Fatalf("the merge adding modules/systemd-resolved asked for %v, not %v", *asked, want) + } + if plans, err := inv.OpenPlans(ctx); err != nil || len(plans) != 0 { + t.Fatalf("a merge moving nothing the mesh holds opened a plan: %+v %v", plans, err) + } +} + +// A merge that changes a held module and adds a new one does both: the held one walks its plan, the new +// one is asked for beside it. A merge adding nothing builds nothing new. +func TestAMergeBuildsItsNewModuleBesideItsPlan(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + asked := asksWithPaths(t) + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "app", Version: "1"}, + inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/app", Ref: "main", + BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } + m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "c1aaaaaaaa", + Paths: []string{"modules/app/index.ts", "modules/fresh/module.json"}, + ModuleDirs: []string{"modules/app", "modules/fresh"}, ModuleDirsSaid: true} + if err := (following{open: open}).SourceMoved(ctx, m); err != nil { + t.Fatal(err) + } + paths := map[string]bool{} + for _, a := range *asked { + paths[a[1]] = true + } + if !paths["modules/fresh"] || !paths["modules/app"] || len(*asked) != 2 { + t.Fatalf("asked %v; want the new module and the plan's first tier", *asked) + } + plans, err := inv.OpenPlans(ctx) + if err != nil || len(plans) != 1 || plans[0].Modules["fresh"] != nil || plans[0].Modules["app"] == nil { + t.Fatalf("the plan should walk app alone: %+v %v", plans, err) + } + + *asked = nil + m2 := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "c2aaaaaaaa", + Paths: []string{"README.md"}, ModuleDirsSaid: true} + if err := (following{open: open}).SourceMoved(ctx, m2); err != nil { + t.Fatal(err) + } + if len(*asked) != 0 { + t.Fatalf("a merge adding no module asked for %v", *asked) + } +} diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 14901270..c44bf035 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -345,7 +345,7 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { e.Manifest.Module, m.Owner, m.Repo, e.Source.Ref, m.Base) } } - touched := whatTheMergeTouched(from, entries, m) + touched, added, _ := touchedBy(from, entries, m) // **A module the merge deleted is not built** (novox/hq ADR 0236): its manifest is gone, so the build // seat finds nothing saying what it is, and the plan failed on it (`has no module.json at …`) with // every other module of its tier left unsent. It is forgotten where nothing holds it, said otherwise. @@ -358,8 +358,17 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { return notNow(err) } } + // **A new module is built and registered, and sent nowhere** (novox/hq issue 300): the delivery plan + // says so, and assigning it is a person's act, which needs it registered first. + built := askNewModules(ctx, m, from, added) moved := append(append([]inventory.Entry{}, touched...), packaging...) if len(moved) == 0 { + if len(built) > 0 { + fmt.Printf("%s/%s merged into %s (%.8s); it changed no module the mesh holds, and adds %s: "+ + "built, registered when the build lands, and sent nowhere\n", m.Owner, m.Repo, m.Base, m.Commit, + strings.Join(built, ", ")) + return nil + } fmt.Printf("%s/%s merged into %s (%.8s); it changed nothing any module the mesh holds is "+ "built from\n", m.Owner, m.Repo, m.Base, m.Commit) return nil @@ -471,6 +480,41 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { return nil } +// askNewModules asks the build seat for every module a merge adds to the repository — a directory holding a +// module.json that no module of the mesh is known from (touchedBy's `added`) — at the branch merged into, +// not waited for: the build's take-in registers it, as a hand `build` does, and sends it nowhere, since no +// machine is assigned it (novox/hq issue 300). Before this a merge said "it changed nothing any module +// the mesh holds is built from", the module was never registered, and `assign` refused it as unknown. +// +// The repository is spelled, and found on the seat, as the modules already built from it are; `from` is +// those of them this merge moves, so a merge read as history, or of a branch nothing follows, builds no +// new module either. Outside the plan: the plan walks modules the catalogue holds, and a new one has no +// machine to send to and nothing standing on it. What could not be asked is said, and left to `build`. +// Returns the directories asked for. +func askNewModules(ctx context.Context, m link.SourceMoved, from []inventory.Entry, added []string) []string { + if len(added) == 0 || len(from) == 0 { + return nil + } + source := buildSource{Repository: from[0].Source.Repository, Seat: from[0].Source.Seat} + var asked []string + for _, dir := range added { + path := dir + if path == "." { + path = "" + } + id, err := askABuild(ctx, source, path, m.Base) + if err != nil { + fmt.Printf(" %s is a new module in %s/%s and could not be built: %v — a hand `build` of it "+ + "asks again\n", dir, m.Owner, m.Repo, err) + continue + } + fmt.Printf(" %s is a new module in %s/%s: build %s asked at %s; registered when it lands, assigned "+ + "nowhere\n", dir, m.Owner, m.Repo, id, m.Base) + asked = append(asked, dir) + } + return asked +} + // actingOnMerges keeps one merge acted on at a time (novox/hq issue 266). var actingOnMerges sync.Mutex @@ -655,7 +699,8 @@ func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved // // `added` is the directories the change holds a module in that no module of this repository is known // from — said by the announcer at the head (issue 278), or a module.json among the changed files — `.` -// for the root: a new module, which a check judges before it merges and a merge does not build. +// for the root: a new module, which a check judges before it merges and the merge builds and registers, +// sending it nowhere (askNewModules, novox/hq issue 300). // // Nothing said about the files, or not all of them said, is still everything: what is not known cannot // be narrowed.