From 37fd73f3781ef876803428daeeaf8fba3f0a70a5 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Thu, 15 Oct 2026 23:27:12 +0000 Subject: [PATCH] Check a delivery group's member beside its siblings' merges, judged by the group's controller (issue 867, ADR 0382) A mesh-host change and a mesh-controller change that need each other could not both pass their own checks: each read the other at its base. The check now asks mesh-delivery for the group's open siblings into a base of the same name, reads each at its head merged onto its base, builds the judge from a controller sibling's merge in the isolated judge environment keyed on its inputs, and names the siblings in the verdict. With no answer from mesh-delivery the verdict says it could not learn them; a pull request with no sibling is checked as before. TestReplay867 replays the failure. Co-Authored-By: Claude --- cmd/mesh-builder/main.go | 19 +- cmd/mesh-builder/release_test.go | 19 ++ cmd/mesh-controller/check_siblings.go | 120 ++++++++ cmd/mesh-controller/check_siblings_test.go | 233 ++++++++++++++ cmd/mesh-controller/checks.go | 3 + cmd/mesh-controller/group_wait.go | 51 +++- cmd/mesh-controller/replay867_test.go | 291 ++++++++++++++++++ internal/builder/check.go | 192 +++++++++++- internal/builder/siblings_test.go | 334 +++++++++++++++++++++ internal/link/build.go | 26 ++ internal/link/events.go | 7 + testdata/store-tests | 1 + 12 files changed, 1277 insertions(+), 19 deletions(-) create mode 100644 cmd/mesh-controller/check_siblings.go create mode 100644 cmd/mesh-controller/check_siblings_test.go create mode 100644 cmd/mesh-controller/replay867_test.go create mode 100644 internal/builder/siblings_test.go diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index 12b6f527..ff1e1062 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -337,7 +337,8 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri 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), Head: v.Head, Trunk: v.Trunk, Beside: v.Beside, Built: checkBuiltOf(v.Built), - Release: v.Release, JudgedBy: v.JudgedBy, BesideAt: v.BesideAt, Validator: v.Validator} + Release: v.Release, JudgedBy: v.JudgedBy, BesideAt: v.BesideAt, Validator: v.Validator, + Siblings: siblingsOut(v.Siblings), SiblingsUnknown: v.SiblingsUnknown, JudgeFrom: v.JudgeFrom} // 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, @@ -467,7 +468,11 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { Owner: c.Owner, Repo: c.Repo, Number: c.Number, Paths: c.Paths, Beside: map[string]builder.Beside{}, Modules: c.Modules, New: c.New, Manifests: c.Manifests, Judge: c.Judge, Base: c.Base, Toolchain: builder.ToolchainOf(request.Held), Toolchains: builder.ToolchainsOf(request.Held), - Tests: c.Tests, StoreOnly: c.StoreOnly, Release: c.Release} + Tests: c.Tests, StoreOnly: c.StoreOnly, Release: c.Release, SiblingsUnknown: c.SiblingsUnknown} + for _, s := range c.Siblings { + spec.Siblings = append(spec.Siblings, builder.Sibling{Repository: s.Repository, Number: s.Number, Head: s.Head, + Base: s.Base, Dir: s.Dir}) + } for dir, b := range c.Beside { spec.Beside[dir] = builder.Beside{Repository: b.Repository, Ref: b.Ref, Else: b.Else} } @@ -482,6 +487,16 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { return spec } +// siblingsOut is the siblings a check read as the outcome carries them (novox/hq ADR 0382 §4). +func siblingsOut(read []builder.JudgedSibling) []link.CheckSibling { + var out []link.CheckSibling + for _, s := range read { + out = append(out, link.CheckSibling{Repository: s.Repository, Number: s.Number, Head: s.Head, Base: s.Base, + BaseCommit: s.BaseCommit}) + } + return out +} + // judgeCache is the judge cache the build agent's manifest declares (MESH_JUDGE_CACHE, novox/hq ADR 0358 §6): a // directory mounted at the same path on the host, so a judge build's mounts name what the agent sees. Empty when the // agent declares none, and every judge is then built cold under its check's root. diff --git a/cmd/mesh-builder/release_test.go b/cmd/mesh-builder/release_test.go index 2eb9c8f3..17eef10f 100644 --- a/cmd/mesh-builder/release_test.go +++ b/cmd/mesh-builder/release_test.go @@ -3,6 +3,7 @@ package main import ( "testing" + "github.com/novox/mesh-controller/internal/builder" "github.com/novox/mesh-controller/internal/link" ) @@ -25,3 +26,21 @@ func TestTheBuildSeatHandsTheCheckItsReleaseBranch(t *testing.T) { t.Fatalf("with no judge cache declared, %q is used", got) } } + +// The siblings a check is asked to read reach the build seat's spec and, as read, the outcome (novox/hq ADR 0382). +func TestTheSiblingsAnAskNamesReachTheSpecAndTheOutcome(t *testing.T) { + spec := checkSpecOf(link.BuildRequest{Check: &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Base: "main", + Siblings: []link.CheckSibling{{Repository: "novox/mesh-controller", Number: 346, Head: "a", Base: "main", Dir: "mesh-controller"}}, + SiblingsUnknown: "why"}}) + if len(spec.Siblings) != 1 || spec.Siblings[0] != (builder.Sibling{Repository: "novox/mesh-controller", Number: 346, + Head: "a", Base: "main", Dir: "mesh-controller"}) || spec.SiblingsUnknown != "why" { + t.Fatalf("the spec reads %+v", spec) + } + out := siblingsOut([]builder.JudgedSibling{{Repository: "novox/mesh-controller", Number: 346, Head: "a", Base: "main", BaseCommit: "b"}}) + if len(out) != 1 || out[0].BaseCommit != "b" || out[0].Number != 346 { + t.Fatalf("the outcome reads %+v", out) + } + if plain := checkSpecOf(link.BuildRequest{Check: &link.CheckRequest{Owner: "o", Repo: "r"}}); plain.Siblings != nil { + t.Fatalf("a request that names none has siblings %+v", plain.Siblings) + } +} diff --git a/cmd/mesh-controller/check_siblings.go b/cmd/mesh-controller/check_siblings.go new file mode 100644 index 00000000..45f1d63b --- /dev/null +++ b/cmd/mesh-controller/check_siblings.go @@ -0,0 +1,120 @@ +package main + +import ( + "context" + "fmt" + "regexp" + "sort" + "strings" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// **A delivery group's member is checked beside the other members' heads** (novox/hq ADR 0382, issue 867). +// +// A mesh-host change and a mesh-controller change that need each other could not both pass their own merge checks: +// each was read beside the other's repository at the commit the mesh runs, which holds neither change. So the +// controller asks mesh-delivery's `groups` once for every pull request's check it asks (§1), and names, in the ask, +// the group's other open pull requests into a base of the same name, in a repository the build seat reads beside the +// check — mesh-lab never — each with its whole head. The build seat reads those repositories at the sibling's merge (§2). +// A pull request for which mesh-delivery answered and named no sibling is asked exactly as before (§7). + +// wholeCommit is a commit as the forge says a head: forty hexadecimal characters. +var wholeCommit = regexp.MustCompile(`^[0-9a-f]{40}$`) + +// besideRepositories are the repositories (lower case, owner/repo) a check reads beside its own, by the directory each +// is found under — the core's, as besideOf clones them, and mesh-sdk beside the controller; never mesh-lab, which is +// always read at its trunk. +func besideRepositories(entries []inventory.Entry) map[string]string { + read := map[string]string{} + for _, e := range entries { + dir, core := coreModules[e.Manifest.Module] + if !core || e.Provided || e.Source.Repository == "" { + continue + } + read[nameOf(e.Source.Repository)] = dir + if dir == "mesh-controller" && e.Source.Seat != "" { + read[nameOf(siblingOf(e.Source.Repository, "mesh-sdk"))] = "mesh-sdk" + } + } + delete(read, "") + return read +} + +// nameOf is a recorded repository as owner/repo in lower case, empty when it names no owner. +func nameOf(repository string) string { + owner, repo := sourceOwner(repository), sourceName(repository) + if owner == "" || repo == "" { + return "" + } + return strings.ToLower(owner + "/" + repo) +} + +// siblingsFor is the pull request's siblings, as mesh-delivery's groups say them (ADR 0382 §1): the other members of +// the group its head branch names that are open, unmerged and into a base of its own base's name, in a repository the +// check reads beside it (beside: the directories the ask already names). The second answer is why they could not be +// learned, empty when they could: the group's members carry no head or base — a mesh-delivery from before the rule. +func siblingsFor(p link.PullUpdated, entries []inventory.Entry, beside map[string]link.CheckedOut, + lines []groupsLine) ([]link.CheckSibling, string) { + if p.Head == "" { + return nil, "" + } + var group *groupsLine + for i := range lines { + if lines[i].ID == p.Head { + group = &lines[i] + } + } + if group == nil { + return nil, "" + } + own := strings.ToLower(p.Owner + "/" + p.Repo) + if len(group.Details) == 0 && len(group.Members) > 0 { + return nil, "mesh-delivery named the group's members without their heads and bases (a mesh-delivery from before novox/hq ADR 0382)" + } + read := besideRepositories(entries) + var siblings []link.CheckSibling + for _, m := range group.Details { + repository := strings.ToLower(m.Repository) + if repository == own { + continue + } + if !m.Open { + continue + } + if m.Head == "" || m.Base == "" { + return nil, "mesh-delivery named " + m.ID + " without its head or base (a mesh-delivery from before novox/hq ADR 0382)" + } + dir, readBeside := read[repository] + if m.Base != p.Base || !readBeside || dir == "mesh-lab" { + continue + } + if _, there := beside[dir]; !there { + continue + } + head := strings.ToLower(m.Head) + if !wholeCommit.MatchString(head) { + return nil, fmt.Sprintf("mesh-delivery named %s with a head that is not a whole commit (%q)", m.ID, m.Head) + } + siblings = append(siblings, link.CheckSibling{Repository: m.Repository, Number: m.Number, Head: head, Base: m.Base, Dir: dir}) + } + sort.Slice(siblings, func(i, j int) bool { return siblings[i].Dir < siblings[j].Dir }) + return siblings, "" +} + +// withSiblings asks mesh-delivery's `groups` once and says in the request the pull request's siblings, or why they +// could not be learned (ADR 0382 §1): a check never waits on mesh-delivery past groupAskWithin and never fails for it, +// so a fix to mesh-delivery is still checked without it. +func withSiblings(ctx context.Context, request *link.BuildRequest, p link.PullUpdated, entries []inventory.Entry) { + if request.Check == nil { + return + } + lines, err := groupsAsked(ctx, deliverySeatHeld(entries)) + if err != nil { + request.Check.SiblingsUnknown = fmt.Sprintf("mesh-delivery did not say the group's pull requests (%v): "+ + "the check reads no siblings", err) + return + } + request.Check.Siblings, request.Check.SiblingsUnknown = siblingsFor(p, entries, request.Check.Beside, lines) +} diff --git a/cmd/mesh-controller/check_siblings_test.go b/cmd/mesh-controller/check_siblings_test.go new file mode 100644 index 00000000..1322da7e --- /dev/null +++ b/cmd/mesh-controller/check_siblings_test.go @@ -0,0 +1,233 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// **A delivery group's member is checked beside the other members' heads** (novox/hq ADR 0382 §1, §7, issue 867): the +// controller asks mesh-delivery's `groups` once for every check it asks, and names in the ask the group's other open +// pull requests into a base of the same name, in a repository the build seat reads beside it — mesh-lab never. + +const ( + headHost = "1111111111111111111111111111111111111111" + headController = "2222222222222222222222222222222222222222" + headSDK = "3333333333333333333333333333333333333333" + headLab = "4444444444444444444444444444444444444444" + headHq = "5555555555555555555555555555555555555555" +) + +func theCoreOnTheGitSeat() []inventory.Entry { + onSeat := func(module, repository string) inventory.Entry { + e := fromRepo(module, repository, "") + e.Source.Seat = "git" + return e + } + nats := onSeat("nats", "novox/mesh-catalog") + nats.Source.Path = "modules/nats" + held := catalogue.Manifest{Module: "mesh-delivery", Claims: []catalogue.Claim{{Name: catalogue.DeliverySeat, Scope: catalogue.ScopeMesh}}} + return []inventory.Entry{onSeat("mesh-controller", "novox/mesh-controller"), onSeat("mesh-host", "novox/mesh-host"), + onSeat("node-tools", "novox/mesh-tools"), nats, {Manifest: held, On: []string{"anchor"}}} +} + +func besideTheCore() map[string]link.CheckedOut { + return map[string]link.CheckedOut{"mesh-controller": {Repository: "c"}, "mesh-host": {Repository: "h"}, + "mesh-tools": {Repository: "t"}, "mesh-catalog": {Repository: "k"}, "mesh-sdk": {Repository: "s"}, "mesh-lab": {Repository: "l"}} +} + +func member(repository string, number int, head, base string, open bool) memberLine { + return memberLine{ID: repository + "@" + head[:12], Repository: repository, State: "checked", Number: number, Head: head, + Base: base, Open: open} +} + +func group(id string, members ...memberLine) groupsLine { + line := groupsLine{ID: id, Details: members} + for _, m := range members { + line.Members = append(line.Members, m.ID+" ("+m.State+")") + } + return line +} + +func TestASiblingIsTheGroupsOtherOpenPullRequestIntoABaseOfTheSameName(t *testing.T) { + entries := theCoreOnTheGitSeat() + pull := link.PullUpdated{Owner: "novox", Repo: "mesh-host", Number: 114, Base: "main", Head: "feat/x", Commit: headHost} + for _, c := range []struct { + what string + lines []groupsLine + want []string + unknown string + }{ + {"the other member, open, same base", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-controller", 346, headController, "main", true))}, + []string{"mesh-controller=novox/mesh-controller#346@" + headController + " onto main"}, ""}, + {"a member into a base of another name is not one", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-controller", 346, headController, "release", true))}, + nil, ""}, + {"a member not open (merged or closed) is not one", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-controller", 346, headController, "main", false))}, + nil, ""}, + {"mesh-lab is never one", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-lab", 5, headLab, "main", true))}, + nil, ""}, + {"a repository the check reads nothing of is not one", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/hq", 9, headHq, "main", true))}, + nil, ""}, + {"mesh-sdk, read beside the controller, is one", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-sdk", 7, headSDK, "main", true))}, + []string{"mesh-sdk=novox/mesh-sdk#7@" + headSDK + " onto main"}, ""}, + {"another group is none of its business", []groupsLine{group("feat/y", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-controller", 346, headController, "main", true))}, + nil, ""}, + {"a group of its own alone has none", []groupsLine{group("feat/x", member("novox/mesh-host", 114, headHost, "main", true))}, + nil, ""}, + {"an older mesh-delivery names no heads", []groupsLine{{ID: "feat/x", + Members: []string{"novox/mesh-host@111111111111 (checked)", "novox/mesh-controller@222222222222 (checked)"}}}, + nil, "without their heads and bases"}, + {"a member without its base", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-controller", 346, headController, "", true))}, + nil, "without its head or base"}, + {"a head that is no commit", []groupsLine{group("feat/x", + member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-controller", 346, headController[:12]+"zzzzzzzzzzzzzzzzzzzzzzzzzzzz", "main", true))}, + nil, "not a whole commit"}, + } { + got, unknown := siblingsFor(pull, entries, besideTheCore(), c.lines) + var said []string + for _, s := range got { + said = append(said, s.Dir+"="+s.Repository+"#"+itoa(s.Number)+"@"+s.Head+" onto "+s.Base) + } + if strings.Join(said, ";") != strings.Join(c.want, ";") { + t.Errorf("%s: siblings %v, wanted %v", c.what, said, c.want) + } + if (c.unknown == "") != (unknown == "") || !strings.Contains(unknown, c.unknown) { + t.Errorf("%s: could not learn %q, wanted %q", c.what, unknown, c.unknown) + } + } + // A pull request into a release branch has siblings into the release branch of the same name (ADR 0358 §1). + release := pull + release.Base = "feat/57" + lines := []groupsLine{group("feat/x", member("novox/mesh-host", 114, headHost, "feat/57", true), + member("novox/mesh-controller", 346, headController, "feat/57", true), member("novox/mesh-tools", 8, headLab, "main", true))} + if got, _ := siblingsFor(release, entries, besideTheCore(), lines); len(got) != 1 || got[0].Dir != "mesh-controller" { + t.Errorf("into a release branch: %+v", got) + } + // A pull request on no head branch is in no group. + pull.Head = "" + if got, unknown := siblingsFor(pull, entries, besideTheCore(), lines); got != nil || unknown != "" { + t.Errorf("no head branch: %+v %q", got, unknown) + } +} + +func itoa(n int) string { b, _ := json.Marshal(n); return string(b) } + +// Every check asks mesh-delivery's `groups` once; the ask names the sibling's repository, head and base. +func TestEveryCheckAsksMeshDeliveryOnceAndNamesItsSiblings(t *testing.T) { + entries := theCoreOnTheGitSeat() + asked := 0 + was := groupsAsked + t.Cleanup(func() { groupsAsked = was }) + groupsAsked = func(_ context.Context, held bool) ([]groupsLine, error) { + asked++ + if !held { + t.Error("the seat is held, yet it was asked as not held") + } + return []groupsLine{group("feat/x", member("novox/mesh-host", 114, headHost, "main", true), + member("novox/mesh-controller", 346, headController, "main", true))}, nil + } + request := link.BuildRequest{Check: &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Base: "main", Beside: besideTheCore()}} + withSiblings(t.Context(), &request, link.PullUpdated{Owner: "novox", Repo: "mesh-host", Base: "main", Head: "feat/x"}, entries) + if asked != 1 { + t.Fatalf("mesh-delivery was asked %d times for one check", asked) + } + want := link.CheckSibling{Repository: "novox/mesh-controller", Number: 346, Head: headController, Base: "main", Dir: "mesh-controller"} + if got := request.Check.Siblings; len(got) != 1 || got[0] != want || request.Check.SiblingsUnknown != "" { + t.Fatalf("the ask names siblings %+v, unknown %q", got, request.Check.SiblingsUnknown) + } + body, _ := json.Marshal(request.Check) + if !strings.Contains(string(body), `"siblings":[{"repository":"novox/mesh-controller","number":346,"head":"`+headController+ + `","base":"main","dir":"mesh-controller"}]`) { + t.Fatalf("on the wire: %s", body) + } +} + +// When mesh-delivery does not answer in time (or is not held, or answered without the fields), the check reads no +// sibling and its request says it could not learn them, so a fix to mesh-delivery is still checked without it. +func TestWhenMeshDeliveryDoesNotAnswerTheCheckReadsNoSiblingAndSaysSo(t *testing.T) { + was := groupsAsked + t.Cleanup(func() { groupsAsked = was }) + for _, c := range []struct { + what string + err error + held bool + }{{"it did not answer", context.DeadlineExceeded, true}, {"nothing serves it", errors.New("nothing serves mesh-delivery.groups"), true}, + {"no holder on record", errors.New("no holder of mesh-delivery is on record to say it"), false}} { + entries := theCoreOnTheGitSeat() + if !c.held { + entries = entries[:4] + } + groupsAsked = func(_ context.Context, held bool) ([]groupsLine, error) { + if held != c.held { + t.Errorf("%s: held %v", c.what, held) + } + return nil, c.err + } + request := link.BuildRequest{Check: &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Base: "main", Beside: besideTheCore()}} + withSiblings(t.Context(), &request, link.PullUpdated{Owner: "novox", Repo: "mesh-host", Base: "main", Head: "feat/x"}, entries) + if len(request.Check.Siblings) != 0 || !strings.Contains(request.Check.SiblingsUnknown, c.err.Error()) || + !strings.Contains(request.Check.SiblingsUnknown, "reads no siblings") { + t.Errorf("%s: siblings %+v, unknown %q", c.what, request.Check.Siblings, request.Check.SiblingsUnknown) + } + } +} + +// **A pull request for which mesh-delivery answered and named no sibling is asked exactly as before** (ADR 0382 §7): +// byte for byte the request it was without the rule. +func TestAPullRequestWithNoSiblingIsAskedByteForByteAsBefore(t *testing.T) { + entries := theCoreOnTheGitSeat() + was := groupsAsked + t.Cleanup(func() { groupsAsked = was }) + for _, answer := range [][]groupsLine{nil, {group("feat/other", member("novox/mesh-controller", 346, headController, "main", true))}, + {group("feat/x", member("novox/mesh-host", 114, headHost, "main", true), member("novox/hq", 3, headHq, "main", true))}} { + groupsAsked = func(context.Context, bool) ([]groupsLine, error) { return answer, nil } + build := func() link.BuildRequest { + return link.BuildRequest{ID: "build-1", Repository: "r", Ref: headHost, Check: &link.CheckRequest{Owner: "novox", + Repo: "mesh-host", Number: 114, Base: "main", Beside: besideTheCore(), Modules: []string{"mesh-host"}, Judge: "validator"}} + } + before, _ := json.Marshal(build()) + after := build() + withSiblings(t.Context(), &after, link.PullUpdated{Owner: "novox", Repo: "mesh-host", Base: "main", Head: "feat/x"}, entries) + got, _ := json.Marshal(after) + if string(before) != string(got) { + t.Fatalf("the request changed:\nbefore %s\nafter %s", before, got) + } + } +} + +// What the build seat's verdict says of its siblings reaches the forge's holder in the controller's `checked`. +func TestTheVerdictCarriesTheSiblingsAndWhereTheJudgeCameFrom(t *testing.T) { + read := []link.CheckSibling{{Repository: "novox/mesh-controller", Number: 346, Head: headController, Base: "main", BaseCommit: headHost}} + c := checkedOf(link.BuildResult{ID: "b", Ref: headHost, Checked: &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Number: 114}, + Check: &link.CheckOutcome{Verdict: "pass", Summary: "ok", Siblings: read, JudgeFrom: "sibling novox/mesh-controller#346 merge abc", + Validator: "the change's own validator"}}) + if len(c.Siblings) != 1 || c.Siblings[0] != read[0] || c.JudgeFrom == "" || c.Validator == "" { + t.Fatalf("the verdict says %+v", c) + } + body, _ := json.Marshal(c) + for _, want := range []string{`"siblings":[{"repository":"novox/mesh-controller","number":346,"head":"` + headController + + `","base":"main","base_commit":"` + headHost + `"}]`, `"judge_from":"sibling novox/mesh-controller#346 merge abc"`} { + if !strings.Contains(string(body), want) { + t.Fatalf("the verdict lacks %s:\n%s", want, body) + } + } + unknown := checkedOf(link.BuildResult{ID: "b", Check: &link.CheckOutcome{Verdict: "pass", SiblingsUnknown: "mesh-delivery did not answer"}}) + if body, _ := json.Marshal(unknown); !strings.Contains(string(body), `"siblings_unknown":"mesh-delivery did not answer"`) || + strings.Contains(string(body), `"siblings":`) { + t.Fatalf("the verdict that could not learn them says %s", body) + } +} diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 630352d5..5ddbd7ee 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -180,6 +180,8 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { return err } request.Check.Plan = &plan + // **The group's other open pull requests are read beside it** (novox/hq ADR 0382 §1). + withSiblings(ctx, &request, p, entries) seat := buildSeatHeld(ctx) ask, err := askOverOn(seat) if err != nil { @@ -600,6 +602,7 @@ func checkedOf(result link.BuildResult) link.Checked { c.Built = result.Check.Built c.Release, c.JudgedBy, c.BesideAt = result.Check.Release, result.Check.JudgedBy, result.Check.BesideAt c.Validator = result.Check.Validator + c.Siblings, c.SiblingsUnknown, c.JudgeFrom = result.Check.Siblings, result.Check.SiblingsUnknown, result.Check.JudgeFrom 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} diff --git a/cmd/mesh-controller/group_wait.go b/cmd/mesh-controller/group_wait.go index 391a9b28..12cc63a3 100644 --- a/cmd/mesh-controller/group_wait.go +++ b/cmd/mesh-controller/group_wait.go @@ -34,31 +34,58 @@ type groupMember struct { Repository, State string } -// deliveryGroupOf is what mesh-delivery says of the group a head names: its members, and whether the head names a -// group at all; an error when it cannot be asked or did not answer. A seam a test replaces. -var deliveryGroupOf = func(ctx context.Context, held bool, head string) ([]groupMember, bool, error) { +// groupsLine is one group as mesh-delivery's `groups` says it: its id (the head branch name), its members as text and, +// from a mesh-delivery that knows novox/hq ADR 0382, each member again as an object (`member_details`) naming its pull +// request's number, whole head, base branch and whether it is open. +type groupsLine struct { + ID string `json:"id"` + Members []string `json:"members"` + Details []memberLine `json:"member_details,omitempty"` +} + +// memberLine is one member of a group, as `groups` says it beside its text. +type memberLine struct { + ID string `json:"id"` + Repository string `json:"repository"` + State string `json:"state"` + Number int `json:"number"` + Head string `json:"head"` + Base string `json:"base"` + Open bool `json:"open"` +} + +// groupsAsked is mesh-delivery's `groups`: every group it keeps, or an error when it cannot be asked or did not +// answer within groupAskWithin. A seam a test replaces. +var groupsAsked = func(ctx context.Context, held bool) ([]groupsLine, error) { if !held { - return nil, false, fmt.Errorf("no holder of %s is on record to say it", catalogue.DeliverySeat) + return nil, fmt.Errorf("no holder of %s is on record to say it", catalogue.DeliverySeat) } serving := servingBus.Load() if serving == nil { - return nil, false, errors.New("this controller is not on the bus") + return nil, errors.New("this controller is not on the bus") } asking, cancel := context.WithTimeout(ctx, groupAskWithin) defer cancel() raw, err := askDeliveryOwner(asking, serving.Conn(), "groups", map[string]any{}) if err != nil { if errors.Is(err, link.ErrNothingServes) { - return nil, false, fmt.Errorf("nothing serves %s.groups", catalogue.DeliverySeat) + return nil, fmt.Errorf("nothing serves %s.groups", catalogue.DeliverySeat) } - return nil, false, err - } - var lines []struct { - ID string `json:"id"` - Members []string `json:"members"` + return nil, err } + var lines []groupsLine if err := json.Unmarshal(raw, &lines); err != nil { - return nil, false, fmt.Errorf("%s.groups answered something unreadable: %w", catalogue.DeliverySeat, err) + return nil, fmt.Errorf("%s.groups answered something unreadable: %w", catalogue.DeliverySeat, err) + } + return lines, nil +} + +// deliveryGroupOf is what mesh-delivery says of the group a head names: its members, and whether the head names a +// group at all; an error when it cannot be asked or did not answer. A seam a test replaces. +var deliveryGroupOf = func(ctx context.Context, held bool, head string) ([]groupMember, bool, error) { + lines, err := groupsAsked(ctx, held) + if err != nil { + return nil, false, err } for _, l := range lines { if l.ID != head { diff --git a/cmd/mesh-controller/replay867_test.go b/cmd/mesh-controller/replay867_test.go new file mode 100644 index 00000000..4194cde6 --- /dev/null +++ b/cmd/mesh-controller/replay867_test.go @@ -0,0 +1,291 @@ +package main + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/nats-io/nats.go" + + "github.com/novox/mesh-controller/internal/builder" + "github.com/novox/mesh-controller/internal/catalogue" + meshfacts "github.com/novox/mesh-controller/internal/facts" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// TestReplay867 is the replay of novox/hq issue 867, written with only what the controller and the build seat had +// before the fix, so mesh-lab's prover can lay it over the commit before. A mesh-host change (#114) and a +// mesh-controller change (#346) of one delivery group need each other: the node-engine change's own check reads the +// controller's lock beside it, which only the controller change brings; the controller change builds only with the +// validator word only the node-engine change brings. Each was read beside the other's repository at the commit the mesh +// runs, which holds neither change, so neither could pass its own merge check — and the pair could not merge. +// +// It runs the whole path: the forge announces each pull request (following.PullUpdated), the controller asks +// mesh-delivery which pull requests the group holds, the ask is queued for the build seat, and the build seat's check +// (builder.Check, with a container runtime that runs each container's command on this machine) judges it. +func TestReplay867(t *testing.T) { + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no Go toolchain on this machine to build a judge with") + } + r := theGroupOfIssue867(t) + for _, c := range []struct { + what, repo string + number int + head string + module string + }{{"the node-engine change", "mesh-host", 114, r.hostHead, "mesh-host"}, + {"the controller change", "mesh-controller", 346, r.controllerHead, "mesh-controller"}} { + v := r.checked(t, c.repo, c.number, c.head) + if v.Gate == nil || v.Gate.Verdict != "pass" || v.Repo == nil || v.Repo.Verdict != "pass" { + t.Errorf("%s (%s#%d) did not pass its own merge check beside its group: gate %+v, repository %+v\n%s", + c.what, c.repo, c.number, v.Gate, v.Repo, v.Report) + } + } +} + +// theIssue867 is the two repositories as the trunk holds them and each pull request's head. +type theIssue867 struct { + open *stores + hostHead, controllerHead string + js interface{ Conn() *nats.Conn } + judgeCache string + registry string +} + +func r867Git(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.org", + "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.org") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +func r867Commit(t *testing.T, dir string, files map[string]string) string { + t.Helper() + for name, body := range files { + if err := os.MkdirAll(filepath.Dir(filepath.Join(dir, name)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + r867Git(t, dir, "add", "-A") + r867Git(t, dir, "commit", "--quiet", "-m", "x") + return r867Git(t, dir, "rev-parse", "HEAD") +} + +// r867Controller is a controller whose judge says who it is and the validator word it was built with. +func r867Controller(who, extra string) map[string]string { + return map[string]string{ + "go.mod": "module example.org/judge\n\ngo 1.22\n\nrequire github.com/novox/mesh-host v0.0.0\n", + "vendor/modules.txt": "# github.com/novox/mesh-host v0.0.0\n## explicit; go 1.22\ngithub.com/novox/mesh-host/validate\n", + "vendor/github.com/novox/mesh-host/validate/v.go": "package validate\n\nconst Word = \"the vendored validator\"\n", + "vendor/github.com/novox/mesh-host/internal/declaration/d.go": "package declaration\n", + "cmd/mesh-controller/main.go": `package main + +import ( + "fmt" + "os" + + "github.com/novox/mesh-host/validate" +) + +func main() { + if len(os.Args) == 2 && os.Args[1] == "merge-gate" { + fmt.Println("usage: merge-gate --facts --store ") + os.Exit(2) + } + fmt.Printf("{\"verdict\":\"pass\",\"summary\":\"%s with %s\"}\n", "` + who + `", validate.Word` + extra + `) +} +`, + } +} + +func theGroupOfIssue867(t *testing.T) *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") + for _, d := range []string{controller, host} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + r867Git(t, d, "init", "--quiet", "-b", "main") + } + // The trunks. + files := r867Controller("the controller", "") + files["lock.txt"] = "foundation: old\n" + files["module.json"] = `{"module":"mesh-controller"}` + files[builder.CheckScript] = "true\n" + r867Commit(t, controller, files) + 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"}) + // The two pull requests of one group, feat/867. + r867Git(t, controller, "checkout", "--quiet", "-b", "feat/867", "main") + 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") + 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") + + 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 { + t.Fatal(err) + } + } + holdTheDeliverySeat(t, open) + + // mesh-delivery says the group: two open pull requests into main, each with its head. + was := askDeliveryOwner + t.Cleanup(func() { askDeliveryOwner = was }) + askDeliveryOwner = func(_ context.Context, _ *nats.Conn, verb string, _ map[string]any) (json.RawMessage, error) { + if verb != "groups" { + t.Fatalf("asked the owner %s", verb) + } + return json.RawMessage(`[{"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},` + + `{"id":"novox/mesh-controller@` + r.controllerHead[:12] + `","repository":"novox/mesh-controller","state":"checked","number":346,"head":"` + + r.controllerHead + `","base":"main","open":true}]}]`), nil + } + r.registry = r867Registry(t) + r867Runtime(t) + return r +} + +// checked is a pull request announced by the forge, asked of the build seat by the controller, and checked there. +func (r *theIssue867) checked(t *testing.T, repo string, number int, head string) builderVerdict { + t.Helper() + js, ask := aBuildQueue(t) + _ = ask + servingBus.Store(js) + t.Cleanup(func() { servingBus.Store(nil) }) + p := link.PullUpdated{Owner: "novox", Repo: repo, Number: number, Base: "main", Head: "feat/867", Commit: head, + Paths: []string{"lock.txt", "validate/more.go", "cmd/mesh-controller/main.go"}, ModuleDirsSaid: true, + MergeCheck: true, MergeCheckSaid: true} + if err := (following{open: r.open, oneShot: true}).PullUpdated(t.Context(), p); err != nil { + t.Fatal(err) + } + q, err := link.ReadQueue(t.Context(), js, link.TheBuildMachine) + if err != nil { + t.Fatal(err) + } + if len(q.Asks) == 0 { + t.Fatalf("%s#%d: nothing was asked of the build seat", repo, number) + } + request := q.Asks[len(q.Asks)-1].Request + // What the build seat makes of the ask: the fields the two share, by name. + var spec builder.CheckSpec + body, _ := json.Marshal(request.Check) + if err := json.Unmarshal(body, &spec); err != nil { + t.Fatal(err) + } + 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) + } + return v +} + +type builderVerdict = builder.CheckVerdict + +func r867Registry(t *testing.T) string { + t.Helper() + body, err := json.Marshal(meshfacts.Facts{Format: meshfacts.Format, Taken: time.Now().UTC(), + Versions: meshfacts.Versions{Bus: "2.11.17", Store: "17.11"}, Machines: []meshfacts.Machine{{Name: "abcdef", Length: 6}}}) + if err != nil { + t.Fatal(err) + } + sum := sha256.Sum256(body) + digest := "sha256:" + hex.EncodeToString(sum[:]) + manifest, _ := json.Marshal(map[string]any{"schemaVersion": 2, "layers": []map[string]any{ + {"mediaType": meshfacts.MediaType, "digest": digest, "size": len(body)}}}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + switch { + case strings.Contains(req.URL.Path, "/manifests/"): + w.Write(manifest) + case strings.HasSuffix(req.URL.Path, "/blobs/"+digest): + w.Write(body) + default: + http.NotFound(w, req) + } + })) + t.Cleanup(srv.Close) + return strings.TrimPrefix(srv.URL, "http://") +} + +// r867Runtime puts a container runtime on PATH that runs every container's command on this machine, in the directory +// and with the environment the check gives it, and a store and a bus that answer. +func r867Runtime(t *testing.T) { + t.Helper() + bus, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { bus.Close() }) + go func() { + for { + c, err := bus.Accept() + if err != nil { + return + } + c.Close() + } + }() + dir := t.TempDir() + script := `#!/bin/sh +cmd="$1"; shift +case "$cmd" in +ps|rm|exec) exit 0 ;; +port) echo "` + bus.Addr().String() + `"; exit 0 ;; +run) + workdir=""; detached="" + while [ $# -gt 0 ]; do + case "$1" in + -d) detached=1; shift ;; + --rm) shift ;; + --workdir) workdir="$2"; shift 2 ;; + --env) export "$2"; shift 2 ;; + --network|--volume|--user|--label|--name|-p|-e) shift 2 ;; + *) break ;; + esac + done + if [ -n "$detached" ]; then echo a-container; exit 0; fi + shift + cd "$workdir" && exec "$@" ;; +esac +exit 1 +` + if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("GOTOOLCHAIN", "local") +} diff --git a/internal/builder/check.go b/internal/builder/check.go index cac6b0ad..f6a5a7ff 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -4,6 +4,8 @@ import ( "bufio" "bytes" "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -122,6 +124,42 @@ type CheckSpec struct { // request (novox/hq issue 686): handed to the script as EnvTests and EnvStoreOnly, which it may read. Tests string StoreOnly bool + // Siblings are the other open pull requests of this one's delivery group, each read beside the check at its head + // merged onto its base head in place of the commit the mesh runs (novox/hq ADR 0382 §2). SiblingsUnknown says the + // controller could not learn them, and the verdict says so. + Siblings []Sibling + SiblingsUnknown string +} + +// Sibling is one other open pull request of a delivery group (novox/hq ADR 0382): the repository it is in as +// owner/repo, its number, its whole head, the branch it merges into, and Dir, the directory the check finds that +// repository under beside it (an entry of Beside). +type Sibling struct { + Repository string + Number int + Head string + Base string + Dir string +} + +// JudgedSibling is a sibling as the check read it (ADR 0382 §4): its head, and BaseCommit, the base's head the head was +// merged onto — both whole. +type JudgedSibling struct { + Repository string + Number int + Head string + Base string + BaseCommit string +} + +// siblingIn is the sibling read under a directory, nil when none is. +func siblingIn(spec CheckSpec, dir string) *Sibling { + for i := range spec.Siblings { + if spec.Siblings[i].Dir == dir { + return &spec.Siblings[i] + } + } + return nil } // selection is what a check hands the repository's script to narrow what it runs (novox/hq issue 686): nothing @@ -206,6 +244,12 @@ type CheckVerdict struct { // mesh-host's of the release branch with its commit, the one the release branch's controller vendors, the // change's own, or the one the change's own controller vendors. Empty for a check of any other base. Validator string + // Siblings are the pull requests of the delivery group the check read beside it, each merged onto its base commit + // (novox/hq ADR 0382 §4); SiblingsUnknown is why it could not learn them; JudgeFrom says where the judge came from + // when the group made it (§3), the validator's source being in Validator. + Siblings []JudgedSibling + SiblingsUnknown string + JudgeFrom string } // Layer is one layer of a check, judged. @@ -412,6 +456,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry named := func(v CheckVerdict) CheckVerdict { v.Head, v.Trunk, v.Merge, v.Beside, v.Members = judged.Head, judged.Trunk, judged.Merge, judged.Beside, judged.Members v.Release, v.JudgedBy, v.BesideAt, v.Validator = spec.Release, judged.JudgedBy, judged.BesideAt, judged.Validator + v.Siblings, v.SiblingsUnknown, v.JudgeFrom = judged.Siblings, spec.SiblingsUnknown, judged.JudgeFrom return v } // **What the merge would move is built from a checkout nothing of the check touches** (novox/hq issue 703): the @@ -467,7 +512,27 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry } b.Else = "" } - if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil { + if sibling := siblingIn(spec, dir); sibling != nil && dir != "mesh-lab" { + // **A sibling's repository is read at the sibling's merge** (novox/hq ADR 0382 §2): its head merged onto its + // base's head, as its own check merges it — in place of the commit the mesh runs. + if err := cloneAt(b.Repository, "", "", dir); err != nil { + return CheckVerdict{}, fmt.Errorf("beside it, %w", err) + } + m, err := mergeIntoTrunk(ctx, run, filepath.Join(root, dir), sibling.Base, sibling.Head) + if err != nil { + return CheckVerdict{}, fmt.Errorf("beside it, the sibling %s#%d: %w", sibling.Repository, sibling.Number, err) + } + judged.Siblings = append(judged.Siblings, JudgedSibling{Repository: sibling.Repository, Number: sibling.Number, + Head: m.Head, Base: sibling.Base, BaseCommit: m.Trunk}) + if len(m.Conflicts) > 0 { + said := fmt.Sprintf("the sibling %s#%d (head %s) does not merge cleanly into its base %s %s: in conflict %s", + sibling.Repository, sibling.Number, twelve(m.Head), sibling.Base, twelve(m.Trunk), strings.Join(m.Conflicts, ", ")) + say("check", "%s", said) + return named(CheckVerdict{Verdict: "fail", Summary: said, Report: said, Took: time.Since(began), + Gate: &Layer{Verdict: "fail", Summary: said}, Repo: &Layer{Verdict: "fail", Summary: said}}), nil + } + readAt[dir] = sibling.Base + } else if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil { return CheckVerdict{}, fmt.Errorf("beside it, %w", err) } if dir == "mesh-lab" { @@ -491,7 +556,12 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry } judged.Beside[dir] = at } - say("check", "beside it %s at %s", dir, refWords(readAt[dir])) + if sibling := siblingIn(spec, dir); sibling != nil && dir != "mesh-lab" { + say("check", "beside it %s at the merge of %s#%d (%s) onto %s", dir, sibling.Repository, sibling.Number, + short(sibling.Head), sibling.Base) + } else { + say("check", "beside it %s at %s", dir, refWords(readAt[dir])) + } } // A delivery group's other heads (novox/hq ADR 0239), each at its head, for the gate to compose with this. @@ -658,7 +728,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry } defer os.RemoveAll(judgeHome) inJudgeBuild := func(dir string, _ []string, command ...string) []string { - args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome, judgeGoCache), command...) + args := in(spec.Toolchain, dir, judgeBuildEnv(judgeHome, judgeGoCache), command...) if spec.JudgeCache == "" { return args // under the check's root, in the workspace mounted already } @@ -685,10 +755,15 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry // The judge. Its failing to build is the change's fault when the change is the judge or its validator, // and the check's when it is the controller the mesh runs. - gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, inJudgeBuild, say) + var siblingJudge siblingJudgement + gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, judged, &siblingJudge, inJudgeBuild, say) if err != nil { return CheckVerdict{}, err } + judged.JudgeFrom = siblingJudge.From + if siblingJudge.Validator != "" { + judged.Validator = siblingJudge.Validator + } if spec.Release != "" { // **A release-branch judgement says so** (novox/hq ADR 0358 §4): the branch, the controller's commit that // judged, and each repository read beside it at its branch and commit. @@ -697,7 +772,9 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry judgedBy = tree } judged.JudgedBy = headAt(ctx, run, judgedBy) - judged.Validator = releaseValidator(spec, judged.BesideAt, judged.Beside) + if siblingJudge.Validator == "" { + judged.Validator = releaseValidator(spec, judged.BesideAt, judged.Beside) + } said := releaseWords(spec.Release, judged.JudgedBy, judged.Validator, judged.BesideAt, judged.Beside) fmt.Fprintf(&out, "--- %s\n", said) say("check", "%s", said) @@ -1160,8 +1237,12 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil // has that branch too, or with the change's own validator for a change to the node-engine — never the controller the // mesh runs. besideAt is the branch each repository beside it was read at. func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, tree string, besideAt map[string]string, + judged CheckVerdict, siblings *siblingJudgement, inToolchain func(string, []string, ...string) []string, say func(step, format string, args ...any)) (string, string, error) { gated := spec.Gated() + if bin, fault, done, err := judgeFromTheGroup(ctx, labelled, run, spec, root, tree, judged, siblings, inToolchain, say); done { + return bin, fault, err + } switch spec.Judge { case "self": bin := filepath.Join(root, "bin", "judge-of-itself") @@ -1263,6 +1344,10 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s return bin, dir, nil } +// judgeBuildEnv is JudgeBuildEnv, a seam only a test replaces: to show what the plain environment would let a planted +// workspace do to a judge's build. +var judgeBuildEnv = JudgeBuildEnv + // JudgeBuildEnv is the environment every judge is built in (novox/hq ADR 0358 §6, issue 482): home, made fresh for the // build, as HOME and configuration; goCache, the judge cache's Go cache that no check's container mounts; no global // or system git configuration; no Go environment file; and no version stamped from git, so no git runs at all. @@ -1859,3 +1944,100 @@ func lastLine(s string) string { lines := strings.Split(strings.TrimSpace(s), "\n") return strings.TrimSpace(lines[len(lines)-1]) } + +// siblingJudgement says, besides the judge's path, where a judge the group made came from (novox/hq ADR 0382 §3): From +// 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 +} + +// judgeFromTheGroup builds the judge of a member that has siblings (novox/hq ADR 0382 §3); done says it did (or failed +// to), and false leaves the judge to the rules before it. +// +// - A member with a mesh-controller sibling is judged by the controller built from that sibling's merge (read beside +// the check), with the node-engine's validator from the member's own change when it is mesh-host's, else from a +// mesh-host sibling's merge, else the one that controller vendors. +// - A mesh-controller member is judged by itself, with a mesh-host sibling's validator in place of the one it vendors. +// +// **The judge is unreviewed code** and is built where nothing the change's script does can reach it: from a copy of the +// merge made apart from the workspace's tree, with the judge-build environment (JudgeBuildEnv) — a home and Go cache of +// its own, no global or system git configuration, no version stamping from git — and kept in the judge cache, which no +// check's container mounts, only for the same inputs: the controller merge and the base commits it was merged onto, the +// validator's source and commit. +func judgeFromTheGroup(ctx context.Context, labelled, run Runner, spec CheckSpec, root, tree string, judged CheckVerdict, + said *siblingJudgement, inToolchain func(string, []string, ...string) []string, + say func(step, format string, args ...any)) (string, string, bool, error) { + controller, host := siblingIn(spec, "mesh-controller"), siblingIn(spec, "mesh-host") + var source, validatorDir, from, validator string + var inputs []string + merge := func(dir string, s *Sibling) string { + return fmt.Sprintf("sibling %s#%d merge %s", s.Repository, s.Number, judged.Beside[dir]) + } + switch { + case spec.Judge == "self" && host != nil: + source, validatorDir = tree, filepath.Join(root, "mesh-host") + from = "this change's own merge " + judged.Head + validator = merge("mesh-host", host) + "'s validator" + case spec.Judge != "self" && controller != nil: + source = filepath.Join(root, "mesh-controller") + from = merge("mesh-controller", controller) + switch { + case spec.Judge == "validator": + validatorDir, validator = tree, "the change's own validator" + case host != nil: + validatorDir, validator = filepath.Join(root, "mesh-host"), merge("mesh-host", host)+"'s validator" + default: + validator = "the validator " + merge("mesh-controller", controller) + " vendors" + } + default: + return "", "", false, nil + } + for _, sib := range judged.Siblings { + inputs = append(inputs, "base "+sib.Repository+" "+sib.BaseCommit+" head "+sib.Head) + } + // A merge made for a check is a new commit each time (its time is in it); what it holds is the same for the same + // heads and bases, so the key names those and the trees the judge is built from. + inputs = append(inputs, "member "+judged.Head+" onto "+judged.Trunk, "controller tree "+treeAt(ctx, run, source)) + if validatorDir != "" { + inputs = append(inputs, "validator tree "+treeAt(ctx, run, validatorDir)) + } + 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 + 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 + } + src := filepath.Join(root, "judge-src") + defer os.RemoveAll(src) + if err := copyCheckout(ctx, run, source, src); err != nil { + return "", "", true, fmt.Errorf("the controller that judges (%s) could not be copied: %w", from, err) + } + if validatorDir != "" { + vendored := filepath.Join(src, "vendor", "github.com", "novox", "mesh-host") + for _, pkg := range []string{"validate", filepath.Join("internal", "declaration")} { + if err := replaceGoFiles(filepath.Join(validatorDir, pkg), filepath.Join(vendored, pkg)); err != nil { + return "", "the validator (" + validator + ") could not be put in the judge: " + err.Error(), true, nil + } + } + } + if _, err := labelled(ctx, root, "docker", inToolchain(src, nil, + "go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil { + return "", "the controller that judges (" + from + ") does not build with " + validator + ": " + firstLine(err.Error()), + true, nil + } + keepJudge(keptJudges(spec.JudgeCache), key, bin, say) + say("check", "judged by %s, with %s", from, validator) + return bin, "", true, nil +} + +// treeAt is the tree object a checkout's commit holds: the content a merge made for a check has, whatever time it was +// made at. +func treeAt(ctx context.Context, run Runner, dir string) string { + out, err := run(ctx, dir, "git", "rev-parse", "--verify", "--quiet", "HEAD^{tree}") + if err != nil { + return "" + } + return strings.TrimSpace(out) +} diff --git a/internal/builder/siblings_test.go b/internal/builder/siblings_test.go new file mode 100644 index 00000000..98dc8951 --- /dev/null +++ b/internal/builder/siblings_test.go @@ -0,0 +1,334 @@ +package builder + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// **A delivery group's member is checked beside the other members' heads** (novox/hq ADR 0382, issue 867): a +// mesh-host change and a mesh-controller change that need each other. The sibling's repository is read beside the +// check at the sibling's head merged onto its base head, in place of the commit the mesh runs. + +// aSiblingWorld is the mesh's two core repositories as the trunk holds them, and a pull request into each that the +// other needs: the controller's adds a lock line the node-engine's own check reads, and builds only with the +// node-engine's new validator word. +type aSiblingWorld struct { + controller, host string // repositories + controllerHead, hostHead string // the pull requests' whole heads + controllerMain, hostMain string // the trunks' heads +} + +func theSiblingWorld(t *testing.T) aSiblingWorld { + t.Helper() + var w aSiblingWorld + // The controller's trunk: a judge saying who it is, its lock without the new line, and the check that judges it. + controllerFiles := aJudgeOf("the trunk's controller") + controllerFiles["lock.txt"] = "foundation: old\n" + controllerFiles["module.json"] = `{"module":"mesh-controller"}` + controllerFiles[CheckScript] = "true\n" + var controllerRelease string + w.controller, w.controllerMain, controllerRelease = aRepositoryWithARelease(t, controllerFiles, map[string]string{"r": "r"}) + _ = controllerRelease + gitInRepo(t, w.controller, "checkout", "--quiet", "-b", "change", "main") + change := aJudgeOf("the sibling controller") + change["lock.txt"] = "foundation: new\n" + w.controllerHead = commitFiles(t, w.controller, change) + gitInRepo(t, w.controller, "checkout", "--quiet", "main") + // The node-engine's trunk: a validator, and the check that reads the controller's lock beside it. + hostFiles := aHostSaying("the trunk's validator") + hostFiles["module.json"] = `{"module":"mesh-host"}` + hostFiles[CheckScript] = "set -e\ngrep -q 'foundation: new' \"$MESH_CHECK_BESIDE/mesh-controller/lock.txt\"\n" + w.host, w.hostMain, _ = aRepositoryWithARelease(t, hostFiles, map[string]string{"r": "r"}) + gitInRepo(t, w.host, "checkout", "--quiet", "-b", "change", "main") + w.hostHead = commitFiles(t, w.host, aHostSaying("the sibling's validator")) + gitInRepo(t, w.host, "checkout", "--quiet", "main") + return w +} + +func (w aSiblingWorld) beside() map[string]Beside { + return map[string]Beside{"mesh-controller": {Repository: w.controller, Ref: w.controllerMain}, + "mesh-host": {Repository: w.host, Ref: w.hostMain}} +} + +// hostCheck is the node-engine's pull request checked, with or without its sibling. +func (w aSiblingWorld) hostCheck(t *testing.T, id, workspace, cache string, siblings ...Sibling) CheckVerdict { + t.Helper() + if workspace == "" { + workspace = t.TempDir() + } + beside := w.beside() + delete(beside, "mesh-host") + v, err := Check(t.Context(), Command, CheckSpec{ID: id, Repository: w.host, Ref: w.hostHead, Owner: "novox", + Repo: "mesh-host", Number: 114, Base: "main", Judge: "validator", JudgeCache: cache, + Paths: []string{"validate/v.go"}, Modules: []string{"mesh-host"}, Beside: beside, Siblings: siblings, + Toolchain: "golang"}, workspace, aFactsRegistry(t), GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + return v +} + +// controllerCheck is the controller's pull request checked, with or without its sibling. +func (w aSiblingWorld) controllerCheck(t *testing.T, id, cache string, siblings ...Sibling) CheckVerdict { + t.Helper() + beside := w.beside() + delete(beside, "mesh-controller") + v, err := Check(t.Context(), Command, CheckSpec{ID: id, Repository: w.controller, Ref: w.controllerHead, Owner: "novox", + Repo: "mesh-controller", Number: 346, Base: "main", Judge: "self", JudgeCache: cache, + Paths: []string{"lock.txt"}, Modules: []string{"mesh-controller"}, Beside: beside, Siblings: siblings, + Toolchain: "golang"}, t.TempDir(), aFactsRegistry(t), GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + return v +} + +func (w aSiblingWorld) controllerSibling() Sibling { + return Sibling{Repository: "novox/mesh-controller", Number: 346, Head: w.controllerHead, Base: "main", Dir: "mesh-controller"} +} + +func (w aSiblingWorld) hostSibling() Sibling { + return Sibling{Repository: "novox/mesh-host", Number: 114, Head: w.hostHead, Base: "main", Dir: "mesh-host"} +} + +// The node-engine's change fails its own check without its sibling (the controller's lock beside it is the trunk's), +// and passes with it read at its merge: the check reads the lock the sibling's change brings, the judge is the +// controller built from the sibling's merge with the node-engine change's own validator, and the verdict names both. +func TestAHostMemberIsCheckedBesideItsControllerSibling(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + w := theSiblingWorld(t) + + alone := w.hostCheck(t, "check-867-host-alone", "", "") + if alone.Repo == nil || alone.Repo.Verdict != "fail" || len(alone.Siblings) != 0 { + t.Fatalf("with no sibling the node-engine change should fail its own check (the lock beside it is the trunk's): %+v", + alone.Repo) + } + + v := w.hostCheck(t, "check-867-host", "", "", w.controllerSibling()) + if v.Repo == nil || v.Repo.Verdict != "pass" { + t.Fatalf("beside its sibling the node-engine change should pass its own check: %+v\n%s", v.Repo, v.Report) + } + if v.Gate == nil || v.Gate.Verdict != "pass" || !strings.Contains(v.Gate.Summary, "the sibling controller with the sibling's validator") { + t.Fatalf("the judge was not the controller built from the sibling's merge with the change's own validator: %+v\n%s", + v.Gate, v.Report) + } + baseCommit := w.controllerMain + if len(v.Siblings) != 1 || v.Siblings[0] != (JudgedSibling{Repository: "novox/mesh-controller", Number: 346, + Head: w.controllerHead, Base: "main", BaseCommit: baseCommit}) { + t.Fatalf("the verdict names siblings %+v, not mesh-controller#346 at %s onto %s", v.Siblings, w.controllerHead, baseCommit) + } + if v.Beside["mesh-controller"] == baseCommit[:12] || v.Beside["mesh-controller"] == w.controllerHead[:12] { + t.Fatalf("the controller was read at %s, which is the trunk's or the head and not their merge", v.Beside["mesh-controller"]) + } + if want := "sibling novox/mesh-controller#346 merge " + v.Beside["mesh-controller"]; v.JudgeFrom != want { + t.Fatalf("the judge came from %q, not %q", v.JudgeFrom, want) + } + if v.Validator != "the change's own validator" { + t.Fatalf("the validator was %q", v.Validator) + } + // The statuses' "beside siblings" part is the forge module's to write, from the verdict's siblings. + if strings.Contains(v.Gate.Summary, "beside siblings") || strings.Contains(v.Repo.Summary, "beside siblings") { + t.Fatalf("the check writes the siblings mark itself:\n%s\n%s", v.Gate.Summary, v.Repo.Summary) + } + // The base of the sibling is the trunk of the sibling's repository, as the trunk stood when the check cloned it. + if v.Trunk != w.hostMain[:12] { + t.Fatalf("the member's own trunk commit is %s, not %s", v.Trunk, w.hostMain[:12]) + } +} + +// The controller's change fails with no sibling (it builds only with the node-engine change's validator), and passes with +// it: judged by itself, with the sibling node-engine's validator in place of the one it vendors. +func TestAControllerMemberIsJudgedByItselfWithItsHostSiblingsValidator(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + w := theSiblingWorld(t) + // The controller change calls a function only the node-engine change brings. + gitInRepo(t, w.controller, "checkout", "--quiet", "change") + main := aJudgeOf("the sibling controller")["cmd/mesh-controller/main.go"] + w.controllerHead = commitFiles(t, w.controller, map[string]string{ + "cmd/mesh-controller/main.go": strings.Replace(main, "validate.Word)", "validate.Word+validate.More)", 1)}) + gitInRepo(t, w.controller, "checkout", "--quiet", "main") + gitInRepo(t, w.host, "checkout", "--quiet", "change") + w.hostHead = commitFiles(t, w.host, map[string]string{ + "validate/more.go": "package validate\n\nconst More = \" and more\"\n"}) + gitInRepo(t, w.host, "checkout", "--quiet", "main") + + alone := w.controllerCheck(t, "check-867-controller-alone", "") + if alone.Gate == nil || alone.Gate.Verdict != "fail" || !strings.Contains(alone.Gate.Summary, "does not build") { + t.Fatalf("with no sibling the controller change should not build (the validator it vendors lacks More): %+v", alone.Gate) + } + + v := w.controllerCheck(t, "check-867-controller", "", w.hostSibling()) + if v.Gate == nil || v.Gate.Verdict != "pass" || !strings.Contains(v.Gate.Summary, "the sibling controller with the sibling's validator and more") { + t.Fatalf("the controller change was not judged by itself with the sibling's validator: %+v\n%s", v.Gate, v.Report) + } + if len(v.Siblings) != 1 || v.Siblings[0].Repository != "novox/mesh-host" || v.Siblings[0].BaseCommit != w.hostMain { + t.Fatalf("the verdict names siblings %+v", v.Siblings) + } + if want := "sibling novox/mesh-host#114 merge " + v.Beside["mesh-host"] + "'s validator"; v.Validator != want { + t.Fatalf("the validator was %q, not %q", v.Validator, want) + } + if !strings.HasPrefix(v.JudgeFrom, "this change's own merge ") { + t.Fatalf("the judge came from %q", v.JudgeFrom) + } + // The member's own tree keeps the validator it vendors: its own script reads the vendored files as they are. + if body := gitInRepo(t, w.controller, "show", "main:vendor/github.com/novox/mesh-host/validate/v.go"); strings.Contains(body, "sibling") { + t.Fatalf("the trunk's vendored validator was touched: %s", body) + } +} + +// A sibling head that does not merge cleanly onto its base fails the check, naming the sibling and the files. +func TestASiblingThatDoesNotMergeCleanlyFailsTheCheckNamingIt(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + w := theSiblingWorld(t) + // The base moves under the sibling's head on the same line of the same file. + gitInRepo(t, w.controller, "checkout", "--quiet", "main") + commitFiles(t, w.controller, map[string]string{"lock.txt": "foundation: elsewhere\n"}) + v := w.hostCheck(t, "check-867-conflict", "", "", w.controllerSibling()) + if v.Verdict != "fail" || v.Gate == nil || v.Gate.Verdict != "fail" || v.Repo == nil || v.Repo.Verdict != "fail" { + t.Fatalf("a sibling that does not merge was answered %+v / %+v", v.Gate, v.Repo) + } + for _, want := range []string{"novox/mesh-controller#346", "does not merge cleanly", "lock.txt", w.controllerHead[:12]} { + if !strings.Contains(v.Summary, want) { + t.Fatalf("the failure does not say %q: %s", want, v.Summary) + } + } + if len(v.Siblings) != 1 || v.Siblings[0].Number != 346 { + t.Fatalf("the verdict does not name the sibling: %+v", v.Siblings) + } +} + +// A check that could not learn its siblings reads none and says so; one with none to read is judged as before. +func TestACheckThatCouldNotLearnItsSiblingsSaysSo(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + w := theSiblingWorld(t) + beside := w.beside() + delete(beside, "mesh-host") + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-867-unknown", Repository: w.host, Ref: w.hostHead, + Owner: "novox", Repo: "mesh-host", Number: 114, Base: "main", Judge: "validator", Paths: []string{"validate/v.go"}, + Modules: []string{"mesh-host"}, Beside: beside, SiblingsUnknown: "mesh-delivery did not answer", + Toolchain: "golang"}, t.TempDir(), aFactsRegistry(t), GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.SiblingsUnknown != "mesh-delivery did not answer" || len(v.Siblings) != 0 || v.JudgeFrom != "" { + t.Fatalf("the verdict says %q, siblings %+v, judge from %q", v.SiblingsUnknown, v.Siblings, v.JudgeFrom) + } + if !strings.Contains(v.Gate.Summary, "the trunk's controller with the sibling's validator") { + t.Fatalf("a check without siblings was not judged as before: %+v", v.Gate) + } + if strings.Contains(v.Gate.Summary, "[beside siblings") { + t.Fatalf("the statuses name siblings that were not read: %s", v.Gate.Summary) + } +} + +// The judge a group made is kept only for the same inputs: the controller's merge and the base commits it was merged +// onto, the validator's source and commit. The same inputs take the judge kept; any other builds it again. +func TestTheGroupsJudgeIsReusedOnlyForTheSameInputs(t *testing.T) { + built := aRuntimeThatBuildsTheJudge(t) + w := theSiblingWorld(t) + cache := t.TempDir() + w.hostCheck(t, "check-867-cache-1", "", cache, w.controllerSibling()) + if n := builds(t, built); n != 1 { + t.Fatalf("the first check built %d judge(s)", n) + } + v := w.hostCheck(t, "check-867-cache-2", "", cache, w.controllerSibling()) + if n := builds(t, built); n != 1 || v.Gate.Verdict != "pass" { + t.Fatalf("the same inputs built the judge again (%d builds) or did not pass: %+v", n, v.Gate) + } + // The controller sibling's head moves: another merge, another judge. + gitInRepo(t, w.controller, "checkout", "--quiet", "change") + w.controllerHead = commitFiles(t, w.controller, map[string]string{"extra.txt": "x"}) + gitInRepo(t, w.controller, "checkout", "--quiet", "main") + w.hostCheck(t, "check-867-cache-3", "", cache, w.controllerSibling()) + if n := builds(t, built); n != 2 { + t.Fatalf("a moved sibling head did not build the judge again: %d builds", n) + } + // The controller's base moves: the merge differs again. + commitFiles(t, w.controller, map[string]string{"base.txt": "moved"}) + w.hostCheck(t, "check-867-cache-4", "", cache, w.controllerSibling()) + if n := builds(t, built); n != 3 { + t.Fatalf("a moved base did not build the judge again: %d builds", n) + } + // The validator's source moves: the member's own change differs. + gitInRepo(t, w.host, "checkout", "--quiet", "change") + w.hostHead = commitFiles(t, w.host, aHostSaying("another validator")) + gitInRepo(t, w.host, "checkout", "--quiet", "main") + w.hostCheck(t, "check-867-cache-5", "", cache, w.controllerSibling()) + if n := builds(t, built); n != 4 { + t.Fatalf("a moved validator did not build the judge again: %d builds", n) + } +} + +// The contrast the isolation test stands on: built the old way, in the workspace with its HOME and no judge-build +// environment, the same planted Go environment file takes effect in the group's judge build (its toolexec hook runs in +// the judge's sources). Without this, the test below could not fail. +func TestWithThePlainEnvironmentThePlantedWorkspaceReachesTheGroupsJudgeBuild(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + was := judgeBuildEnv + judgeBuildEnv = func(string, string) []string { return nil } + t.Cleanup(func() { judgeBuildEnv = was }) + w := theSiblingWorld(t) + workspace, ran := aPlantedWorkspace(t) + w.hostCheck(t, "check-867-plain", workspace, "", w.controllerSibling()) + if body, _ := readFile(ran); !strings.Contains(body, "/judge-src") { + t.Fatalf("with the plain environment the planted hook did not run in the judge's build, so the isolation test proves nothing: %q", body) + } +} + +// **The group's judge is unreviewed code, built where nothing the change's script does can reach it**: a git hook, a +// Go environment file and a Go cache planted in the workspace, and a script writing into its home, change nothing in the +// judge; the judge cache is mounted into judge builds alone. +func TestTheGroupsJudgeIsBuiltWhereNothingTheChangeDoesReachesIt(t *testing.T) { + built := aRuntimeThatBuildsTheJudge(t) + homes := filepath.Join(t.TempDir(), "homes") + t.Setenv("JUDGE_HOMES", homes) + runs := filepath.Join(t.TempDir(), "runs") + t.Setenv("RUNS", runs) + w := theSiblingWorld(t) + // The member's own script (the base branch's) touches git in its tree and writes into its home. + gitInRepo(t, w.host, "checkout", "--quiet", "main") + commitFiles(t, w.host, map[string]string{CheckScript: "set -e\ngit status >/dev/null 2>&1 || true\n" + + "echo planted > \"$HOME/.gitconfig\"\ngrep -q 'foundation: new' \"$MESH_CHECK_BESIDE/mesh-controller/lock.txt\"\n"}) + w.hostMain = gitInRepo(t, w.host, "rev-parse", "main") + gitInRepo(t, w.host, "checkout", "--quiet", "change") + gitInRepo(t, w.host, "rebase", "--quiet", "main") + w.hostHead = gitInRepo(t, w.host, "rev-parse", "HEAD") + gitInRepo(t, w.host, "checkout", "--quiet", "main") + workspace, ran := aPlantedWorkspace(t) + cache := t.TempDir() + v := w.hostCheck(t, "check-867-planted", workspace, cache, w.controllerSibling()) + if v.Gate == nil || v.Gate.Verdict != "pass" || v.Repo == nil || v.Repo.Verdict != "pass" { + t.Fatalf("the check did not pass: %+v %+v\n%s", v.Gate, v.Repo, v.Report) + } + if body, _ := readFile(ran); !strings.Contains(body, "/check/mesh-host") { + t.Fatalf("the planted hook never ran in the member's tree, so this test proves nothing: %q", body) + } + if body, _ := readFile(ran); strings.Contains(body, "/judge-src") || judgeRanTheHook(t, ran) != "" { + t.Fatalf("the hook planted in the workspace ran during the group's judge:\n%s", body) + } + if body, _ := readFile(homes); len(body) == 0 || strings.Contains(body, workspace) { + t.Fatalf("the judge was built with a HOME in the workspace: %q", body) + } + said, _ := readFile(built) + if !strings.Contains(said, "GOCACHE="+filepath.Join(cache, "go-cache")+" GOENV=off go build") { + t.Fatalf("the judge was not built with the judge cache's Go cache and no Go environment file:\n%s", said) + } + // Only a judge build mounts the judge cache. + body, _ := readFile(runs) + for _, line := range strings.Split(strings.TrimSpace(body), "\n") { + if !strings.Contains(line, " go build ") && strings.Contains(line, cache) { + t.Fatalf("a container the check runs for the pull request names the judge cache: %s", line) + } + } + // The sources the judge was built from are not left where the member's script could have written them. + if _, err := os.Stat(filepath.Join(workspace, "check", "judge-src")); err == nil { + t.Fatal("the judge's sources were left in the workspace") + } +} + +func readFile(path string) (string, error) { + body, err := os.ReadFile(path) + return string(body), err +} diff --git a/internal/link/build.go b/internal/link/build.go index e881889a..99597e2a 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -173,6 +173,13 @@ type CheckRequest struct { // node-engine with its own validator — and each repository beside it is read at its branch of that name where it // has one (CheckedOut.Else otherwise). Empty for every other base, checked as before it. Release string `json:"release,omitempty"` + // Siblings are the other open pull requests of this one's delivery group the check reads beside it (novox/hq ADR + // 0382 §1): each repository found under Dir is read at the sibling's head merged onto the sibling's base, in + // place of the commit the mesh runs. SiblingsUnknown says mesh-delivery could not be asked, or answered without + // what a sibling is found by, and the check reads none. Both empty when mesh-delivery answered and named none: + // the request is then what it was before the rule. + Siblings []CheckSibling `json:"siblings,omitempty"` + SiblingsUnknown string `json:"siblings_unknown,omitempty"` // Group names a delivery group whose members' heads are composed together with this one (novox/hq ADR // 0239), and Members are those other heads, each cloned beside it and laid over the mesh in turn. The // repository's own check is not run for a group: each member's pull request runs its own. @@ -250,6 +257,19 @@ type CheckBuilt struct { SameAs map[string]string `json:"same-as,omitempty"` } +// CheckSibling is one other open pull request of a delivery group (novox/hq ADR 0382): in a request, the repository it +// is in as owner/repo, the directory the check finds that repository under (Dir), its number, its whole head and its +// base branch; in a verdict, what the check read of it — Head, Base and BaseCommit, whole: the base's head the sibling's +// head was merged onto. +type CheckSibling struct { + Repository string `json:"repository"` + Number int `json:"number"` + Head string `json:"head"` + Base string `json:"base"` + BaseCommit string `json:"base_commit,omitempty"` + Dir string `json:"dir,omitempty"` +} + // GroupMember is one other head of a delivery group's composed check. type GroupMember struct { Owner string `json:"owner"` @@ -309,6 +329,12 @@ type CheckOutcome struct { BesideAt map[string]string `json:"beside-at,omitempty"` // Validator says which node-engine validator the judge of a release-branch check was built with (ADR 0358 §2). Validator string `json:"validator,omitempty"` + // Siblings are the pull requests of its delivery group it read beside it, each as its head merged onto its base + // commit (ADR 0382 §4), SiblingsUnknown why it could not learn them, and JudgeFrom where the judge came from when + // the group made it (§3), such as "sibling novox/mesh-controller#346 merge 0123456789ab". + Siblings []CheckSibling `json:"siblings,omitempty"` + SiblingsUnknown string `json:"siblings_unknown,omitempty"` + JudgeFrom string `json:"judge_from,omitempty"` } // SourceOnSeat names a repository by the seat whose holder serves it and its path there. diff --git a/internal/link/events.go b/internal/link/events.go index 83ba26c6..a1e8aa6e 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -301,6 +301,13 @@ type Checked struct { BesideAt map[string]string `json:"beside-at,omitempty"` // Validator says which node-engine validator the judge of a release-branch check was built with (ADR 0358 §2). Validator string `json:"validator,omitempty"` + // Siblings are the other pull requests of the delivery group the check read beside it (novox/hq ADR 0382 §4), each + // with its head and the base commit it was merged onto; SiblingsUnknown is why it could not learn them, set only + // then; JudgeFrom says where the judge came from when the group made it. The forge's holder keeps Siblings per + // open pull request (§5). All omitted for a check that read none. + Siblings []CheckSibling `json:"siblings,omitempty"` + SiblingsUnknown string `json:"siblings_unknown,omitempty"` + JudgeFrom string `json:"judge_from,omitempty"` // Built are the builds the check made for its merge, each with its reuse key and digests, or why not // (novox/hq ADR 0350 §1): what the walk of the merge takes instead of building. Built []CheckBuilt `json:"built,omitempty"` diff --git a/testdata/store-tests b/testdata/store-tests index 8f060418..83f5dc5f 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -79,3 +79,4 @@ ./cmd/mesh-controller TestARecordedModulesMoveThatTouchesDataOrAProviderWaits ./cmd/mesh-controller TestAPersonsRecordAndTheBusHoldEveryMove ./cmd/mesh-controller TestARecordedMoveThatCannotBeJudgedWaits +./cmd/mesh-controller TestReplay867