Merge pull request 'A merge check judges the head merged into its trunk, and names the trunk commit (hq ADR 0323, issue 341)' (#271) from feat/341-a-merge-check-judges-the-merge into main

cmd/mesh-builder/main.go

@@ -256,7 +256,13 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri
 		v, err = builder.Check(building, builder.Command, checkSpecOf(request), workspace, registry, forgeFrom(), say)
 		if err == nil {
 			result.Check = &link.CheckOutcome{Verdict: v.Verdict, Summary: v.Summary, Report: v.Report,
-				Took: v.Took.Round(time.Second).String(), Gate: layerOf(v.Gate), RepoCheck: layerOf(v.Repo)}
+				Took: v.Took.Round(time.Second).String(), Gate: layerOf(v.Gate), RepoCheck: layerOf(v.Repo),
+				Head: v.Head, Trunk: v.Trunk, Beside: v.Beside}
+			// What each of a group's heads was judged as (novox/hq ADR 0323 §6).
+			for _, m := range v.Members {
+				result.Check.Members = append(result.Check.Members, link.CheckedMember{Owner: m.Owner, Repo: m.Repo,
+					Commit: m.Head, Trunk: m.Trunk})
+			}
 		}
 	} else if err == nil {
 		// The package-registry credential is a build input, so it is resolved before the clone: a
@@ -354,7 +360,7 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec {
 	}
 	for _, m := range c.Members {
 		spec.Group = append(spec.Group, builder.GroupHead{Owner: m.Owner, Repo: m.Repo, Repository: m.Repository,
-			Ref: m.Ref, Paths: m.Paths})
+			Ref: m.Ref, Paths: m.Paths, Base: m.Base})
 	}
 	return spec
 }

cmd/mesh-controller/checks.go

@@ -161,9 +161,7 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error {
 		return u, err == nil
 	})
 	fmt.Print(planText(plan))
-	direct := link.Checked{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Commit: p.Commit,
-		ID: link.NewBuildID(time.Now()), Verdict: "pass", Summary: noModuleTouched,
-		Gate: &link.CheckLayer{Verdict: "pass", Summary: noModuleTouched}, Plan: &plan}
+	direct := directVerdict(p, plan)
 	switch {
 	case !scope.gated() && !scope.Mesh:
 		fmt.Printf("%s/%s#%d (%.8s): %s, and the repository is not the mesh's: said, nothing run\n",
@@ -201,6 +199,24 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error {
 	return nil
 }
 
+// directVerdict is the verdict of a change no build judges — it touches no module, and either the repository is not
+// the mesh's or the head holds no merge-check.sh. **It names the trunk commit it holds for** (novox/hq ADR 0323 §3):
+// the base branch's head the forge announced with the head, so the merge tool can weigh it against the trunk's head
+// and does not refuse it for ever (a verdict naming no trunk is never the trunk's head's). Nothing was built to
+// judge: the trunk commit is the one the forge said the change was announced against.
+func directVerdict(p link.PullUpdated, plan link.ChangePlan) link.Checked {
+	trunk := p.BaseSHA
+	if len(trunk) > 12 {
+		trunk = trunk[:12]
+	}
+	if trunk != "" {
+		plan.Trunk = trunk
+	}
+	return link.Checked{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Commit: p.Commit,
+		ID: link.NewBuildID(time.Now()), Verdict: "pass", Summary: noModuleTouched,
+		Gate: &link.CheckLayer{Verdict: "pass", Summary: noModuleTouched}, Plan: &plan, Trunk: trunk}
+}
+
 // checkRequestFor is the ask for one pull request's head: the repository as the mesh clones it, the head,
 // and what is read beside it.
 func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scope checkScope,
@@ -363,6 +379,9 @@ func checkedOf(result link.BuildResult) link.Checked {
 	case result.Check != nil:
 		c.Verdict, c.Summary, c.Report = result.Check.Verdict, result.Check.Summary, result.Check.Report
 		c.Gate, c.RepoCheck = result.Check.Gate, result.Check.RepoCheck
+		// What was judged (novox/hq ADR 0323 §3): the head merged into the trunk commit, and the commits of the
+		// repositories read beside it.
+		c.Trunk, c.Beside, c.Took = result.Check.Trunk, result.Check.Beside, result.Check.Took
 		if c.Gate == nil {
 			// A build seat from before the layers: its verdict is the gate's.
 			c.Gate = &link.CheckLayer{Verdict: c.Verdict, Summary: c.Summary}
@@ -388,14 +407,20 @@ func checkedOf(result link.BuildResult) link.Checked {
 	}
 	if result.Checked != nil {
 		c.Plan = result.Checked.Plan
+		if c.Plan != nil && c.Trunk != "" {
+			// The kept delivery plan records the trunk commit with its other inputs (ADR 0323 §3).
+			plan := *c.Plan
+			plan.Trunk = c.Trunk
+			c.Plan = &plan
+		}
 		// A delivery group's composed check (novox/hq ADR 0239): every head it judged, this one first.
 		if g := result.Checked; g.Group != "" {
 			c.Group = g.Group
 			c.Members = append(c.Members, link.CheckedMember{Owner: g.Owner, Repo: g.Repo, Number: g.Number,
-				Commit: result.Ref})
+				Commit: result.Ref, Trunk: c.Trunk})
 			for _, m := range g.Members {
 				c.Members = append(c.Members, link.CheckedMember{Owner: m.Owner, Repo: m.Repo, Number: m.Number,
-					Commit: m.Ref})
+					Commit: m.Ref, Trunk: memberTrunk(result.Check, m)})
 			}
 		}
 	}
@@ -410,6 +435,20 @@ func checkedOf(result link.BuildResult) link.Checked {
 	return c
 }
 
+// memberTrunk is the trunk commit the build seat merged one of a group's heads into (novox/hq ADR 0323 §6), empty
+// when it said none.
+func memberTrunk(outcome *link.CheckOutcome, m link.GroupMember) string {
+	if outcome == nil {
+		return ""
+	}
+	for _, j := range outcome.Members {
+		if strings.EqualFold(j.Owner, m.Owner) && strings.EqualFold(j.Repo, m.Repo) && strings.HasPrefix(m.Ref, j.Commit) {
+			return j.Trunk
+		}
+	}
+	return ""
+}
+
 func prefixedAll(prefix string, items []string) []string {
 	out := make([]string, 0, len(items))
 	for _, i := range items {
@@ -424,8 +463,12 @@ func sayChecked(ctx context.Context, c link.Checked) {
 	if c.RepoCheck != nil {
 		repo = strings.ToUpper(c.RepoCheck.Verdict) + " — " + c.RepoCheck.Summary
 	}
-	fmt.Printf("%s: %s/%s#%d at %.8s checked on %s: gate %s — %s; repository %s\n", c.ID, c.Owner, c.Repo, c.Number,
-		c.Commit, orSomewhere(c.On), strings.ToUpper(c.Verdict), c.Summary, repo)
+	judged := ""
+	if c.Trunk != "" {
+		judged = " merged into trunk " + c.Trunk
+	}
+	fmt.Printf("%s: %s/%s#%d at %.8s%s checked on %s: gate %s — %s; repository %s\n", c.ID, c.Owner, c.Repo, c.Number,
+		c.Commit, judged, orSomewhere(c.On), strings.ToUpper(c.Verdict), c.Summary, repo)
 	if checkEvents == nil {
 		return
 	}

cmd/mesh-controller/checks_test.go

@@ -210,3 +210,81 @@ func TestABuildOffTheTrunkIsNeverPublished(t *testing.T) {
 		t.Errorf("a build seat that said nothing was refused: %v", err)
 	}
 }
+
+// **A verdict names the trunk commit it judged** (novox/hq ADR 0323 §3, issue 341): the controller's `checked`
+// carries the head and the trunk commit the build seat merged it into, the commits of the repositories read beside
+// it, how long it took, and the kept delivery plan records the trunk commit with its other inputs. A group's
+// composed check names, per member, the trunk commit that head was merged into (§6).
+func TestAVerdictNamesTheTrunkCommitItJudged(t *testing.T) {
+	plan := &link.ChangePlan{Repository: "novox/mesh-host", Base: "main", Head: "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+		Summary: "builds mesh-host"}
+	asked := &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Number: 66, Base: "main", Plan: plan}
+	c := checkedOf(link.BuildResult{ID: "b", Ref: "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Checked: asked,
+		Check: &link.CheckOutcome{Verdict: "pass", Took: "4m12s", Head: "dfb1415aaaaa", Trunk: "d28ca95bbbbb",
+			Gate: &link.CheckLayer{Verdict: "pass"}, RepoCheck: &link.CheckLayer{Verdict: "pass"},
+			Beside: map[string]string{"mesh-catalog": "0123456789ab", "mesh-lab": "ba9876543210"}}})
+	if c.Trunk != "d28ca95bbbbb" || c.Commit != "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" {
+		t.Fatalf("the verdict names head %s and trunk %q", c.Commit, c.Trunk)
+	}
+	if c.Beside["mesh-catalog"] != "0123456789ab" || c.Beside["mesh-lab"] != "ba9876543210" {
+		t.Fatalf("the verdict names the repositories read beside it as %v", c.Beside)
+	}
+	if c.Took != "4m12s" {
+		t.Fatalf("the verdict says it took %q", c.Took)
+	}
+	if c.Plan == nil || c.Plan.Trunk != "d28ca95bbbbb" || c.Plan.Head != plan.Head {
+		t.Fatalf("the kept plan records %+v", c.Plan)
+	}
+	if plan.Trunk != "" {
+		t.Fatal("the plan the controller asked with was changed: the verdict's is a copy")
+	}
+
+	group := &link.CheckRequest{Owner: "novox", Repo: "mesh-controller", Number: 7, Base: "main", Group: "feat/x",
+		Members: []link.GroupMember{{Owner: "novox", Repo: "mesh-host", Number: 8, Ref: "cccccccccccc0000", Base: "main"}}}
+	c = checkedOf(link.BuildResult{ID: "g", Ref: "aaaaaaaaaaaa0000", Checked: group, Check: &link.CheckOutcome{
+		Verdict: "pass", Head: "aaaaaaaaaaaa", Trunk: "111111111111", Gate: &link.CheckLayer{Verdict: "pass"},
+		Members: []link.CheckedMember{{Owner: "novox", Repo: "mesh-host", Commit: "cccccccccccc", Trunk: "222222222222"}}}})
+	if len(c.Members) != 2 || c.Members[0].Trunk != "111111111111" || c.Members[1].Trunk != "222222222222" ||
+		c.Members[1].Commit != "cccccccccccc0000" {
+		t.Fatalf("the composed check names its members %+v", c.Members)
+	}
+}
+
+// **The merge made for a check is never registered** (novox/hq ADR 0323 §2): a build of it — a commit on no branch
+// of the forge — is refused, naming the trunk. The rule of ADR 0238 §1 stands unchanged; this holds it for the
+// merge commit a check makes.
+func TestABuildOfACheckMergeIsRefusedNamingTheTrunk(t *testing.T) {
+	merge := link.BuildResult{ID: "b", Commit: "4b5c6d7e8f90aabbccddeeff0011223344556677", Trunk: "main"}
+	err := publishable(merge, "")
+	if !errors.Is(err, errOffTheTrunk) || !strings.Contains(err.Error(), "not on main") {
+		t.Fatalf("a build of a check's merge was answered %v", err)
+	}
+}
+
+// **A verdict no build judges names its trunk commit too** (novox/hq ADR 0323 §3): a change touching no module of a
+// repository with no merge-check.sh (novox/mesh-tools-go) is answered by the controller itself. Naming no trunk, the
+// merge tool refused it, asked again, got the same verdict, and refused for ever.
+func TestADirectVerdictNamesTheTrunkTheForgeAnnounced(t *testing.T) {
+	p := link.PullUpdated{Owner: "novox", Repo: "mesh-tools-go", Number: 3, Base: "main", Commit: "1a2b3c4d5e6f7a8b9c0d",
+		BaseSHA: "7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b"}
+	c := directVerdict(p, link.ChangePlan{Repository: "novox/mesh-tools-go", Base: "main", Head: p.Commit})
+	if c.Trunk != "7a8b9c0d1e2f" || c.Plan == nil || c.Plan.Trunk != "7a8b9c0d1e2f" {
+		t.Fatalf("the direct verdict names trunk %q, its plan %+v", c.Trunk, c.Plan)
+	}
+	if c.Verdict != "pass" || c.Commit != p.Commit {
+		t.Fatalf("the direct verdict is %+v", c)
+	}
+	if c := directVerdict(link.PullUpdated{Owner: "novox", Repo: "x", Commit: "abc"}, link.ChangePlan{}); c.Trunk != "" {
+		t.Fatalf("a forge that said no base head gave the trunk %q", c.Trunk)
+	}
+}
+
+// A head checked again through mesh-delivery (`delivery check --members`, a recheck) reaches the same direct verdict:
+// it names the base head the forge last announced, kept on the member, or the merge tool would refuse it for ever.
+func TestARecheckedHeadsDirectVerdictNamesItsTrunk(t *testing.T) {
+	m := orderMember{ID: "novox/mesh-tools-go@1a2b3c4d5e6f", Repository: "novox/mesh-tools-go", Base: "main",
+		Head: "1a2b3c4d5e6f7a8b9c0d", Number: 3, BaseSHA: "7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b"}
+	if c := directVerdict(m.pull(), link.ChangePlan{}); c.Trunk != "7a8b9c0d1e2f" {
+		t.Fatalf("a rechecked head's direct verdict names trunk %q", c.Trunk)
+	}
+}

cmd/mesh-controller/delivery.go

@@ -580,6 +580,9 @@ type orderMember struct {
 	CloneURL       string   `json:"clone_url,omitempty"`
 	// After are the repositories its pull request says it goes after (`after: <repository>`).
 	After []string `json:"after,omitempty"`
+	// BaseSHA is the base branch's head the forge last announced the head with (novox/hq ADR 0323): the trunk commit
+	// a verdict no build judges names when the head is checked again through mesh-delivery.
+	BaseSHA string `json:"base_sha,omitempty"`
 }
 
 // pull is the member as the forge announced it.
@@ -589,7 +592,7 @@ func (m orderMember) pull() link.PullUpdated {
 	if base == "" {
 		base = "main"
 	}
-	return link.PullUpdated{Owner: owner, Repo: repo, Number: m.Number, Base: base, Commit: m.Head,
+	return link.PullUpdated{Owner: owner, Repo: repo, Number: m.Number, Base: base, Commit: m.Head, BaseSHA: m.BaseSHA,
 		CloneURL: m.CloneURL, Paths: m.Paths, PathsTruncated: m.PathsTruncated, Removed: m.Removed,
 		ModuleDirs: m.ModuleDirs, ModuleDirsSaid: m.ModuleDirsSaid}
 }
@@ -1185,7 +1188,7 @@ func askGroupCheck(ctx context.Context, open *stores, group string, members []or
 			return "", fmt.Errorf("%s cannot be cloned for the group's check: %w", m.ID, err)
 		}
 		request.Check.Members = append(request.Check.Members, link.GroupMember{Owner: mp.Owner, Repo: mp.Repo,
-			Number: mp.Number, Repository: url, Ref: mp.Commit, Paths: mp.Paths})
+			Number: mp.Number, Repository: url, Ref: mp.Commit, Paths: mp.Paths, Base: mp.Base})
 	}
 	seat := buildSeatHeld(ctx)
 	ask, err := askOverOn(seat)