A new module from a repository nothing registered is built from registers on the operator's Approve of its merge (issue 1146, ADR 0432)

cmd/mesh-controller/asker_wire.go

@@ -201,7 +201,7 @@ func routerRecordOf(conn *nats.Conn, inv *inventory.Inventory) func(ctx context.
 }
 
 // asksRecords is the bucket the asks seat's declarer keeps its record of asks in.
-func asksRecords(ctx context.Context, inv *inventory.Inventory) (string, error) {
+func asksRecords(ctx context.Context, inv catalogueReader) (string, error) {
 	declared, err := inv.Catalogue(ctx)
 	if err != nil {
 		return "", err

cmd/mesh-controller/build_source.go

@@ -389,10 +389,31 @@ func (f *sourceForms) onTheForge(repository, seat string) (owner, name string, o
 //     least one required status, no administrator merging past one.
 //
 // Anything else only when the build request was kept as asked at the controller's terminal, for this very
-// repository and path. path is the module's directory as built; branch the trunk it is registered from.
+// repository and path; or, for the build of a merge, when mesh-delivery announced it on the operator's Approve of
+// that head (novox/hq ADR 0420 §7 for a moved module, ADR 0432 for a new one), the forge's checks still holding. path is the module's directory as built; branch the trunk it is registered from.
 func mayRegisterFrom(ctx context.Context, inv *inventory.Inventory, module string, built inventory.Source,
 	buildID, path, branch string) (int64, error) {
-	forms := newSourceForms(ctx, inv)
+	return mayRegisterIn(ctx, inv, newSourceForms(ctx, inv), module, built, buildID, path, branch)
+}
+
+// catalogueReader is the catalogue's manifests, as the router's records are found in them.
+type catalogueReader interface {
+	Catalogue(ctx context.Context) (map[string]catalogue.Manifest, error)
+}
+
+// registrations is what a registration's decision reads of the mesh: *inventory.Inventory, or a test's stand-in.
+type registrations interface {
+	catalogueReader
+	SourceOf(ctx context.Context, module string) (inventory.Source, error)
+	SourceIdentity(ctx context.Context, module string) (int64, error)
+	Catalogued(ctx context.Context) ([]inventory.Entry, error)
+	BuildRequestByID(ctx context.Context, id string) (inventory.BuildRequest, bool, error)
+	MergeOf(ctx context.Context, repository, commit string) (inventory.BatchedMerge, bool, error)
+}
+
+// mayRegisterIn is mayRegisterFrom on what it reads of the mesh handed in.
+func mayRegisterIn(ctx context.Context, inv registrations, forms *sourceForms, module string, built inventory.Source,
+	buildID, path, branch string) (int64, error) {
 	was, err := inv.SourceOf(ctx, module)
 	isNew := errors.Is(err, inventory.ErrNoSuchModule)
 	if err != nil && !isNew {
@@ -404,7 +425,8 @@ func mayRegisterFrom(ctx context.Context, inv *inventory.Inventory, module strin
 	}
 	refuse := func(why string) (int64, error) {
 		return 0, fmt.Errorf("%w: %s. A module is registered from such a source only by a build asked at the "+
-			"controller's terminal, never through a verb: a registered module is what the next push sends, and whoever "+
+			"controller's terminal, or by the build of a merge mesh-delivery announced on the operator's Approve of that "+
+			"head, never through a verb: a registered module is what the next push sends, and whoever "+
 			"may call a verb includes agents (novox/hq ADR 0266)", errNotItsSource, why)
 	}
 
@@ -438,8 +460,11 @@ func mayRegisterFrom(ctx context.Context, inv *inventory.Inventory, module strin
 	// **Or the move the operator approved** (novox/hq ADR 0420 §7, issue 1025): a known module built from another
 	// repository by a merge, whose pull request mesh-delivery merged on the operator's Approve of that head.
 	// A verb alone, or a merge with no such warrant, is refused as before.
+	// **Or a new module from a repository nothing registered is built from, on the same warrant** (novox/hq ADR 0432,
+	// issue 1146): the build of a merge mesh-delivery announced on the operator's Approve of that head registers it.
+	// The forge's protection and the repository's identity are checked below as for every registration.
 	movedWhy := ""
-	if why != "" && !terminal && !isNew {
+	if why != "" && !terminal {
 		accepted, err := approvedMove(ctx, inv, forms, buildID, built)
 		switch {
 		case err == nil:
@@ -507,7 +532,10 @@ func mayRegisterFrom(ctx context.Context, inv *inventory.Inventory, module strin
 				owner, name, m, facts.ID, m, id))
 		}
 	}
-	if movedWhy != "" {
+	if movedWhy != "" && isNew {
+		fmt.Printf("%s: %s is new, from %s/%s — registered at the operator's Approve: %s\n", buildID, module, owner, name,
+			movedWhy)
+	} else if movedWhy != "" {
 		fmt.Printf("%s: %s moved from %s to %s/%s — registered at the operator's Approve: %s\n", buildID, module,
 			sourceWords(was.Repository, was.Seat), owner, name, movedWhy)
 	}
@@ -516,7 +544,7 @@ func mayRegisterFrom(ctx context.Context, inv *inventory.Inventory, module strin
 
 // askedHereFor says the build was asked at the controller's terminal, for this repository and this path: a kept
 // request marked so, whose source is the outcome's (novox/hq ADR 0266).
-func askedHereFor(ctx context.Context, inv *inventory.Inventory, forms *sourceForms, buildID string,
+func askedHereFor(ctx context.Context, inv registrations, forms *sourceForms, buildID string,
 	built inventory.Source, path string) (bool, error) {
 	r, found, err := inv.BuildRequestByID(ctx, buildID)
 	if err != nil || !found || !r.AtTerminal {

cmd/mesh-controller/build_source_integration_test.go

@@ -317,7 +317,7 @@ func TestAModuleMovesOnTheOperatorsApproveOfTheMergeThatMovedIt(t *testing.T) {
 		keptDeliveryMerge = func(_ context.Context, owner, repo string, number int, head string) (deliveryMerge, bool, error) {
 			return a.merge, !c.unkept && owner == "novox" && repo == "mesh-host" && number == 7 && head == a.merge.HeadSHA, nil
 		}
-		routerAskOf = func(_ context.Context, _ *inventory.Inventory, id string) (*asks.Ask, *asks.Warrant, error) {
+		routerAskOf = func(_ context.Context, _ catalogueReader, id string) (*asks.Ask, *asks.Warrant, error) {
 			if id != a.merge.AskID {
 				t.Errorf("%s: the router was asked for %q", c.what, id)
 			}

cmd/mesh-controller/moved.go

@@ -168,7 +168,7 @@ var keptDeliveryMerge = func(ctx context.Context, owner, repo string, number int
 
 // routerAskOf is the router's record of one of mesh-delivery's asks: the ask as published and its warrant. A
 // variable so a test can hand it one.
-var routerAskOf = func(ctx context.Context, inv *inventory.Inventory, id string) (*asks.Ask, *asks.Warrant, error) {
+var routerAskOf = func(ctx context.Context, inv catalogueReader, id string) (*asks.Ask, *asks.Warrant, error) {
 	serving := servingBus.Load()
 	if serving == nil {
 		return nil, nil, errors.New("only the serving controller reads the router's record")
@@ -216,7 +216,7 @@ var errNotAMerge = errors.New("the build is not of a merge")
 // (ADR 0420 §7): the build is of a merge, that merge is a pull request mesh-delivery announced it made, and the
 // ask it made it on holds an Approve by the operator of that head. Answers why it was accepted; an error says why
 // it was not, and the refusal of ADR 0266 stands.
-func approvedMove(ctx context.Context, inv *inventory.Inventory, forms *sourceForms, buildID string,
+func approvedMove(ctx context.Context, inv registrations, forms *sourceForms, buildID string,
 	built inventory.Source) (string, error) {
 	r, found, err := inv.BuildRequestByID(ctx, buildID)
 	if err != nil {

cmd/mesh-controller/new_module_approved_test.go

@@ -0,0 +1,112 @@
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"strings"
+	"testing"
+	"time"
+
+	"git.novox.be/novox/mesh-sdk/go/asks"
+	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/inventory"
+)
+
+// **The operator's Approve of a merge registers a new module from its repository** (novox/hq ADR 0432, issue 1146).
+// The mesh as the decision reads it, with nothing registered from the repository, and the build request and merge the
+// controller kept.
+type anEmptyMesh struct {
+	request inventory.BuildRequest
+	asked   bool
+	merge   inventory.BatchedMerge
+}
+
+func (m *anEmptyMesh) Catalogue(context.Context) (map[string]catalogue.Manifest, error) {
+	return nil, nil
+}
+func (m *anEmptyMesh) SourceOf(context.Context, string) (inventory.Source, error) {
+	return inventory.Source{}, inventory.ErrNoSuchModule
+}
+func (m *anEmptyMesh) SourceIdentity(context.Context, string) (int64, error) { return 0, nil }
+func (m *anEmptyMesh) Catalogued(context.Context) ([]inventory.Entry, error) { return nil, nil }
+func (m *anEmptyMesh) BuildRequestByID(_ context.Context, id string) (inventory.BuildRequest, bool, error) {
+	return m.request, m.asked && id == m.request.ID, nil
+}
+func (m *anEmptyMesh) MergeOf(context.Context, string, string) (inventory.BatchedMerge, bool, error) {
+	return m.merge, true, nil
+}
+
+func TestANewModuleIsRegisteredOnTheOperatorsApproveOfItsMergeAndRefusedOtherwise(t *testing.T) {
+	const commit, head = "c0ffee0123456789", "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1"
+	built := inventory.Source{Repository: "novox/mesh-delivery", Seat: "git"}
+	for _, c := range []struct {
+		what     string
+		change   func(*anApprovedMerge)
+		request  string // "merge", or "build" through a verb
+		unkept   bool
+		forge    any // what the forge says of the repository; nil is protected
+		built    *inventory.Source
+		accepted bool
+		ac       string
+	}{
+		{what: "the Approve of the merged head, a protected trunk on the forge", request: "merge", accepted: true, ac: "1146/AC1"},
+		{what: "a build asked through a verb", request: "build", ac: "1146/AC2"},
+		{what: "a merge with no warrant", request: "merge", unkept: true, ac: "1146/AC2"},
+		{what: "a warrant of another head", request: "merge", ac: "1146/AC2", change: func(a *anApprovedMerge) {
+			a.merge.Heads = map[string]string{"novox/mesh-host@aaaaaaaaaaaa": "e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5"}
+		}},
+		{what: "a repository not on the mesh's forge", request: "merge", ac: "1146/AC3",
+			built: &inventory.Source{Repository: "http://elsewhere.example/x/y.git"}},
+		{what: "a forge that cannot be asked", request: "merge", ac: "1146/AC3", forge: errors.New("connection refused")},
+		{what: "an unprotected trunk", request: "merge", ac: "1146/AC3", forge: forgeFacts{ID: 9, Guarded: false, Why: "direct push"}},
+	} {
+		t.Run(c.ac+" "+c.what, func(t *testing.T) {
+			a := approvedMerge(t)
+			a.merge.Owner, a.merge.Repo, a.merge.Number, a.merge.HeadSHA = "novox", "mesh-delivery", 7, head
+			if c.change != nil {
+				c.change(&a)
+			}
+			wasKept, wasAsk := keptDeliveryMerge, routerAskOf
+			t.Cleanup(func() { keptDeliveryMerge, routerAskOf = wasKept, wasAsk })
+			keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) {
+				return a.merge, !c.unkept, nil
+			}
+			routerAskOf = func(context.Context, catalogueReader, string) (*asks.Ask, *asks.Warrant, error) {
+				return &a.ask, &a.warrant, nil
+			}
+			if c.forge != nil {
+				theForge.Store("novox/mesh-delivery", c.forge)
+				t.Cleanup(func() { theForge.Delete("novox/mesh-delivery") })
+			}
+			event, _ := json.Marshal(map[string]any{"owner": "novox", "repo": "mesh-delivery", "base": "main", "number": 7,
+				"head_sha": head, "merge_commit_sha": commit})
+			mesh := &anEmptyMesh{asked: true, merge: inventory.BatchedMerge{Repository: "novox/mesh-delivery", Branch: "main",
+				Commit: commit, Merged: time.Now(), Event: event},
+				request: inventory.BuildRequest{ID: "build-1", Repository: "novox/mesh-delivery", Seat: "git", Path: "",
+					Ref: "main", Commit: commit, For: c.request}}
+			forms := &sourceForms{bases: map[string]string{}, base: func(seat string) string {
+				if seat == "git" {
+					return "http://forge.internal:20000"
+				}
+				return ""
+			}}
+			src := built
+			if c.built != nil {
+				src = *c.built
+			}
+			id, err := mayRegisterIn(t.Context(), mesh, forms, "mesh-delivery-view", src, "build-1", "", "main")
+			switch {
+			case c.accepted && err != nil:
+				t.Fatalf("%s: the Approved merge's new module was refused: %v", c.ac, err)
+			case c.accepted && id == 0:
+				t.Fatalf("%s: registered with no forge identity to record", c.ac)
+			case !c.accepted && !errors.Is(err, errNotItsSource):
+				t.Fatalf("%s: %s was not refused: %v", c.ac, c.what, err)
+			case !c.accepted && c.ac == "1146/AC2" &&
+				(!strings.Contains(err.Error(), "controller's terminal") || !strings.Contains(err.Error(), "Approve")):
+				t.Fatalf("%s: the refusal does not name both ways in: %v", c.ac, err)
+			}
+		})
+	}
+}

cmd/mesh-controller/new_module_bus_test.go

@@ -0,0 +1,127 @@
+//go:build integration
+
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/nats-io/nats.go/jetstream"
+
+	"github.com/novox/mesh-controller/internal/broker"
+	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+	"github.com/novox/mesh-controller/internal/testbus"
+)
+
+// 1146/AC4: a new module from a repository nothing registered is built from, whole against a real bus and the store
+// (novox/hq ADR 0432, issue 1146; the warrant is read as for a move, ADR 0420 §7, issue 1025): mesh-delivery's word that it
+// merged a pull request is published on the subject the controller consumes and kept in its bucket; the router's
+// record of the ask is read back from the router's bucket by a direct get; and the build of a module from the
+// repository that merge was made in is registered from there only when that record holds the operator's Approve of
+// the merged head. Store-backed: skips without one.
+func TestANewModuleFromANewRepositoryIsRegisteredOnlyOnTheOperatorsApproveOnTheBus(t *testing.T) {
+	for _, c := range []struct {
+		what   string
+		answer bool // the router holds the operator's Approve of the merged heads
+		moved  bool
+	}{
+		{what: "the router holds the operator's Approve", answer: true, moved: true},
+		{what: "the router holds no record of the ask"},
+	} {
+		t.Run(c.what, func(t *testing.T) {
+			open := theCatalogue(t)
+			ctx := t.Context()
+			inv := open.inventory
+			// The module declaring the router's records, as the controller finds them.
+			if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "operator-channel", Version: "1",
+				DefinesSeats: []catalogue.SeatDeclaration{{Name: broker.AsksSeat, Records: []string{"asks"}}}},
+				inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/operator-channel", BuiltFrom: "c0ffee"}); err != nil {
+				t.Fatal(err)
+			}
+
+			js, err := broker.Dial(testbus.URL(t))
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(js.Close)
+			if err := broker.AssertMeshStreams(js); err != nil {
+				t.Fatal(err)
+			}
+			if err := js.EnsureControllerBuckets(); err != nil {
+				t.Fatal(err)
+			}
+			api, err := jetstream.New(js.Conn())
+			if err != nil {
+				t.Fatal(err)
+			}
+			records, err := api.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: broker.BucketName("operator-channel", "asks")})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			a := approvedMerge(t)
+			a.merge.Owner, a.merge.Repo, a.merge.Number = "novox", "mesh-delivery", 7
+			if c.answer {
+				record, _ := json.Marshal(map[string]any{"state": "closed", "ask": a.ask, "warrant": a.warrant})
+				if _, err := records.Put(ctx, theDelivery+"."+a.merge.AskID, record); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			server := link.ConnectNats(js, nil, noListener{})
+			if err := server.KeepsDeliveryMerges(keepingDeliveryMerges{js: server.JetStream}); err != nil {
+				t.Fatal(err)
+			}
+			// The events consumer exists, from now on, before the word is published: Receive would make it too late.
+			if err := broker.AssertServingConsumers(js); err != nil {
+				t.Fatal(err)
+			}
+			serving, stop := context.WithCancel(context.Background())
+			done := make(chan struct{})
+			go func() { defer close(done); _ = server.Serve(serving) }()
+			t.Cleanup(func() { stop(); <-done })
+			servingBus.Store(js)
+			t.Cleanup(func() { servingBus.Store(nil) })
+
+			// mesh-delivery says it merged the pull request, and the controller keeps what it said.
+			body, _ := json.Marshal(a.merge)
+			if _, err := js.Context().Publish(broker.DeliveryMergedSubject, body); err != nil {
+				t.Fatal(err)
+			}
+			becomes(t, 15*time.Second, "the merge being kept", func() bool {
+				_, kept, err := keptDeliveryMerge(ctx, "novox", "mesh-delivery", 7, a.merge.HeadSHA)
+				return err == nil && kept
+			})
+
+			// The forge's merge of that pull request, as the controller heard it, and the build it asked for.
+			const commit = "c0ffee0123456789"
+			event, _ := json.Marshal(map[string]any{"owner": "novox", "repo": "mesh-delivery", "base": "main", "number": 7,
+				"head_sha": a.merge.HeadSHA, "merge_commit_sha": commit})
+			if _, err := inv.AddMerge(ctx, inventory.BatchedMerge{Repository: "novox/mesh-delivery", Branch: "main", Commit: commit,
+				Merged: time.Now(), Heard: time.Now(), Event: event}); err != nil {
+				t.Fatal(err)
+			}
+			id := fmt.Sprintf("build-%d", time.Now().Add(time.Hour).UnixNano())
+			if err := inv.RecordBuildRequest(ctx, inventory.BuildRequest{ID: id, Repository: "novox/mesh-delivery", Seat: "git",
+				Path: "modules/mesh-delivery-view", Ref: "main", Commit: commit, For: "merge"}); err != nil {
+				t.Fatal(err)
+			}
+			moved := onTrunk(id, "novox/mesh-delivery", "git", "modules/mesh-delivery-view", map[string]any{"module": "mesh-delivery-view", "version": "2"})
+			moved.Commit = commit
+			_, _, err = takeIn(ctx, inv, moved)
+			src, _ := inv.SourceOf(ctx, "mesh-delivery-view")
+			if c.moved && (err != nil || src.Repository != "novox/mesh-delivery") {
+				t.Fatalf("the Approved move was not registered from its repository: %v (built from %q)", err, src.Repository)
+			}
+			if !c.moved && (!errors.Is(err, errNotItsSource) || src.Repository != "") {
+				t.Fatalf("a new module with no Approve was not refused as it is: %v (built from %q)", err, src.Repository)
+			}
+		})
+	}
+}