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.