Merge pull request 'Build and register a new module when its merge lands (hq issue 300)' (#122) from fix/a-new-module-is-built-on-merge into main

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)
+	}
+}

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.