Ask build-agent no composed check of a delivery group, and answer that the checker judges it (issue 1299, C2, ADR 0503 §3)

cmd/mesh-controller/checks_stop_integration_test.go

@@ -6,6 +6,9 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"io"
+	"os"
+	"strings"
 	"testing"
 
 	"git.novox.be/novox/mesh-sdk/go/catalogue"
@@ -92,3 +95,47 @@ func TestThePullRequestChecksAreAskedOnlyForARepositoryTheCheckerDoesNotJudge(t
 		t.Fatalf("0503 §5: the verdict said names a gate layer, or no repo-check layer: %+v", c)
 	}
 }
+
+// C2 (ADR 0503 §3; issue 1299): a delivery group's composed check asks build-agent nothing and answers that the checker
+// judges the group.
+func TestTheGroupCheckAsksNothingAndNamesTheChecker(t *testing.T) {
+	aMesh(t)
+	ctx := t.Context()
+	js, _ := aBuildQueue(t)
+	servingBus.Store(js)
+	t.Cleanup(func() { servingBus.Store(nil) })
+	members := `[{"id":"novox/a@111111111111","repository":"novox/a","base":"main","head":"1111111111111111111111111111111111111111","number":1},` +
+		`{"id":"novox/b@222222222222","repository":"novox/b","base":"main","head":"2222222222222222222222222222222222222222","number":2}]`
+	out := capturedStdout(t, func() error {
+		return deliveryCommand(ctx, []string{"check", "--group", "feat/x", "--members", members})
+	})
+	if !strings.Contains(out, `"judged-by": "mesh-delivery-checker"`) || !strings.Contains(out, `"asked": ""`) {
+		t.Fatalf("0503 §3: the group check answered %s", out)
+	}
+	q, err := link.ReadQueue(ctx, js, link.TheBuildMachine)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(q.Asks) != 0 {
+		t.Fatalf("0503 §3: the group check asked %+v", q.Asks)
+	}
+}
+
+// capturedStdout is what run writes to standard output.
+func capturedStdout(t *testing.T, run func() error) string {
+	t.Helper()
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	was := os.Stdout
+	os.Stdout = w
+	runErr := run()
+	os.Stdout = was
+	w.Close()
+	out, _ := io.ReadAll(r)
+	if runErr != nil {
+		t.Fatal(runErr)
+	}
+	return string(out)
+}

cmd/mesh-controller/composed_verdict_test.go

@@ -1,13 +1,10 @@
 package main
 
 import (
-	"context"
 	"encoding/json"
 	"strings"
 	"testing"
 
-	"git.novox.be/novox/mesh-sdk/go/catalogue"
-	"github.com/novox/mesh-controller/internal/inventory"
 	"github.com/novox/mesh-controller/internal/link"
 )
 
@@ -62,49 +59,3 @@ func TestTheComposedVerdictCarriesEveryMember(t *testing.T) {
 		}
 	}
 }
-
-// Members whose manifests order their modules in a cycle fail the composed check, naming every module and edge, and no
-// ask is made of the build seat.
-func TestAGroupInACycleFailsItsComposedCheckWithoutABuild(t *testing.T) {
-	members := []orderMember{
-		{ID: "novox/x@" + headHost[:12], Repository: "novox/x", Base: "main", Head: headHost, Number: 7, Paths: []string{"module.json"}},
-		{ID: "novox/y@" + headController[:12], Repository: "novox/y", Base: "main", Head: headController, Number: 9, Paths: []string{"module.json"}},
-	}
-	reach := map[string]orderReach{members[0].ID: {Moved: []string{"mx"}}, members[1].ID: {Moved: []string{"my"}}}
-	was, wasParent := manifestAt, parentOf
-	t.Cleanup(func() { manifestAt, parentOf = was, wasParent })
-	parentOf = func(context.Context, string, string, string) (string, error) { return "", nil }
-	manifestAt = func(_ context.Context, _, repo, _, ref string) (catalogue.Manifest, bool, error) {
-		if ref == "main" {
-			return catalogue.Manifest{}, false, nil // what the pull requests merge into holds no such manifest
-		}
-		if repo == "x" {
-			return catalogue.Manifest{Module: "mx", DefinesSeats: []catalogue.SeatDeclaration{{Name: "s", Serves: []catalogue.Verb{{Name: "a"}}}},
-				Invokes: []string{"seat:s.b"}}, true, nil
-		}
-		return catalogue.Manifest{Module: "my", DefinesSeats: []catalogue.SeatDeclaration{{Name: "s", Serves: []catalogue.Verb{{Name: "b"}}}},
-			Invokes: []string{"seat:s.a"}}, true, nil
-	}
-	c, cyclic, err := cycleVerdict(context.Background(), "feat/cycle", members, reach, []inventory.Edge(nil), nil)
-	if err != nil || !cyclic {
-		t.Fatalf("no cycle found: %v", err)
-	}
-	if c.Verdict != "fail" || c.Group != "feat/cycle" || len(c.Members) != 2 || c.Gate == nil || c.Gate.Verdict != "fail" {
-		t.Fatalf("the verdict is %+v", c)
-	}
-	for _, w := range []string{"mx", "my", "s.a", "s.b"} {
-		if !strings.Contains(c.Summary, w) {
-			t.Errorf("the cycle does not name %s: %s", w, c.Summary)
-		}
-	}
-	// Members with no edge between them are no cycle.
-	manifestsReadMu.Lock()
-	clear(manifestsRead)
-	manifestsReadMu.Unlock()
-	manifestAt = func(context.Context, string, string, string, string) (catalogue.Manifest, bool, error) {
-		return catalogue.Manifest{}, false, nil
-	}
-	if _, cyclic, err := cycleVerdict(context.Background(), "feat/cycle", members, reach, nil, nil); err != nil || cyclic {
-		t.Fatalf("a cycle where there is none: %v %v", cyclic, err)
-	}
-}

cmd/mesh-controller/delivery.go

@@ -914,11 +914,10 @@ func deliveryCommand(ctx context.Context, args []string) error {
 		if *group == "" || len(members) < 2 {
 			return errors.New("delivery check --group <id> --members <json> (two heads or more), or --members <one head>")
 		}
-		id, err := askGroupCheck(ctx, open, *group, members)
-		if err != nil {
-			return err
-		}
-		return answer(map[string]any{"asked": id, "group": *group})
+		// **The group's composed check is the checker's** (novox/hq ADR 0503 §3; issue 1299, C2): nothing is asked of
+		// build-agent, and the answer says who judges it.
+		return answer(map[string]any{"asked": "", "group": *group, "judged-by": checkerModule,
+			"said": "the checker judges a delivery group's heads merged together (ADR 0503 §3): nothing was asked"})
 	case "go":
 		if len(positionals) != 1 {
 			return errors.New("delivery go <plan> [--by <who>] [--why <text>]")
@@ -1093,38 +1092,6 @@ func theGraph(ctx context.Context, inv *inventory.Inventory) ([]inventory.Entry,
 	return entries, read, edges, nil
 }
 
-// cycleVerdict is the failing verdict of a group whose modules are ordered in a cycle, and whether there is one: the
-// group's composed check, said without a build. The manifests' change is read from the forge; an answer it cannot give
-// is an error, never read as no cycle.
-func cycleVerdict(ctx context.Context, group string, members []orderMember, reach map[string]orderReach,
-	edges []inventory.Edge, entries []inventory.Entry) (link.Checked, bool, error) {
-	changes, err := changesOf(ctx, changeSourcesOf(members, entries, reach))
-	if err != nil {
-		return link.Checked{}, false, fmt.Errorf("what the group's members say of their manifests cannot be read: %w", err)
-	}
-	var moved []string
-	for _, m := range members {
-		moved = appendNew(moved, reach[m.ID].Moved...)
-	}
-	said := orderEdges(moved, changes, edges).CycleSaid()
-	if said == "" {
-		return link.Checked{}, false, nil
-	}
-	first := members[0].pull()
-	c := link.Checked{ID: link.NewBuildID(time.Now()), Owner: first.Owner, Repo: first.Repo, Number: first.Number,
-		Commit: first.Commit, Verdict: "fail", Summary: said, Report: said, Group: group,
-		Gate: &link.CheckLayer{Verdict: "fail", Summary: said}}
-	for _, m := range members {
-		if group == "" {
-			break // a lone pull request's verdict names no group and no member
-		}
-		p := m.pull()
-		c.Members = append(c.Members, link.CheckedMember{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Commit: p.Commit,
-			Trunk: twelve(p.BaseSHA)})
-	}
-	return c, true, nil
-}
-
 // twelve is a commit as twelve characters.
 func twelve(c string) string {
 	if len(c) > 12 {
@@ -1133,99 +1100,6 @@ func twelve(c string) string {
 	return c
 }
 
-// groupPrimary is the head a group's composed check is run from: the one moving the controller, which then
-// judges the group by itself; else the one moving the node-engine, whose validator judges; else the first.
-func groupPrimary(members []orderMember, reach map[string]orderReach) int {
-	for _, want := range []string{"mesh-controller", "mesh-host"} {
-		for i, m := range members {
-			if slices.Contains(reach[m.ID].Moved, want) {
-				return i
-			}
-		}
-	}
-	return 0
-}
-
-// askGroupCheck asks the build seat for a group's composed check: every head laid over the mesh in turn,
-// judged as one future state (novox/hq ADR 0239). The verdict comes back as `checked` with the group's id,
-// for mesh-delivery; the forge's holder sets no status from it.
-func askGroupCheck(ctx context.Context, open *stores, group string, members []orderMember) (string, error) {
-	entries, read, edges, err := theGraph(ctx, open.inventory)
-	if err != nil {
-		return "", err
-	}
-	reach := reachOfMembers(members, entries, read, edges)
-	// **A cycle of edges fails the group's composed check, naming every module and edge** (novox/hq ADR 0419 clause 6),
-	// before the build seat is asked: no composition of the heads has an order to be sent in.
-	if failing, cyclic, err := cycleVerdict(ctx, group, members, reach, edges, entries); err != nil {
-		return "", err
-	} else if cyclic {
-		sayChecked(ctx, os.Stderr, failing)
-		return failing.ID, nil
-	}
-	primary := groupPrimary(members, reach)
-	p := members[primary].pull()
-	scope := pullScope(p, entries, read, edges)
-	// The gate runs over what any member moves; a judge from the primary alone.
-	for _, m := range members {
-		r := reach[m.ID]
-		scope.Modules = appendNew(scope.Modules, r.Moved...)
-		scope.Dependents = appendNew(scope.Dependents, r.Dependents...)
-	}
-	request, err := checkRequestFor(ctx, open, p, scope, entries)
-	if err != nil {
-		return "", err
-	}
-	request.Check.Group = group
-	// **The values held for the keys the members introduce are told and carried** (novox/hq ADR 0502, issue 1826).
-	var pulls []pullAt
-	for i, s := range changeSourcesOf(members, entries, reach) {
-		pulls = append(pulls, pullAt{changeSource: s, Number: members[i].pull().Number})
-	}
-	withHeldConfig(ctx, os.Stderr, &request, pulls, true)
-	// A group's check is queued as ready when any member's pull request is marked ready.
-	request.Check.Ready = slices.ContainsFunc(members, func(m orderMember) bool { return m.Ready })
-	world, err := restOfTheMesh(ctx, open, shelfOf(entries), "")
-	if err != nil {
-		return "", err
-	}
-	for i, m := range members {
-		if i == primary {
-			continue
-		}
-		mp := m.pull()
-		// As the mesh clones a module built from it; a repository no module is built from, from the forge.
-		source := inventory.Source{Seat: gitSeat, Repository: mp.Owner + "/" + mp.Repo}
-		for _, e := range entries {
-			if !e.Provided && sameRepository(e.Source.Repository, link.SourceMoved{Owner: mp.Owner, Repo: mp.Repo}) {
-				source = e.Source
-				break
-			}
-		}
-		url := source.Repository
-		if source.Seat != "" {
-			url, err = clonedFromSeat(world, source.Seat, source.Repository)
-		}
-		if err != nil {
-			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, Base: mp.Base, Trunk: mp.BaseSHA})
-	}
-	seat := buildSeatHeld(ctx)
-	ask, err := askOverOn(seat)
-	if err != nil {
-		return "", err
-	}
-	defer ask.Close()
-	if err := ask.Ask(ctx, request); err != nil {
-		return "", err
-	}
-	fmt.Fprintf(os.Stderr, "group %s: asked %s to check %d head(s) composed together, as %s\n", group, seat,
-		len(members), request.ID)
-	return request.ID, nil
-}
-
 // appendNew appends what is not there yet.
 func appendNew(to []string, items ...string) []string {
 	for _, i := range items {
@@ -1236,15 +1110,6 @@ func appendNew(to []string, items ...string) []string {
 	return to
 }
 
-// shelfOf is the catalogue's manifests by name.
-func shelfOf(entries []inventory.Entry) map[string]catalogue.Manifest {
-	shelf := map[string]catalogue.Manifest{}
-	for _, e := range entries {
-		shelf[e.Manifest.Module] = e.Manifest
-	}
-	return shelf
-}
-
 // sayPlanMoved says a walk kept in a new state as the controller's `plan-moved`, the plan whole: what the
 // delivery it walks reads its steps from. Never in the way of the walk: said beside it, and a save that could
 // not be said is logged — the delivery's owner, which also asks for the walks it follows, finds it by

cmd/mesh-controller/delivery_edges_test.go

@@ -181,32 +181,3 @@ func TestATruncatedChangeIsReadAsChangingTheManifestsOfWhatItMoves(t *testing.T)
 		t.Fatalf("the member's change is read as %+v", src)
 	}
 }
-
-// A lone pull request's cycle fails its check, naming every module and edge, with no group and no members.
-func TestALonePullRequestInACycleFailsWithNoGroup(t *testing.T) {
-	was, wasParent := manifestAt, parentOf
-	t.Cleanup(func() { manifestAt, parentOf = was, wasParent })
-	manifestsReadMu.Lock()
-	clear(manifestsRead)
-	manifestsReadMu.Unlock()
-	const head = "3333333333333333333333333333333333333333"
-	parentOf = func(context.Context, string, string, string) (string, error) { return "", nil }
-	manifestAt = func(_ context.Context, _, _, file, ref string) (catalogue.Manifest, bool, error) {
-		if ref == "main" {
-			return catalogue.Manifest{}, false, nil
-		}
-		if file == "modules/x/module.json" {
-			return catalogue.Manifest{Module: "mx", DefinesSeats: []catalogue.SeatDeclaration{{Name: "s", Serves: []catalogue.Verb{{Name: "a"}}}},
-				Invokes: []string{"seat:s.b"}}, true, nil
-		}
-		return catalogue.Manifest{Module: "my", DefinesSeats: []catalogue.SeatDeclaration{{Name: "s", Serves: []catalogue.Verb{{Name: "b"}}}},
-			Invokes: []string{"seat:s.a"}}, true, nil
-	}
-	m := orderMember{ID: "novox/r@" + head[:12], Repository: "novox/r", Base: "main", Head: head, Number: 5,
-		Paths: []string{"modules/x/module.json", "modules/y/module.json"}}
-	c, cyclic, err := cycleVerdict(context.Background(), "", []orderMember{m}, map[string]orderReach{m.ID: {Moved: []string{"mx", "my"}}}, nil, nil)
-	if err != nil || !cyclic || c.Verdict != "fail" || c.Group != "" || len(c.Members) != 0 || !strings.Contains(c.Summary, "s.a") ||
-		!strings.Contains(c.Summary, "my") {
-		t.Fatalf("the verdict is %+v (cyclic %v, %v)", c, cyclic, err)
-	}
-}

cmd/mesh-controller/pull_declares.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"io"
 	"path"
@@ -138,71 +137,6 @@ func tellPullDeclares(ctx context.Context, w io.Writer, pulls []pullAt) {
 	}
 }
 
-// heldFor asks mesh-config for the approved held values of these pull requests at their heads (ADR 0502 clause 4). Only
-// those of the pull requests asked about are carried. A failure is said to w and answers none.
-func heldFor(ctx context.Context, w io.Writer, pulls []pullAt) []link.HeldConfig {
-	if len(pulls) == 0 {
-		return nil
-	}
-	type pullHead struct {
-		Pull string `json:"pull"`
-		Head string `json:"head"`
-	}
-	asked := make([]pullHead, 0, len(pulls))
-	for _, p := range pulls {
-		asked = append(asked, pullHead{p.key(), p.New})
-	}
-	raw, err := meshConfigAsk(ctx, map[string]any{"action": "held", "pulls": asked})
-	if err != nil {
-		fmt.Fprintf(w, "mesh-config cannot say what it holds for these pull requests (%v); the check goes on without held values\n", err)
-		return nil // empty-on-error: said above; a check without held values fails for want of the value and says so, it never passes on a guess
-	}
-	var answer struct {
-		Held    []link.HeldConfig `json:"held"`
-		Skipped []struct {
-			Pull string `json:"pull"`
-			Why  string `json:"why"`
-		} `json:"skipped"`
-	}
-	if len(raw) == 0 {
-		return nil
-	}
-	if err := json.Unmarshal(raw, &answer); err != nil {
-		fmt.Fprintf(w, "mesh-config's held values cannot be read (%v); the check goes on without them\n", err)
-		return nil // empty-on-error: said above; a check without held values fails for want of the value and says so, it never passes on a guess
-	}
-	for _, s := range answer.Skipped {
-		fmt.Fprintf(w, "%s: mesh-config holds a value it does not carry: %s\n", s.Pull, s.Why)
-	}
-	var out []link.HeldConfig
-	for _, h := range answer.Held {
-		for _, p := range pulls {
-			if h.Pull == p.key() {
-				out = append(out, h)
-				break
-			}
-		}
-	}
-	return out
-}
-
-// withHeldConfig tells mesh-config what these pull requests declare and puts the approved held values of them on the
-// check request (ADR 0502 clause 4). With carry false, a member of a group whose composed check composes, only the
-// declarations are told.
-func withHeldConfig(ctx context.Context, w io.Writer, request *link.BuildRequest, pulls []pullAt, carry bool) {
-	tellPullDeclares(ctx, w, pulls)
-	if carry {
-		carryHeld(ctx, w, request, pulls)
-	}
-}
-
-// carryHeld puts the approved held values of these pull requests on the check request.
-func carryHeld(ctx context.Context, w io.Writer, request *link.BuildRequest, pulls []pullAt) {
-	if request.Check != nil {
-		request.Check.HeldConfig = heldFor(ctx, w, pulls)
-	}
-}
-
 // holdsModules is whether a module of the mesh is built from this repository.
 func holdsModules(entries []inventory.Entry, owner, repo string) bool {
 	return slices.ContainsFunc(entries, func(e inventory.Entry) bool {

cmd/mesh-controller/pull_declares_test.go

@@ -9,7 +9,6 @@ import (
 
 	"git.novox.be/novox/mesh-sdk/go/catalogue"
 	"github.com/novox/mesh-controller/internal/inventory"
-	"github.com/novox/mesh-controller/internal/link"
 )
 
 const (
@@ -153,50 +152,8 @@ func TestMeshConfigNotAnsweringIsSaidAndGoneOn(t *testing.T) {
 	meshConfigAsk = func(context.Context, map[string]any) ([]byte, error) { return nil, io.ErrUnexpectedEOF }
 	var said strings.Builder
 	tellPullDeclares(context.Background(), &said, []pullAt{thePull()})
-	got := heldFor(context.Background(), &said, []pullAt{thePull()})
-	if len(got) != 0 || !strings.Contains(said.String(), "mesh-config") {
-		t.Fatalf("held %v, said %q", got, said.String())
-	}
-}
-
-// 1826/AC2: the held values mesh-config answers for the check's pull requests are what the check request carries.
-func TestTheHeldValuesOfTheChecksPullRequestsAreAsked(t *testing.T) {
-	calls := fakeConfig(t, map[string]string{"held": `{"held":[{"pull":"novox/mesh-catalog#434","head":"` + headSHA +
-		`","node":"ace","module":"n8n","key":"media-gid","value":2000,"approved":"the operator"}]}`})
-	other := pullAt{Number: 7, changeSource: changeSource{Owner: "novox", Repo: "mesh-controller", New: baseSHA}}
-	got := heldFor(context.Background(), io.Discard, []pullAt{thePull(), other})
-	if len(got) != 1 || got[0].Node != "ace" || got[0].Key != "media-gid" || string(got[0].Value) != "2000" {
-		t.Fatalf("got %+v", got)
-	}
-	c := (*calls)[0]
-	pulls, _ := json.Marshal(c["pulls"])
-	if c["action"] != "held" || string(pulls) != `[{"pull":"novox/mesh-catalog#434","head":"`+headSHA+
-		`"},{"pull":"novox/mesh-controller#7","head":"`+baseSHA+`"}]` {
-		t.Fatalf("asked %v %s", c, pulls)
-	}
-}
-
-// 1826/AC2: a value held for a pull request the check is not of is never carried.
-func TestAHeldValueOfAnotherPullRequestIsNotCarried(t *testing.T) {
-	fakeConfig(t, map[string]string{"held": `{"held":[{"pull":"novox/mesh-catalog#999","head":"x","node":"ace","module":"n8n","key":"k","value":1}]}`})
-	if got := heldFor(context.Background(), io.Discard, []pullAt{thePull()}); len(got) != 0 {
-		t.Fatalf("carried %+v", got)
-	}
-}
-
-// 1826/AC2: the check request carries what mesh-config answered; a member of a composed group asks none.
-func TestTheCheckRequestCarriesTheHeldValues(t *testing.T) {
-	fakeManifests(t, map[string]*catalogue.Manifest{baseSHA: manifest("n8n", nil), headSHA: manifest("n8n", nil)})
-	fakeConfig(t, map[string]string{"held": `{"held":[{"pull":"novox/mesh-catalog#434","node":"ace","module":"n8n","key":"media-gid","value":2000}]}`})
-	request := link.BuildRequest{Check: &link.CheckRequest{}}
-	withHeldConfig(context.Background(), io.Discard, &request, []pullAt{thePull()}, true)
-	if len(request.Check.HeldConfig) != 1 || request.Check.HeldConfig[0].Key != "media-gid" {
-		t.Fatalf("the request carries %+v", request.Check.HeldConfig)
-	}
-	request = link.BuildRequest{Check: &link.CheckRequest{}}
-	withHeldConfig(context.Background(), io.Discard, &request, []pullAt{thePull()}, false)
-	if len(request.Check.HeldConfig) != 0 {
-		t.Fatalf("a composed member carries %+v", request.Check.HeldConfig)
+	if !strings.Contains(said.String(), "mesh-config") {
+		t.Fatalf("said %q", said.String())
 	}
 }