Merge pull request 'An approved merge's first build passes the guard of a build through a verb, and a failed one is recorded (issues 1146, 1169)' (#387) from fix/1146-first-build-ask into main

cmd/mesh-controller/build.go

@@ -473,7 +473,7 @@ func buildOneAsked(ctx context.Context, source buildSource, path, ref string, wa
 		return "", err
 	}
 	// Through a verb, only a repository the catalogue builds from (novox/hq ADR 0266).
-	if err := verbMayAsk(ctx, source, repository); err != nil {
+	if err := verbMayAsk(ctx, source, repository, path); err != nil {
 		return "", err
 	}
 
@@ -812,7 +812,7 @@ func buildAndShow(ctx context.Context, source buildSource, path, ref string, wai
 	if err != nil {
 		return err
 	}
-	if err := verbMayAsk(ctx, source, repository); err != nil {
+	if err := verbMayAsk(ctx, source, repository, path); err != nil {
 		return err
 	}
 	ident, err := openIdentity(ctx)

cmd/mesh-controller/build_source.go

@@ -567,13 +567,49 @@ func sourceWords(repository, seat string) string {
 	return buildSource{Repository: repository, Seat: seat}.String()
 }
 
+// firstBuildWarrant is what the controller checked before it asks the first build of the modules a merge adds to a
+// repository nothing is built from: the merge was made on the operator's Approve of its head (approvedAnnounced). It
+// travels in the context of that ask alone, in this process, so no verb carries one.
+type firstBuildWarrant struct {
+	repository string // owner/name on the git seat
+	paths      map[string]bool
+	why        string
+}
+
+type firstBuildWarrantKey struct{}
+
+// withFirstBuildWarrant is ctx carrying the warrant for asking the first builds of paths in repository.
+func withFirstBuildWarrant(ctx context.Context, repository string, paths []string, why string) context.Context {
+	w := &firstBuildWarrant{repository: repository, paths: map[string]bool{}, why: why}
+	for _, p := range paths {
+		w.paths[strings.Trim(p, "/")] = true
+	}
+	return context.WithValue(ctx, firstBuildWarrantKey{}, w)
+}
+
+func firstBuildWarrantIn(ctx context.Context) *firstBuildWarrant {
+	w, _ := ctx.Value(firstBuildWarrantKey{}).(*firstBuildWarrant)
+	return w
+}
+
+// covers says the warrant is for exactly this ask: the repository on the git seat, and a module path the merge holds.
+func (w *firstBuildWarrant) covers(source buildSource, path string) bool {
+	return w != nil && source.Seat == gitSeat && strings.EqualFold(strings.Trim(source.Repository, "/"), w.repository) &&
+		w.paths[strings.Trim(path, "/")]
+}
+
 // verbMayAsk refuses, for a call through a verb, a build of a repository the catalogue builds no module from
 // (novox/hq ADR 0266): the build node would run what an agent wrote, and its outcome could never be
 // registered anyway. url is the repository as it is cloned. At the terminal anything may be asked.
-func verbMayAsk(ctx context.Context, source buildSource, url string) error {
+func verbMayAsk(ctx context.Context, source buildSource, url, path string) error {
 	if startedAtTheTerminal() {
 		return nil
 	}
+	// **Or the first build of a module a merge adds on the operator's Approve** (novox/hq ADR 0432, issue 1146): the
+	// controller's own ask, carrying the warrant it checked, for that repository and the module paths the merge holds.
+	if firstBuildWarrantIn(ctx).covers(source, path) {
+		return nil
+	}
 	open, err := openStores(ctx)
 	if err != nil {
 		return err

cmd/mesh-controller/build_source_integration_test.go

@@ -134,18 +134,18 @@ func TestAVerbAsksNoBuildOfARepositoryTheCatalogueDoesNotBuildFrom(t *testing.T)
 	ctx := t.Context()
 	t.Setenv(verbVar, "build")
 	t.Setenv(link.CallerVar, "node-tools.anchor, through the mesh-controller seat")
-	err := verbMayAsk(ctx, buildSource{Repository: "agent/sudo", Seat: "git"}, "http://forge.internal:20000/agent/sudo.git")
+	err := verbMayAsk(ctx, buildSource{Repository: "agent/sudo", Seat: "git"}, "http://forge.internal:20000/agent/sudo.git", "")
 	var policy *heldAtTheTerminal
 	if !errors.As(err, &policy) {
 		t.Fatalf("a verb's build of an agent's repository was asked: %v", err)
 	}
 	if err := verbMayAsk(ctx, buildSource{Repository: "novox/mesh-catalog", Seat: "git"},
-		"http://forge.internal:20000/novox/mesh-catalog.git"); err != nil {
+		"http://forge.internal:20000/novox/mesh-catalog.git", ""); err != nil {
 		t.Fatalf("a verb's build of the catalogue repository was refused: %v", err)
 	}
 	t.Setenv(verbVar, "")
 	t.Setenv(link.CallerVar, "")
-	if err := verbMayAsk(ctx, buildSource{Repository: "agent/sudo", Seat: "git"}, ""); err != nil {
+	if err := verbMayAsk(ctx, buildSource{Repository: "agent/sudo", Seat: "git"}, "", ""); err != nil {
 		t.Fatalf("the terminal was refused: %v", err)
 	}
 }

cmd/mesh-controller/first_build_ask_integration_test.go

@@ -0,0 +1,84 @@
+//go:build integration
+
+package main
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"git.novox.be/novox/mesh-sdk/go/asks"
+	"github.com/novox/mesh-controller/internal/broker"
+	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/identity"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// aForgeAndABuildSeat gives a test what a real ask of a build needs: the forge holding the git seat, assigned to anchor, so
+// the clone URL composes; the identity store; the build seat's queue on a real bus; and the environment of the serving controller, which is
+// never the controller's terminal (a verb's, in effect: startedAtTheTerminal is false).
+func aForgeAndABuildSeat(t *testing.T, open *stores) *broker.JetStream {
+	t.Helper()
+	js, _ := aBuildQueue(t)
+	// The ask signs what it sends with the controller's identity, as the serving controller does.
+	identity.ForTest(t)
+	forge, err := catalogue.ParseManifest([]byte(`{"module":"gitea","version":"1",
+	  "claims":[{"name":"git","scope":"mesh"}],"provides":[{"name":"git","scope":"mesh"}],
+	  "serves":{"git":{"scheme":"http","port":3000}}}`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	register(t, open, forge)
+	if _, err := assign(t.Context(), open, "anchor", "gitea"); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv(servedVar, "1")
+	return js
+}
+
+// 1146/AC9, through the real ask (askABuild, buildOneAsked, verbMayAsk) and a real bus: the first build of a module a
+// merge adds on the operator's Approve is queued on the build seat; the same ask without that warrant, and the warrant's
+// ask for a path the merge does not hold, are refused by the guard of a build through a verb.
+func TestTheApprovedFirstBuildPassesTheGuardOfAVerbAndIsQueued(t *testing.T) {
+	const head, commit = "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "0177ea7a00000000"
+	open := aMesh(t)
+	ctx := t.Context()
+	js := aForgeAndABuildSeat(t, open)
+
+	a := aDeliveryMergeOf(t, head, map[string]string{"novox/mesh-delivery@a1a1a1a1a1a1": head})
+	wasKept, wasAsk := keptDeliveryMerge, routerAskOf
+	keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) {
+		return a.merge, true, nil
+	}
+	routerAskOf = func(context.Context, catalogueReader, string) (*asks.Ask, *asks.Warrant, error) {
+		return &a.ask, &a.warrant, nil
+	}
+	t.Cleanup(func() { keptDeliveryMerge, routerAskOf = wasKept, wasAsk })
+
+	// Without the warrant the guard refuses, whatever the repository holds.
+	_, err := askABuild(ctx, buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "", "main")
+	var policy *heldAtTheTerminal
+	if !errors.As(err, &policy) {
+		t.Fatalf("1146/AC9: a first build asked without the warrant was not refused: %v", err)
+	}
+	// And a path the merge does not hold is refused on the warrant.
+	warranted := withFirstBuildWarrant(ctx, "novox/mesh-delivery", []string{""}, "the Approve")
+	if _, err := askABuild(warranted, buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "other", "main"); !errors.As(err, &policy) {
+		t.Fatalf("1146/AC9: the warrant covered a path the merge does not hold: %v", err)
+	}
+
+	m := link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", HeadSHA: head, Commit: commit, Number: 7,
+		MergedAt: time.Now().Add(-time.Hour).UTC().Format(time.RFC3339Nano), Paths: []string{"module.json", "main.go"},
+		ModuleDirs: []string{"."}, ModuleDirsSaid: true}
+	if err := (following{open: open}).SourceMoved(ctx, m); err != nil {
+		t.Fatal(err)
+	}
+	queue, err := link.ReadQueue(ctx, js, link.TheBuildMachine)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(queue.Asks) != 1 {
+		t.Fatalf("1146/AC9: the build queue holds %+v, want the one first build", queue)
+	}
+}

cmd/mesh-controller/first_build_warrant_test.go

@@ -0,0 +1,29 @@
+package main
+
+import "testing"
+
+// 1146/AC9: the warrant the controller checked covers exactly the repository on the git seat and the module paths the
+// merge holds; nothing else passes verbMayAsk on it.
+func TestTheFirstBuildWarrantCoversOnlyItsRepositoryAndItsModulePaths(t *testing.T) {
+	ctx := withFirstBuildWarrant(t.Context(), "novox/mesh-delivery", []string{"", "view"}, "the Approve")
+	w := firstBuildWarrantIn(ctx)
+	for _, c := range []struct {
+		source buildSource
+		path   string
+		covers bool
+	}{
+		{buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "", true},
+		{buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "/view/", true},
+		{buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "other", false},
+		{buildSource{Repository: "agent/mesh-delivery", Seat: "git"}, "", false},
+		{buildSource{Repository: "novox/mesh-delivery"}, "", false},
+		{buildSource{Repository: "novox/mesh-delivery", Seat: "elsewhere"}, "", false},
+	} {
+		if got := w.covers(c.source, c.path); got != c.covers {
+			t.Errorf("1146/AC9: %v at %q covered = %v, want %v", c.source, c.path, got, c.covers)
+		}
+	}
+	if firstBuildWarrantIn(t.Context()).covers(buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "") {
+		t.Error("1146/AC9: a context with no warrant covers an ask")
+	}
+}

cmd/mesh-controller/first_builds.go

@@ -76,7 +76,16 @@ func askFirstBuilds(ctx context.Context, inv *inventory.Inventory, m link.Source
 	}
 	fmt.Printf("%s merged into %s (%.8s) on the operator's Approve (%s): the mesh builds nothing from it yet, so the "+
 		"first build of %s is asked\n", repository, m.Base, m.Commit, why, strings.Join(added, ", "))
-	askBuilds(ctx, inv, m, buildSource{Repository: repository, Seat: gitSeat}, added)
+	// The ask carries the warrant just checked, for this repository and these module paths alone: the guard of a build
+	// through a verb (verbMayAsk) lets exactly that through.
+	paths := make([]string, 0, len(added))
+	for _, dir := range added {
+		if dir == "." {
+			dir = ""
+		}
+		paths = append(paths, dir)
+	}
+	askBuilds(withFirstBuildWarrant(ctx, repository, paths, why), inv, m, buildSource{Repository: repository, Seat: gitSeat}, added)
 	// Answered: it no longer waits, so the word's arrival again asks nothing twice.
 	return inv.AnswerMerge(ctx, repository, m.Commit, inventory.WaitingRecord, false)
 }

cmd/mesh-controller/first_builds_integration_test.go

@@ -5,6 +5,8 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"slices"
+	"sync"
 	"testing"
 	"time"
 
@@ -39,7 +41,25 @@ func TestAMergeOnTheApproveIntoANewRepositoryAsksTheFirstBuildAndItsOutcomeRegis
 			open := aMesh(t)
 			ctx := t.Context()
 			inv := open.inventory
-			asked := asksWithPaths(t)
+			// The real ask, guard and queue included: only its answers are kept, so the build's outcome can be matched.
+			aForgeAndABuildSeat(t, open)
+			// Written by the controller's merge path on its own goroutine, read by the test: under one lock.
+			var mu sync.Mutex
+			var asks [][3]string
+			var ids []string
+			asked := func() [][3]string { mu.Lock(); defer mu.Unlock(); return slices.Clone(asks) }
+			realAsk := askABuild
+			askABuild = func(c context.Context, source buildSource, path, ref string) (string, error) {
+				id, err := realAsk(c, source, path, ref)
+				if err == nil {
+					mu.Lock()
+					asks = append(asks, [3]string{source.Repository, path, ref})
+					ids = append(ids, id)
+					mu.Unlock()
+				}
+				return id, err
+			}
+			t.Cleanup(func() { askABuild = realAsk })
 			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 {
@@ -113,23 +133,26 @@ func TestAMergeOnTheApproveIntoANewRepositoryAsksTheFirstBuildAndItsOutcomeRegis
 				forge()
 			} else {
 				forge()
-				if len(*asked) != 0 {
+				if len(asked()) != 0 {
-					t.Fatalf("1146/AC7: the merge asked %v before mesh-delivery's word was kept", *asked)
+					t.Fatalf("1146/AC7: the merge asked %v before mesh-delivery's word was kept", asked())
 				}
 				word()
 			}
 			if !c.wantRegiter {
-				if len(*asked) != 0 {
+				if len(asked()) != 0 {
-					t.Fatalf("1146/AC7: a merge whose record holds no Approve asked %v", *asked)
+					t.Fatalf("1146/AC7: a merge whose record holds no Approve asked %v", asked())
 				}
 				return
 			}
 			want := [3]string{"novox/mesh-delivery", "", "main"}
-			becomes(t, 10*time.Second, "the first build asked", func() bool { return len(*asked) >= 1 })
+			becomes(t, 10*time.Second, "the first build asked", func() bool { return len(asked()) >= 1 })
-			if len(*asked) != 1 || (*asked)[0] != want {
+			if got := asked(); len(got) != 1 || got[0] != want {
-				t.Fatalf("1146/AC7: the merge asked %v, want %v once", *asked, want)
+				t.Fatalf("1146/AC7: the merge asked %v, want %v once", got, want)
 			}
-			build := onTrunk("b-", "novox/mesh-delivery", "git", "", map[string]any{"module": "mesh-delivery-view", "version": "1"})
+			mu.Lock()
+			first := ids[0]
+			mu.Unlock()
+			build := onTrunk(first, "novox/mesh-delivery", "git", "", map[string]any{"module": "mesh-delivery-view", "version": "1"})
 			build.Commit = commit
 			if _, _, err := takeIn(ctx, inv, build); err != nil {
 				t.Fatalf("1146/AC7: the outcome of the first build was refused: %v", err)

cmd/mesh-controller/not_asked_id_test.go

@@ -0,0 +1,24 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/novox/mesh-controller/internal/chain"
+)
+
+// 1146: a merge's ask that failed is kept under an id the bus takes as a key, for a module at the repository's root
+// (".") as for one in a folder.
+func TestTheIdOfABuildNotAskedIsAKeyTheBusTakes(t *testing.T) {
+	for dir, want := range map[string]string{
+		".":                          "not-asked-0177ea7a",
+		"modules/mesh-delivery-view": "not-asked-0177ea7a-modules-mesh-delivery-view",
+	} {
+		id := notAskedID("0177ea7a00000000", dir)
+		if id != want {
+			t.Errorf("notAskedID(%q) = %q, want %q", dir, id, want)
+		}
+		if !chain.ValidKey("req." + id) {
+			t.Errorf("notAskedID(%q) = %q: the bus takes no key req.%s", dir, id, id)
+		}
+	}
+}

cmd/mesh-controller/upgrades.go

@@ -424,7 +424,7 @@ func askBuilds(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved
 		request := inventory.BuildRequest{ID: id, Repository: source.Repository, Seat: source.Seat, Path: path,
 			Ref: m.Base, Commit: m.Commit, For: "merge"}
 		if err != nil {
-			request.ID = fmt.Sprintf("not-asked-%s-%s", short(m.Commit), strings.ReplaceAll(dir, "/", "-"))
+			request.ID = notAskedID(m.Commit, dir)
 			request.NotAsked = err.Error()
 			recordBuildRequest(ctx, inv, request)
 			fmt.Printf("  %s is a new module in %s/%s and could not be built: %v — a hand `build` of it "+
@@ -439,6 +439,15 @@ func askBuilds(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved
 	return asked
 }
 
+// notAskedID is the id a merge's build that could not be asked is kept under: a key the bus takes, so a module at the
+// repository's root (".") is kept as one in a folder is.
+func notAskedID(commit, dir string) string {
+	if dir == "." || dir == "" {
+		return "not-asked-" + short(commit)
+	}
+	return fmt.Sprintf("not-asked-%s-%s", short(commit), strings.ReplaceAll(strings.Trim(dir, "/"), "/", "-"))
+}
+
 // actingOnMerges keeps one merge acted on at a time (novox/hq issue 266).
 var actingOnMerges sync.Mutex
 

internal/broker/system_exports_live_test.go

@@ -104,15 +104,23 @@ func TestNatsTheSystemAccountsExportsReachMeshOnAReload(t *testing.T) { // 1147/
 	}
 	// Another user connects and disconnects: the watchman hears both, and the events name that user.
 	gitea, _ := connect("one.gitea")
+	// The server says each connection's own events too, the watchman's among them, and may say its connect after the
+	// subscription is made (issue 1169): the event looked for is the one naming the other user, whatever comes before it.
 	hears := func(ch chan *nats.Msg, what string) {
 		t.Helper()
-		select {
+		bound := time.After(5 * time.Second)
-		case m := <-ch:
+		var others []string
-			if !strings.Contains(string(m.Data), "one.gitea") {
+		for {
-				t.Errorf("the %s event does not name the user that connected: %s", what, m.Data)
+			select {
+			case m := <-ch:
+				if strings.Contains(string(m.Data), "one.gitea") {
+					return
+				}
+				others = append(others, string(m.Data))
+			case <-bound:
+				t.Errorf("the watchman did not hear another user's %s; it heard only %q", what, others)
+				return
 			}
-		case <-time.After(5 * time.Second):
-			t.Errorf("the watchman did not hear another user's %s", what)
 		}
 	}
 	hears(connects, "connect")

testdata/store-tests

@@ -165,3 +165,4 @@
 ./cmd/mesh-controller TestAComposedDeclarationCarriesModulesAndBuildsOnlyForANodeThatReadsThem
 ./cmd/mesh-controller TestAPlanPassesATierWhoseMachineNeverReportsAfterTheBoundAndSendsTheNextToTheOthers
 ./cmd/mesh-controller TestAMergeOnTheApproveIntoANewRepositoryAsksTheFirstBuildAndItsOutcomeRegistersTheModule
+./cmd/mesh-controller TestTheApprovedFirstBuildPassesTheGuardOfAVerbAndIsQueued