Merge pull request 'Judge a delivery group's composed check by the group's own controller, release branch included (issue 867, ADR 0239 §14)' (#352) from fix/867-group-check-judged-by-its-controller into main

cmd/mesh-controller/replay867_test.go

@@ -67,6 +67,7 @@ type theIssue867 struct {
 	js                       interface{ Conn() *nats.Conn }
 	judgeCache               string
 	registry                 string
+	hostRepo, controllerRepo string
 	groups                   string // what mesh-delivery's groups answers
 }
 
@@ -125,12 +126,17 @@ func main() {
 	}
 }
 
-func theGroupOfIssue867(t *testing.T) *theIssue867 {
+func theGroupOfIssue867(t *testing.T) *theIssue867 { return theGroupOfIssue867Into(t, "main") }
+
+// theGroupOfIssue867Into is the group with both pull requests into base: main, or a release branch both repositories
+// hold, cut from their trunks.
+func theGroupOfIssue867Into(t *testing.T, base string) *theIssue867 {
 	t.Helper()
 	open := aMesh(t)
 	r := &theIssue867{open: open, judgeCache: t.TempDir()}
 	root := filepath.Join(t.TempDir(), "novox")
 	controller, host := filepath.Join(root, "mesh-controller"), filepath.Join(root, "mesh-host")
+	r.hostRepo, r.controllerRepo = host, controller
 	for _, d := range []string{controller, host} {
 		if err := os.MkdirAll(d, 0o755); err != nil {
 			t.Fatal(err)
@@ -146,21 +152,36 @@ func theGroupOfIssue867(t *testing.T) *theIssue867 {
 	r867Commit(t, host, map[string]string{"validate/v.go": "package validate\n\nconst Word = \"the trunk's validator\"\n",
 		"internal/declaration/d.go": "package declaration\n", "module.json": `{"module":"mesh-host"}`,
 		builder.CheckScript: "set -e\ngrep -q 'foundation: new' \"$MESH_CHECK_BESIDE/mesh-controller/lock.txt\"\n"})
+	if base != "main" {
+		for _, d := range []string{controller, host} {
+			r867Git(t, d, "branch", base, "main")
+		}
+	}
 	// The two pull requests of one group, feat/867.
-	r867Git(t, controller, "checkout", "--quiet", "-b", "feat/867", "main")
+	r867Git(t, controller, "checkout", "--quiet", "-b", "feat/867", base)
 	change := r867Controller("the controller", "+validate.More")
 	change["lock.txt"] = "foundation: new\n"
 	r.controllerHead = r867Commit(t, controller, change)
 	r867Git(t, controller, "checkout", "--quiet", "main")
-	r867Git(t, host, "checkout", "--quiet", "-b", "feat/867", "main")
+	r867Git(t, host, "checkout", "--quiet", "-b", "feat/867", base)
 	r.hostHead = r867Commit(t, host, map[string]string{"validate/more.go": "package validate\n\nconst More = \" and more\"\n"})
 	r867Git(t, host, "checkout", "--quiet", "main")
 
+	followed := func(string) string { return "main" }
 	for _, m := range []struct{ module, dir string }{{"mesh-controller", controller}, {"mesh-host", host}} {
 		// The mesh runs each at its trunk's head, the commit a check reads it at beside another repository's change.
 		running := r867Git(t, m.dir, "rev-parse", "main")
 		if err := open.inventory.RegisterModule(t.Context(), catalogue.Manifest{Module: m.module, Version: "1"},
-			inventory.Source{Repository: m.dir, Ref: "main", BuiltFrom: running, Head: running}); err != nil {
+			inventory.Source{Repository: m.dir, Ref: followed(m.module), BuiltFrom: running, Head: running}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if base != "main" {
+		// A module built from the controller's repository follows the release branch, as the mesh runs it there: a pull
+		// request into the release branch moves it, so the composed check's gate runs.
+		running := r867Git(t, controller, "rev-parse", "main")
+		if err := open.inventory.RegisterModule(t.Context(), catalogue.Manifest{Module: "zz-agent", Version: "1"},
+			inventory.Source{Repository: controller, Ref: base, BuiltFrom: running, Head: running}); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -178,9 +199,9 @@ func theGroupOfIssue867(t *testing.T) *theIssue867 {
 	r.groups = (`[{"id":"feat/867","members":["` + root + `/mesh-host@` + r.hostHead[:12] + ` (checked)","` +
 		root + `/mesh-controller@` + r.controllerHead[:12] + ` (checked)"],"member_details":[` +
 		`{"id":"novox/mesh-host@` + r.hostHead[:12] + `","repository":"novox/mesh-host","state":"checked","number":114,"head":"` +
-		r.hostHead + `","base":"main","open":true},` +
+		r.hostHead + `","base":"` + base + `","open":true},` +
 		`{"id":"novox/mesh-controller@` + r.controllerHead[:12] + `","repository":"novox/mesh-controller","state":"checked","number":346,"head":"` +
-		r.controllerHead + `","base":"main","open":true}]}]`)
+		r.controllerHead + `","base":"` + base + `","open":true}]}]`)
 	r.registry = r867Registry(t)
 	r867Runtime(t)
 	return r
@@ -344,3 +365,105 @@ func TestTheRecheckVerbAsksMeshDeliveryOnABusOfItsOwn(t *testing.T) {
 		t.Fatalf("mesh-delivery was asked %d times", asked)
 	}
 }
+
+// **A delivery group's composed check is judged by the group's own controller** (novox/hq ADR 0239 §14), into a release
+// branch too: the controller member and the mesh-host member of the pair are consistent only together (the controller
+// calls what only the node-engine change brings), and both merge into a release branch. The release judge (the
+// controller of the release branch with the validator of its own) cannot build it; the group's own controller, as
+// merged, with the node-engine member's validator, can. It drives askGroupCheck and the build seat's Check for real.
+func TestAGroupsCompositionIntoAReleaseBranchIsJudgedByItsOwnController(t *testing.T) {
+	groupIntoARelease(t, false)
+}
+
+// The shape the live check had: the node-engine member is the group's primary, so the composed request is asked with
+// Judge "validator" (a union of modules that includes mesh-host's) and the release branch set; the release judge then
+// put this change's validator into the old controller and "judge-with-this-validator" did not build.
+func TestAGroupsCompositionJudgedWithTheValidatorIntoAReleaseBranchIsJudgedByItsOwnController(t *testing.T) {
+	groupIntoARelease(t, true)
+}
+
+func groupIntoARelease(t *testing.T, hostPrimary bool) {
+	if _, err := exec.LookPath("go"); err != nil {
+		t.Skip("no Go toolchain on this machine to build a judge with")
+	}
+	const release = "feat/r"
+	r := theGroupOfIssue867Into(t, release)
+	aForgeWithBranches(t, map[string][]string{"novox/mesh-controller": {release}}, nil)
+	// The pair renames what the validator says: the node-engine change drops Word for Word2, the controller change
+	// calls Word2. The controller of the release branch calls Word, so it does not build with the new validator; the
+	// controller change does not build with the validator it vendors. Only together are they consistent.
+	r867Git(t, r.hostRepo, "checkout", "--quiet", "-b", "swap", release)
+	r.hostHead = r867Commit(t, r.hostRepo, map[string]string{"validate/v.go": "package validate\n\nconst Word2 = \"the new validator\"\n"})
+	r867Git(t, r.hostRepo, "checkout", "--quiet", "main")
+	r867Git(t, r.controllerRepo, "checkout", "--quiet", "-b", "swap", release)
+	swapped := strings.Replace(r867Controller("the controller", "")["cmd/mesh-controller/main.go"], "validate.Word)", "validate.Word2)", 1)
+	r.controllerHead = r867Commit(t, r.controllerRepo, map[string]string{"cmd/mesh-controller/main.go": swapped})
+	r867Git(t, r.controllerRepo, "checkout", "--quiet", "main")
+	js, _ := aBuildQueue(t)
+	servingBus.Store(js)
+	t.Cleanup(func() { servingBus.Store(nil) })
+	members := []orderMember{
+		{ID: "novox/mesh-controller@" + r.controllerHead[:12], Repository: "novox/mesh-controller", Base: release,
+			Head: r.controllerHead, Number: 346, Paths: []string{"lock.txt", "cmd/mesh-controller/main.go"}, ModuleDirsSaid: true},
+		{ID: "novox/mesh-host@" + r.hostHead[:12], Repository: "novox/mesh-host", Base: release, Head: r.hostHead, Number: 114,
+			Paths: []string{"validate/v.go"}, ModuleDirsSaid: true}}
+	if hostPrimary {
+		members[0], members[1] = members[1], members[0]
+	}
+	if _, err := askGroupCheck(t.Context(), r.open, "feat/867", members); err != nil {
+		t.Fatal(err)
+	}
+	q, err := link.ReadQueue(t.Context(), js, link.TheBuildMachine)
+	if err != nil || len(q.Asks) == 0 {
+		t.Fatalf("nothing was asked of the build seat: %v", err)
+	}
+	request := q.Asks[len(q.Asks)-1].Request
+	if request.Check.Release != release || request.Check.Group != "feat/867" || len(request.Check.Members) != 1 || len(request.Check.Modules) == 0 {
+		t.Fatalf("the composed check is asked as release %q, group %q, %d other head(s)", request.Check.Release,
+			request.Check.Group, len(request.Check.Members))
+	}
+	if hostPrimary {
+		if request.Check.Repo != "mesh-host" {
+			t.Fatalf("the primary is %s, not the node-engine", request.Check.Repo)
+		}
+		// As the live ask had it (the mesh-host module among the union's): the node-engine's validator judges.
+		request.Check.Judge = link.JudgeValidator
+	}
+	// What the build seat makes of the ask.
+	var wire map[string]json.RawMessage
+	body, _ := json.Marshal(request.Check)
+	_ = json.Unmarshal(body, &wire)
+	delete(wire, "group")
+	delete(wire, "members")
+	body, _ = json.Marshal(wire)
+	var spec builder.CheckSpec
+	if err := json.Unmarshal(body, &spec); err != nil {
+		t.Fatal(err)
+	}
+	for _, m := range request.Check.Members {
+		spec.Group = append(spec.Group, builder.GroupHead{Owner: m.Owner, Repo: m.Repo, Repository: m.Repository, Ref: m.Ref,
+			Paths: m.Paths, Base: m.Base})
+	}
+	spec.ID, spec.Repository, spec.Ref = request.ID, request.Repository, request.Ref
+	spec.Toolchain, spec.JudgeCache = "golang", r.judgeCache
+	v, err := builder.Check(t.Context(), builder.Command, spec, t.TempDir(), r.registry, builder.GitCredential{}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if v.Gate == nil || v.Gate.Verdict != "pass" {
+		t.Fatalf("the group's composition into %s was not judged by its own controller: %+v\n%s", release, v.Gate, v.Report)
+	}
+	if mark := "[release " + release + ", judged by the group's controller " + v.JudgedBy + "] "; !strings.HasPrefix(v.Gate.Summary, mark) ||
+		!strings.Contains(v.Report, "by the group's controller at "+v.JudgedBy) {
+		t.Fatalf("the statuses and the report do not say the group's controller judged:\n%s\n%s", v.Gate.Summary, v.Report)
+	}
+	if !strings.Contains(v.Gate.Summary, "the new validator") {
+		t.Fatalf("the gate was judged by %q, not by the group's controller with the node-engine member's validator", v.Gate.Summary)
+	}
+	if !strings.HasPrefix(v.JudgeFrom, "the group's own controller") || !strings.Contains(v.Validator, "the group's mesh-host merge") {
+		t.Fatalf("the verdict does not say where the judge and the validator came from: %q / %q", v.JudgeFrom, v.Validator)
+	}
+	if v.Release != release || v.JudgedBy == "" {
+		t.Fatalf("the verdict names release %q judged by %q", v.Release, v.JudgedBy)
+	}
+}

internal/builder/check.go

@@ -771,11 +771,14 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 		if spec.Judge == "self" {
 			judgedBy = tree
 		}
+		if siblingJudge.Dir != "" {
+			judgedBy = siblingJudge.Dir
+		}
 		judged.JudgedBy = headAt(ctx, run, judgedBy)
 		if siblingJudge.Validator == "" {
 			judged.Validator = releaseValidator(spec, judged.BesideAt, judged.Beside)
 		}
-		said := releaseWords(spec.Release, judged.JudgedBy, judged.Validator, judged.BesideAt, judged.Beside)
+		said := releaseWords(spec.Release, whoJudges(siblingJudge), judged.JudgedBy, judged.Validator, judged.BesideAt, judged.Beside)
 		fmt.Fprintf(&out, "--- %s\n", said)
 		say("check", "%s", said)
 	}
@@ -846,7 +849,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 
 	if spec.Release != "" {
 		// The statuses name the release branch and the judge first, so the forge's short description keeps them.
-		mark := releaseMark(spec.Release, judged.JudgedBy)
+		mark := releaseMark(spec.Release, whoJudges(siblingJudge), judged.JudgedBy)
 		v.Gate.Summary, v.Repo.Summary = mark+v.Gate.Summary, mark+v.Repo.Summary
 	}
 	// What the check printed travels in the verdict to the forge and the controller: redacted as the log is.
@@ -1586,21 +1589,30 @@ func releaseValidator(spec CheckSpec, at, commits map[string]string) string {
 }
 
 // releaseWords is how a release-branch check says what judged it and what it read (novox/hq ADR 0358 §4).
-func releaseWords(release, judgedBy, validator string, at, commits map[string]string) string {
+func releaseWords(release, controller, judgedBy, validator string, at, commits map[string]string) string {
 	var read []string
 	for _, dir := range sortedKeys(at) {
 		read = append(read, fmt.Sprintf("%s %s at %s", dir, refWords(at[dir]), commits[dir]))
 	}
-	said := fmt.Sprintf("judged for the release branch %s by its controller at %s, with %s", release, judgedBy, validator)
+	said := fmt.Sprintf("judged for the release branch %s by %s at %s, with %s", release, controller, judgedBy, validator)
 	if len(read) > 0 {
 		said += "; read beside it: " + strings.Join(read, ", ")
 	}
 	return said
 }
 
+// whoJudges says which controller judged: the release branch's own ("its controller"), or — when a delivery group's
+// member in mesh-controller judged (novox/hq ADR 0239 §14) — the group's.
+func whoJudges(j siblingJudgement) string {
+	if j.Dir != "" {
+		return "the group's controller"
+	}
+	return "its controller"
+}
+
 // releaseMark opens each layer's summary of a release-branch check, so a status says it first.
-func releaseMark(release, judgedBy string) string {
+func releaseMark(release, controller, judgedBy string) string {
-	return fmt.Sprintf("[release %s, judged by its controller %s] ", release, judgedBy)
+	return fmt.Sprintf("[release %s, judged by %s %s] ", release, controller, judgedBy)
 }
 
 // refWords is a ref as a line says it: a commit by its first eight characters, a branch whole.
@@ -1949,6 +1961,22 @@ func lastLine(s string) string {
 // the controller it was built from and Validator the node-engine validator in it. Both empty for any other judge.
 type siblingJudgement struct {
 	From, Validator string
+	// Dir is the checkout of the controller that judges.
+	Dir string
+}
+
+// groupDir is the checkout of the group's member in a repository of that name — the checked one (tree) or one cloned
+// beside it for the group — empty when no member is in it.
+func groupDir(spec CheckSpec, root, tree, repo string) string {
+	if spec.Repo == repo {
+		return tree
+	}
+	for _, g := range spec.Group {
+		if g.Repo == repo && safeName.MatchString(g.Repo) {
+			return filepath.Join(root, "group-"+g.Repo)
+		}
+	}
+	return ""
 }
 
 // judgeFromTheGroup builds the judge of a member that has siblings (novox/hq ADR 0382 §3); done says it did (or failed
@@ -1974,6 +2002,21 @@ func judgeFromTheGroup(ctx context.Context, labelled, run Runner, spec CheckSpec
 		return fmt.Sprintf("sibling %s#%d merge %s", s.Repository, s.Number, judged.Beside[dir])
 	}
 	switch {
+	case len(spec.Group) > 0 && groupDir(spec, root, tree, "mesh-controller") != "":
+		// **A delivery group's composed check is judged by the group's own controller** (novox/hq ADR 0239 §14), whatever
+		// base it merges into — a release branch included (ADR 0358 §2 changes the judge of one pull request, not of a
+		// group): the member that moves mesh-controller, as merged, with the validator of the group's mesh-host member's
+		// merge when it has one, else the one that controller vendors.
+		source, validatorDir = groupDir(spec, root, tree, "mesh-controller"), groupDir(spec, root, tree, "mesh-host")
+		from = "the group's own controller, merge " + headAt(ctx, run, source)
+		validator = "the validator the group's controller vendors"
+		if validatorDir != "" {
+			validator = "the group's mesh-host merge " + headAt(ctx, run, validatorDir) + "'s validator"
+		}
+		inputs = append(inputs, "group member "+spec.Repo+" "+judged.Head+" onto "+judged.Trunk)
+		for _, m := range judged.Members {
+			inputs = append(inputs, "group member "+m.Repo+" "+m.Head+" onto "+m.Trunk)
+		}
 	case spec.Judge == "self" && host != nil:
 		source, validatorDir = tree, filepath.Join(root, "mesh-host")
 		from = "this change's own merge " + judged.Head
@@ -2004,7 +2047,7 @@ func judgeFromTheGroup(ctx context.Context, labelled, run Runner, spec CheckSpec
 	sum := sha256.Sum256([]byte(strings.Join(inputs, "\n")))
 	key := "siblings-" + hex.EncodeToString(sum[:16])
 	bin := filepath.Join(root, "bin", "judge-of-the-group")
-	said.From, said.Validator = from, validator
+	said.From, said.Validator, said.Dir = from, validator, source
 	if judgeKept(keptJudges(spec.JudgeCache), key, bin) {
 		say("check", "judged by %s, with %s, built for an earlier check of the same inputs", from, validator)
 		return bin, "", true, nil

testdata/store-tests

@@ -82,3 +82,5 @@
 ./cmd/mesh-controller TestReplay867
 ./cmd/mesh-controller TestReplay867Recheck
 ./cmd/mesh-controller TestTheRecheckVerbAsksMeshDeliveryOnABusOfItsOwn
+./cmd/mesh-controller TestAGroupsCompositionIntoAReleaseBranchIsJudgedByItsOwnController
+./cmd/mesh-controller TestAGroupsCompositionJudgedWithTheValidatorIntoAReleaseBranchIsJudgedByItsOwnController