The trunk's manifest validator is built without a group's heads; the package function's tests fold into existing ones (issue 1921)

Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i

modules/mesh-delivery-checker/internal/check/examiner_test.go

@@ -1138,8 +1138,24 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) {
 		}) {
 			t.Errorf("%s: the case says %v, want the new indexer's manifest said accepted by the trunk's manifest validator", row.name, cs.notes)
 		}
-		for _, j := range judged {
-			if pkgs, _ := buildstep.PackagesArg(j.Fetch["packages"]); !row.try && pkgs[delivery.ForgeModulePrefix+"mesh-networking"] != head2 {
+		// Issue 1921: the judge of the change is built at the group's heads (also a member that publishes no package yet), but
+		// the trunk's manifest validator that rules on a new indexer is built at the latest published versions alone, never
+		// at a group's head (ADR 0633 clause 6): a group with a member of its own then judges the indexer a second time.
+		trunkRuns := 0
+		if row.indexer != "" && row.indexer != "heldplain" && (row.ownJudge || !row.try) {
+			trunkRuns = 1
+		}
+		if want := 1 + trunkRuns; len(judged) != want {
+			t.Fatalf("%s: %d judge asks %+v, want %d", row.name, len(judged), judged, want)
+		}
+		for i, j := range judged {
+			pkgs, _ := buildstep.PackagesArg(j.Fetch["packages"])
+			atGroup := pkgs[delivery.ForgeModulePrefix+"mesh-networking"] == head2
+			switch trunkRun := trunkRuns == 1 && i == len(judged)-1; {
+			case row.try:
+			case trunkRun && atGroup:
+				t.Errorf("%s: the trunk's manifest validator is built with the group's head: %v", row.name, pkgs)
+			case !trunkRun && !atGroup:
 				t.Errorf("%s: the judge at %v is fetched with the packages %v, want the group's mesh-networking member at its head %s", row.name, j.Fetch["commit"], pkgs, head2)
 			}
 		}
@@ -1148,6 +1164,8 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) {
 		}
 		if row.ownJudge {
 			judged = judged[1:]
+		} else if trunkRuns == 1 {
+			judged = judged[1:] // the group's run; the trunk's is the one read below
 		}
 		if len(judged) != 1 || judged[0].Fetch["repository"] != DeliveryRepository || judged[0].Fetch["commit"] != trunk ||
 			judged[0].Package != JudgePackage || judged[0].Dir != JudgeDir {

modules/mesh-delivery-checker/internal/check/manifests_judge.go

@@ -120,7 +120,7 @@ func (st *state) judgeManifests(ctx context.Context) *delivery.Verdict {
 		req.Predefined = list
 		st.c.note("the manifests are judged against the predefined keys of " + from)
 	}
-	cv, trunkCase, v := st.runJudge(ctx, req, touched, commit, at)
+	cv, trunkCase, v := st.runJudge(ctx, req, touched, commit, at, st.packagesOf(DeliveryRepository))
 	if v != nil {
 		return v
 	}
@@ -177,12 +177,12 @@ func (st *state) judgeManifests(ctx context.Context) *delivery.Verdict {
 
 // runJudge asks the Go build seat to build the judge at the commit and run it over the request, and reads its change and
 // trunk cases; a failed verdict when it cannot be asked or does not answer.
-func (st *state) runJudge(ctx context.Context, req composer.Request, touched []string, commit, at string) (change, trunk composer.CaseVerdict, v *delivery.Verdict) {
+func (st *state) runJudge(ctx context.Context, req composer.Request, touched []string, commit, at string, versions delivery.PackageVersions) (change, trunk composer.CaseVerdict, v *delivery.Verdict) {
 	raw, err := json.Marshal(req)
 	if err != nil {
 		return change, trunk, fail(goModule, err.Error())
 	}
-	job := delivery.ComposeJob{Fetch: withPackages(map[string]any{"repository": DeliveryRepository, "commit": commit}, st.packagesOf(DeliveryRepository)), Dir: JudgeDir,
+	job := delivery.ComposeJob{Fetch: withPackages(map[string]any{"repository": DeliveryRepository, "commit": commit}, versions), Dir: JudgeDir,
 		Package: JudgePackage, Request: raw, Reader: "the indexer's judge at " + at}
 	a := delivery.StepAsk{ID: buildstep.NewID(), Kind: delivery.AskCheck, Language: delivery.AskGo, Asker: delivery.CheckerModule,
 		At: st.j.now().UTC(), Compose: &job}
@@ -232,7 +232,10 @@ func (st *state) refuseUnmovableIndexer(ctx context.Context, req composer.Reques
 	if indexer == "" {
 		return nil
 	}
-	if st.judgeHead != "" {
+	// The validator the running parts hold is the trunk's, built at the latest published versions alone, never at the group's
+	// heads (ADR 0633 clause 6): a group's mesh-sdk or mesh-contracts head must not change the validator that judges the new
+	// indexer. The head's run is reused when it was that validator already.
+	if trunkVersions := st.trunkJudgeVersions(); st.judgeHead != "" || !maps.Equal(st.packagesOf(DeliveryRepository), trunkVersions) {
 		trunk := ""
 		if st.j.Trunks != nil {
 			trunk = st.j.Trunks.Heads()[DeliveryRepository]
@@ -242,7 +245,7 @@ func (st *state) refuseUnmovableIndexer(ctx context.Context, req composer.Reques
 		}
 		var v *delivery.Verdict
 		judgedAt = "mesh-delivery's trunk " + trunk
-		if cv, _, v = st.runJudge(ctx, req, touched, trunk, judgedAt); v != nil {
+		if cv, _, v = st.runJudge(ctx, req, touched, trunk, judgedAt, trunkVersions); v != nil {
 			return v
 		}
 	}
@@ -340,3 +343,9 @@ func (st *state) judgedByMember(p Pull, merged []Fetched) {
 		}
 	}
 }
+
+// trunkJudgeVersions are the versions the trunk's manifest validator is built at: the latest published version of each
+// package, with no group member's head (ADR 0633 clause 6).
+func (st *state) trunkJudgeVersions() delivery.PackageVersions {
+	return packagesFor(st.j.Published, &Case{}, DeliveryRepository)
+}

modules/mesh-delivery-checker/internal/check/pins_test.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 	"testing"
 
+	"git.novox.be/novox/mesh-delivery/sdk/grouppackages"
 	"git.novox.be/novox/mesh-sdk/go/buildstep"
 	"git.novox.be/novox/mesh-sdk/go/delivery"
 )
@@ -69,6 +70,39 @@ func TestAGroupMemberIsBuiltAgainstTheSDKMembersHeadWhateverItsGoModNames(t *tes
 	if got := packagesOfFetch(t, b, repoA); len(got) != 1 || got["git.novox.be/novox/mesh-host"] != head2 {
 		t.Errorf("the member's packages = %v, want the unpublished member's root at its head %s alone", got, head2)
 	}
+	// The function every producer takes the versions from (issue 1921): rows for what a build at a group's heads is asked.
+	h := func(c string) string { return strings.Repeat(c, 40) }
+	const (
+		contracts = "git.novox.be/novox/mesh-contracts/go/clients/assignerclient"
+		tools     = "git.novox.be/novox/mesh-tools/go/x"
+	)
+	for _, tc := range []struct {
+		name   string
+		latest delivery.PackageVersions
+		heads  map[string]string
+		own    string
+		want   delivery.PackageVersions
+	}{
+		{"none", nil, nil, "novox/mesh-host", nil},
+		{"trunk latest alone", delivery.PackageVersions{tools: h("a")}, nil, "novox/mesh-host", delivery.PackageVersions{tools: h("a")}},
+		{"a member at its head replaces the latest", delivery.PackageVersions{SDKModulePath: h("a")}, map[string]string{SDKRepository: h("b")}, "novox/mesh-host",
+			delivery.PackageVersions{SDKModulePath: h("b")}},
+		{"a member that publishes no package yet is named by its root module", delivery.PackageVersions{SDKModulePath: h("a")},
+			map[string]string{"novox/mesh-contracts": h("c")}, "novox/mesh-host",
+			delivery.PackageVersions{SDKModulePath: h("a"), "git.novox.be/novox/mesh-contracts": h("c")}},
+		{"mesh-sdk is named by its go module before any package of it is known", nil, map[string]string{SDKRepository: h("b")}, "novox/mesh-host",
+			delivery.PackageVersions{SDKModulePath: h("b")}},
+		{"a member whose packages are known gets no root", delivery.PackageVersions{contracts: h("a")},
+			map[string]string{"novox/mesh-contracts": h("c")}, "novox/mesh-host", delivery.PackageVersions{contracts: h("c")}},
+		{"the build's own repository is left out", delivery.PackageVersions{contracts: h("a")},
+			map[string]string{"novox/mesh-contracts": h("c")}, "novox/mesh-contracts", nil},
+		{"a head that is no full commit names nothing", nil, map[string]string{"novox/mesh-contracts": "abc"}, "novox/mesh-host", nil},
+	} {
+		got := grouppackages.Heads(tc.latest, tc.heads, tc.own)
+		if !maps.Equal(got, tc.want) {
+			t.Errorf("%s: got %v, want %v", tc.name, got, tc.want)
+		}
+	}
 }
 
 type runsAt map[string]string

modules/mesh-delivery-indexer/internal/index/previews_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"maps"
 	"os"
 	"runtime"
 	"slices"
@@ -108,6 +109,42 @@ func TestACandidatesModuleGetsTheFingerprintATrunkModuleOfTheSameTreeHas(t *test
 		t.Errorf("candidate base = %d, want the source revision 5 its preview was read onto", c.Base)
 	}
 	written(t, writtenCandidate{Source: sourceRev{Rev: 5, Heads: map[string]string{catalogueRepo: trunkNow}}, Candidate: c})
+	// Issue 1921: a candidate's build is asked every member of the group at its head, also one that publishes no package yet
+	// and one the module names no package of; the fingerprint stays the trunk's, so the build is the one the merge keeps.
+	for _, tc := range []struct {
+		name    string
+		members map[string]string // the other pull requests of the group: repository, head
+		want    delivery.PackageVersions
+	}{
+		{"a group of one names nothing", nil, nil},
+		{"a member that publishes no package yet", map[string]string{"novox/mesh-contracts": h2},
+			delivery.PackageVersions{delivery.ForgeModulePrefix + "mesh-contracts": h2}},
+		{"mesh-sdk by its go module", map[string]string{"novox/mesh-sdk": h2, "novox/mesh-contracts": h3},
+			delivery.PackageVersions{delivery.ForgeModulePrefix + "mesh-sdk/go": h2, delivery.ForgeModulePrefix + "mesh-contracts": h3}},
+	} {
+		ix, mods, cands := previewBed(t, true)
+		trunk, _ := moduleOf(t, mods, "avahi")
+		if err := ix.Apply(context.Background(), avahiPull(t, h1)); err != nil {
+			t.Fatal(err)
+		}
+		for repository, head := range tc.members {
+			other := forgemsg.Forge{V: delivery.Version, Kind: forgemsg.ForgePull, Repository: repository, Branch: "feat/x", Number: 7, Head: head,
+				Action: forgemsg.PullUpdated}
+			if err := ix.Apply(context.Background(), other); err != nil {
+				t.Fatal(err)
+			}
+		}
+		applyPreview(t, ix, avahiPreview(t, trunkNow, h1))
+		var got delivery.CandidateModule
+		for _, p := range candidateOf(t, cands).Pulls {
+			if p.Repository == catalogueRepo {
+				got = p.Fps["avahi"]
+			}
+		}
+		if got.FP2 != trunk.FP2 || !maps.Equal(got.Packages, tc.want) {
+			t.Errorf("%s: candidate avahi = %+v, want the trunk's fingerprint %s and the packages %v", tc.name, got, trunk.FP2, tc.want)
+		}
+	}
 	// Another tree gives another fingerprint.
 	pv := avahiPreview(t, trunkNow, h1)
 	m := pv.Modules["modules/avahi"]
@@ -138,55 +175,6 @@ func TestACandidatesModuleGetsTheFingerprintATrunkModuleOfTheSameTreeHas(t *test
 	}
 }
 
-// Issue 1921: a candidate's build is asked every member of the group at its head, also one that publishes no package yet
-// and one the module names no package of; the fingerprint stays the trunk's, so the build is the one the merge keeps.
-func TestACandidatesBuildNamesEveryMemberOfItsGroupAtItsHead(t *testing.T) {
-	for _, tc := range []struct {
-		name    string
-		members map[string]string // the other pull requests of the group: repository, head
-		want    delivery.PackageVersions
-	}{
-		{"a group of one names nothing", nil, nil},
-		{"a member that publishes no package yet", map[string]string{"novox/mesh-contracts": h2},
-			delivery.PackageVersions{delivery.ForgeModulePrefix + "mesh-contracts": h2}},
-		{"mesh-sdk by its go module", map[string]string{"novox/mesh-sdk": h2, "novox/mesh-contracts": h3},
-			delivery.PackageVersions{delivery.ForgeModulePrefix + "mesh-sdk/go": h2, delivery.ForgeModulePrefix + "mesh-contracts": h3}},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			ix, mods, cands := previewBed(t, true)
-			trunk, _ := moduleOf(t, mods, "avahi")
-			if err := ix.Apply(context.Background(), avahiPull(t, h1)); err != nil {
-				t.Fatal(err)
-			}
-			for repository, head := range tc.members {
-				other := forgemsg.Forge{V: delivery.Version, Kind: forgemsg.ForgePull, Repository: repository, Branch: "feat/x", Number: 7, Head: head,
-					Action: forgemsg.PullUpdated}
-				if err := ix.Apply(context.Background(), other); err != nil {
-					t.Fatal(err)
-				}
-			}
-			applyPreview(t, ix, avahiPreview(t, trunkNow, h1))
-			var got delivery.CandidateModule
-			for _, p := range candidateOf(t, cands).Pulls {
-				if p.Repository == catalogueRepo {
-					got = p.Fps["avahi"]
-				}
-			}
-			if got.FP2 != trunk.FP2 {
-				t.Fatalf("candidate avahi = %+v, want the trunk's fingerprint %s", got, trunk.FP2)
-			}
-			if len(got.Packages) != len(tc.want) {
-				t.Fatalf("packages = %v, want %v", got.Packages, tc.want)
-			}
-			for pkg, head := range tc.want {
-				if got.Packages[pkg] != head {
-					t.Errorf("%s = %q, want %q", pkg, got.Packages[pkg], head)
-				}
-			}
-		})
-	}
-}
-
 func TestAnUnknownToolchainLeavesTheCandidatesModuleNamingIt(t *testing.T) { // 1445/AC2
 	ix, _, cands := previewBed(t, false)
 	if err := ix.Apply(context.Background(), avahiPull(t, h1)); err != nil {

sdk/grouppackages/grouppackages.go

@@ -6,19 +6,29 @@
 package grouppackages
 
 import (
+	"strings"
+
 	"git.novox.be/novox/mesh-sdk/go/delivery"
+	"git.novox.be/novox/mesh-sdk/go/sdkversion"
 )
 
-// RootModules are the repositories whose Go module is not at their root, by repository: the module a member of that
-// repository stands for when no package of it is known yet.
-var RootModules = map[string]string{
-	"novox/mesh-sdk": delivery.ForgeModulePrefix + "mesh-sdk/go",
+// rootOf is the module a member of repo stands for when no package of it is known: its root module path, or for the
+// repository of mesh-sdk its Go module (sdkversion.Module, which is not at the root).
+func rootOf(repo string) (string, bool) {
+	if sdkRepo, _, ok := delivery.PackageRepository(sdkversion.Module); ok && delivery.SameRepository(repo, sdkRepo) {
+		return sdkversion.Module, true
+	}
+	name, found := strings.CutPrefix(repo, "novox/")
+	if !found || name == "" {
+		return "", false
+	}
+	return delivery.ForgeModulePrefix + name, true
 }
 
 // Heads is the version of each mesh package a build of own builds against: latest (the latest published version of each
 // package the mesh's modules use, nil for none) with every package of a group member at that member's head (heads, by
 // repository <owner>/<name>, a full commit each), and for a member of which latest holds no package, the member's root
-// module path (RootModules where the module is not at the root) at its head: the build module takes the head of a
+// module path (mesh-sdk's Go module for mesh-sdk) at its head: the build module takes the head of a
 // repository from any package of it. A package of own comes with the tree the build fetched, and is left out, as is a
 // package no version is known of. Nil for none. latest is not changed.
 func Heads(latest delivery.PackageVersions, heads map[string]string, own string) delivery.PackageVersions {
@@ -42,26 +52,12 @@ func Heads(latest delivery.PackageVersions, heads map[string]string, own string)
 		if delivery.SameRepository(repo, own) || len(head) != 40 || held[repo] {
 			continue
 		}
-		root, ok := RootModules[repo]
-		if !ok {
-			name, found := cutOwner(repo)
-			if !found {
-				continue
-			}
-			root = delivery.ForgeModulePrefix + name
+		if root, ok := rootOf(repo); ok {
+			versions[root] = head
 		}
-		versions[root] = head
 	}
 	if len(versions) == 0 {
 		return nil
 	}
 	return versions
 }
-
-func cutOwner(repo string) (string, bool) {
-	const owner = "novox/"
-	if len(repo) > len(owner) && repo[:len(owner)] == owner {
-		return repo[len(owner):], true
-	}
-	return "", false
-}

sdk/grouppackages/grouppackages_test.go

@@ -1,51 +0,0 @@
-package grouppackages
-
-import (
-	"strings"
-	"testing"
-
-	"git.novox.be/novox/mesh-sdk/go/delivery"
-)
-
-func TestHeads(t *testing.T) {
-	h := func(c string) string { return strings.Repeat(c, 40) }
-	const (
-		contracts = "git.novox.be/novox/mesh-contracts/go/clients/assignerclient"
-		sdk       = "git.novox.be/novox/mesh-sdk/go"
-		tools     = "git.novox.be/novox/mesh-tools/go/x"
-	)
-	for _, tc := range []struct {
-		name   string
-		latest delivery.PackageVersions
-		heads  map[string]string
-		own    string
-		want   delivery.PackageVersions
-	}{
-		{"none", nil, nil, "novox/mesh-host", nil},
-		{"trunk latest alone", delivery.PackageVersions{tools: h("a")}, nil, "novox/mesh-host", delivery.PackageVersions{tools: h("a")}},
-		{"a member at its head replaces the latest", delivery.PackageVersions{sdk: h("a")}, map[string]string{"novox/mesh-sdk": h("b")}, "novox/mesh-host",
-			delivery.PackageVersions{sdk: h("b")}},
-		{"a member that publishes no package yet is named by its root module", delivery.PackageVersions{sdk: h("a")},
-			map[string]string{"novox/mesh-contracts": h("c")}, "novox/mesh-host",
-			delivery.PackageVersions{sdk: h("a"), "git.novox.be/novox/mesh-contracts": h("c")}},
-		{"mesh-sdk named by its go module before any package of it is known", nil, map[string]string{"novox/mesh-sdk": h("b")}, "novox/mesh-host",
-			delivery.PackageVersions{sdk: h("b")}},
-		{"a member whose packages are known gets no root", delivery.PackageVersions{contracts: h("a")},
-			map[string]string{"novox/mesh-contracts": h("c")}, "novox/mesh-host", delivery.PackageVersions{contracts: h("c")}},
-		{"the build's own repository is left out", delivery.PackageVersions{contracts: h("a")},
-			map[string]string{"novox/mesh-contracts": h("c")}, "novox/mesh-contracts", nil},
-		{"a head that is no full commit names nothing", nil, map[string]string{"novox/mesh-contracts": "abc"}, "novox/mesh-host", nil},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			got := Heads(tc.latest, tc.heads, tc.own)
-			if len(got) != len(tc.want) {
-				t.Fatalf("got %v, want %v", got, tc.want)
-			}
-			for k, v := range tc.want {
-				if got[k] != v {
-					t.Errorf("%s = %q, want %q (all: %v)", k, got[k], v, got)
-				}
-			}
-		})
-	}
-}