From 151faa10a13b5322512eb87d32d889f6b332b2ab Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Fri, 16 Oct 2026 00:59:27 +0000 Subject: [PATCH] =?UTF-8?q?Judge=20a=20delivery=20group's=20composed=20che?= =?UTF-8?q?ck=20by=20the=20group's=20own=20controller,=20release=20branch?= =?UTF-8?q?=20included=20(issue=20867,=20ADR=200239=20=C2=A714)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The composed check of mesh-controller#346 and mesh-host#114 into feat/57-config-and-provisioning was judged by the release branch's controller, which cannot build the node-engine member's validator, so the group could never pass. ADR 0239 §14 has the group's own controller judge when a member moves it; ADR 0358 §2 changes the judge only for a pull request into a release branch. A group member in mesh-controller now judges, with the mesh-host member's validator, built in the isolated judge environment and kept by content; the statuses say the group's controller judged. Co-Authored-By: Claude --- cmd/mesh-controller/replay867_test.go | 135 ++++++++++++++++++++++++-- internal/builder/check.go | 57 +++++++++-- testdata/store-tests | 2 + 3 files changed, 181 insertions(+), 13 deletions(-) diff --git a/cmd/mesh-controller/replay867_test.go b/cmd/mesh-controller/replay867_test.go index ac6b2add..5e8d67db 100644 --- a/cmd/mesh-controller/replay867_test.go +++ b/cmd/mesh-controller/replay867_test.go @@ -67,6 +67,7 @@ type theIssue867 struct { js interface{ Conn() *nats.Conn } judgeCache string registry string + hostRepo, controllerRepo string groups string // what mesh-delivery's groups answers } @@ -125,12 +126,17 @@ func main() { } } -func theGroupOfIssue867(t *testing.T) *theIssue867 { +func theGroupOfIssue867(t *testing.T) *theIssue867 { return theGroupOfIssue867Into(t, "main") } + +// theGroupOfIssue867Into is the group with both pull requests into base: main, or a release branch both repositories +// hold, cut from their trunks. +func theGroupOfIssue867Into(t *testing.T, base string) *theIssue867 { t.Helper() open := aMesh(t) r := &theIssue867{open: open, judgeCache: t.TempDir()} root := filepath.Join(t.TempDir(), "novox") controller, host := filepath.Join(root, "mesh-controller"), filepath.Join(root, "mesh-host") + r.hostRepo, r.controllerRepo = host, controller for _, d := range []string{controller, host} { if err := os.MkdirAll(d, 0o755); err != nil { t.Fatal(err) @@ -146,21 +152,36 @@ func theGroupOfIssue867(t *testing.T) *theIssue867 { r867Commit(t, host, map[string]string{"validate/v.go": "package validate\n\nconst Word = \"the trunk's validator\"\n", "internal/declaration/d.go": "package declaration\n", "module.json": `{"module":"mesh-host"}`, builder.CheckScript: "set -e\ngrep -q 'foundation: new' \"$MESH_CHECK_BESIDE/mesh-controller/lock.txt\"\n"}) + if base != "main" { + for _, d := range []string{controller, host} { + r867Git(t, d, "branch", base, "main") + } + } // The two pull requests of one group, feat/867. - r867Git(t, controller, "checkout", "--quiet", "-b", "feat/867", "main") + r867Git(t, controller, "checkout", "--quiet", "-b", "feat/867", base) change := r867Controller("the controller", "+validate.More") change["lock.txt"] = "foundation: new\n" r.controllerHead = r867Commit(t, controller, change) r867Git(t, controller, "checkout", "--quiet", "main") - r867Git(t, host, "checkout", "--quiet", "-b", "feat/867", "main") + r867Git(t, host, "checkout", "--quiet", "-b", "feat/867", base) r.hostHead = r867Commit(t, host, map[string]string{"validate/more.go": "package validate\n\nconst More = \" and more\"\n"}) r867Git(t, host, "checkout", "--quiet", "main") + followed := func(string) string { return "main" } for _, m := range []struct{ module, dir string }{{"mesh-controller", controller}, {"mesh-host", host}} { // The mesh runs each at its trunk's head, the commit a check reads it at beside another repository's change. running := r867Git(t, m.dir, "rev-parse", "main") if err := open.inventory.RegisterModule(t.Context(), catalogue.Manifest{Module: m.module, Version: "1"}, - inventory.Source{Repository: m.dir, Ref: "main", BuiltFrom: running, Head: running}); err != nil { + inventory.Source{Repository: m.dir, Ref: followed(m.module), BuiltFrom: running, Head: running}); err != nil { + t.Fatal(err) + } + } + if base != "main" { + // A module built from the controller's repository follows the release branch, as the mesh runs it there: a pull + // request into the release branch moves it, so the composed check's gate runs. + running := r867Git(t, controller, "rev-parse", "main") + if err := open.inventory.RegisterModule(t.Context(), catalogue.Manifest{Module: "zz-agent", Version: "1"}, + inventory.Source{Repository: controller, Ref: base, BuiltFrom: running, Head: running}); err != nil { t.Fatal(err) } } @@ -178,9 +199,9 @@ func theGroupOfIssue867(t *testing.T) *theIssue867 { r.groups = (`[{"id":"feat/867","members":["` + root + `/mesh-host@` + r.hostHead[:12] + ` (checked)","` + root + `/mesh-controller@` + r.controllerHead[:12] + ` (checked)"],"member_details":[` + `{"id":"novox/mesh-host@` + r.hostHead[:12] + `","repository":"novox/mesh-host","state":"checked","number":114,"head":"` + - r.hostHead + `","base":"main","open":true},` + + r.hostHead + `","base":"` + base + `","open":true},` + `{"id":"novox/mesh-controller@` + r.controllerHead[:12] + `","repository":"novox/mesh-controller","state":"checked","number":346,"head":"` + - r.controllerHead + `","base":"main","open":true}]}]`) + r.controllerHead + `","base":"` + base + `","open":true}]}]`) r.registry = r867Registry(t) r867Runtime(t) return r @@ -344,3 +365,105 @@ func TestTheRecheckVerbAsksMeshDeliveryOnABusOfItsOwn(t *testing.T) { t.Fatalf("mesh-delivery was asked %d times", asked) } } + +// **A delivery group's composed check is judged by the group's own controller** (novox/hq ADR 0239 §14), into a release +// branch too: the controller member and the mesh-host member of the pair are consistent only together (the controller +// calls what only the node-engine change brings), and both merge into a release branch. The release judge (the +// controller of the release branch with the validator of its own) cannot build it; the group's own controller, as +// merged, with the node-engine member's validator, can. It drives askGroupCheck and the build seat's Check for real. +func TestAGroupsCompositionIntoAReleaseBranchIsJudgedByItsOwnController(t *testing.T) { + groupIntoARelease(t, false) +} + +// The shape the live check had: the node-engine member is the group's primary, so the composed request is asked with +// Judge "validator" (a union of modules that includes mesh-host's) and the release branch set; the release judge then +// put this change's validator into the old controller and "judge-with-this-validator" did not build. +func TestAGroupsCompositionJudgedWithTheValidatorIntoAReleaseBranchIsJudgedByItsOwnController(t *testing.T) { + groupIntoARelease(t, true) +} + +func groupIntoARelease(t *testing.T, hostPrimary bool) { + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no Go toolchain on this machine to build a judge with") + } + const release = "feat/r" + r := theGroupOfIssue867Into(t, release) + aForgeWithBranches(t, map[string][]string{"novox/mesh-controller": {release}}, nil) + // The pair renames what the validator says: the node-engine change drops Word for Word2, the controller change + // calls Word2. The controller of the release branch calls Word, so it does not build with the new validator; the + // controller change does not build with the validator it vendors. Only together are they consistent. + r867Git(t, r.hostRepo, "checkout", "--quiet", "-b", "swap", release) + r.hostHead = r867Commit(t, r.hostRepo, map[string]string{"validate/v.go": "package validate\n\nconst Word2 = \"the new validator\"\n"}) + r867Git(t, r.hostRepo, "checkout", "--quiet", "main") + r867Git(t, r.controllerRepo, "checkout", "--quiet", "-b", "swap", release) + swapped := strings.Replace(r867Controller("the controller", "")["cmd/mesh-controller/main.go"], "validate.Word)", "validate.Word2)", 1) + r.controllerHead = r867Commit(t, r.controllerRepo, map[string]string{"cmd/mesh-controller/main.go": swapped}) + r867Git(t, r.controllerRepo, "checkout", "--quiet", "main") + js, _ := aBuildQueue(t) + servingBus.Store(js) + t.Cleanup(func() { servingBus.Store(nil) }) + members := []orderMember{ + {ID: "novox/mesh-controller@" + r.controllerHead[:12], Repository: "novox/mesh-controller", Base: release, + Head: r.controllerHead, Number: 346, Paths: []string{"lock.txt", "cmd/mesh-controller/main.go"}, ModuleDirsSaid: true}, + {ID: "novox/mesh-host@" + r.hostHead[:12], Repository: "novox/mesh-host", Base: release, Head: r.hostHead, Number: 114, + Paths: []string{"validate/v.go"}, ModuleDirsSaid: true}} + if hostPrimary { + members[0], members[1] = members[1], members[0] + } + if _, err := askGroupCheck(t.Context(), r.open, "feat/867", members); err != nil { + t.Fatal(err) + } + q, err := link.ReadQueue(t.Context(), js, link.TheBuildMachine) + if err != nil || len(q.Asks) == 0 { + t.Fatalf("nothing was asked of the build seat: %v", err) + } + request := q.Asks[len(q.Asks)-1].Request + if request.Check.Release != release || request.Check.Group != "feat/867" || len(request.Check.Members) != 1 || len(request.Check.Modules) == 0 { + t.Fatalf("the composed check is asked as release %q, group %q, %d other head(s)", request.Check.Release, + request.Check.Group, len(request.Check.Members)) + } + if hostPrimary { + if request.Check.Repo != "mesh-host" { + t.Fatalf("the primary is %s, not the node-engine", request.Check.Repo) + } + // As the live ask had it (the mesh-host module among the union's): the node-engine's validator judges. + request.Check.Judge = link.JudgeValidator + } + // What the build seat makes of the ask. + var wire map[string]json.RawMessage + body, _ := json.Marshal(request.Check) + _ = json.Unmarshal(body, &wire) + delete(wire, "group") + delete(wire, "members") + body, _ = json.Marshal(wire) + var spec builder.CheckSpec + if err := json.Unmarshal(body, &spec); err != nil { + t.Fatal(err) + } + for _, m := range request.Check.Members { + spec.Group = append(spec.Group, builder.GroupHead{Owner: m.Owner, Repo: m.Repo, Repository: m.Repository, Ref: m.Ref, + Paths: m.Paths, Base: m.Base}) + } + spec.ID, spec.Repository, spec.Ref = request.ID, request.Repository, request.Ref + spec.Toolchain, spec.JudgeCache = "golang", r.judgeCache + v, err := builder.Check(t.Context(), builder.Command, spec, t.TempDir(), r.registry, builder.GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Verdict != "pass" { + t.Fatalf("the group's composition into %s was not judged by its own controller: %+v\n%s", release, v.Gate, v.Report) + } + if mark := "[release " + release + ", judged by the group's controller " + v.JudgedBy + "] "; !strings.HasPrefix(v.Gate.Summary, mark) || + !strings.Contains(v.Report, "by the group's controller at "+v.JudgedBy) { + t.Fatalf("the statuses and the report do not say the group's controller judged:\n%s\n%s", v.Gate.Summary, v.Report) + } + if !strings.Contains(v.Gate.Summary, "the new validator") { + t.Fatalf("the gate was judged by %q, not by the group's controller with the node-engine member's validator", v.Gate.Summary) + } + if !strings.HasPrefix(v.JudgeFrom, "the group's own controller") || !strings.Contains(v.Validator, "the group's mesh-host merge") { + t.Fatalf("the verdict does not say where the judge and the validator came from: %q / %q", v.JudgeFrom, v.Validator) + } + if v.Release != release || v.JudgedBy == "" { + t.Fatalf("the verdict names release %q judged by %q", v.Release, v.JudgedBy) + } +} diff --git a/internal/builder/check.go b/internal/builder/check.go index f6a5a7ff..3f272fcd 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -771,11 +771,14 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if spec.Judge == "self" { judgedBy = tree } + if siblingJudge.Dir != "" { + judgedBy = siblingJudge.Dir + } judged.JudgedBy = headAt(ctx, run, judgedBy) if siblingJudge.Validator == "" { judged.Validator = releaseValidator(spec, judged.BesideAt, judged.Beside) } - said := releaseWords(spec.Release, judged.JudgedBy, judged.Validator, judged.BesideAt, judged.Beside) + said := releaseWords(spec.Release, whoJudges(siblingJudge), judged.JudgedBy, judged.Validator, judged.BesideAt, judged.Beside) fmt.Fprintf(&out, "--- %s\n", said) say("check", "%s", said) } @@ -846,7 +849,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if spec.Release != "" { // The statuses name the release branch and the judge first, so the forge's short description keeps them. - mark := releaseMark(spec.Release, judged.JudgedBy) + mark := releaseMark(spec.Release, whoJudges(siblingJudge), judged.JudgedBy) v.Gate.Summary, v.Repo.Summary = mark+v.Gate.Summary, mark+v.Repo.Summary } // What the check printed travels in the verdict to the forge and the controller: redacted as the log is. @@ -1586,21 +1589,30 @@ func releaseValidator(spec CheckSpec, at, commits map[string]string) string { } // releaseWords is how a release-branch check says what judged it and what it read (novox/hq ADR 0358 §4). -func releaseWords(release, judgedBy, validator string, at, commits map[string]string) string { +func releaseWords(release, controller, judgedBy, validator string, at, commits map[string]string) string { var read []string for _, dir := range sortedKeys(at) { read = append(read, fmt.Sprintf("%s %s at %s", dir, refWords(at[dir]), commits[dir])) } - said := fmt.Sprintf("judged for the release branch %s by its controller at %s, with %s", release, judgedBy, validator) + said := fmt.Sprintf("judged for the release branch %s by %s at %s, with %s", release, controller, judgedBy, validator) if len(read) > 0 { said += "; read beside it: " + strings.Join(read, ", ") } return said } +// whoJudges says which controller judged: the release branch's own ("its controller"), or — when a delivery group's +// member in mesh-controller judged (novox/hq ADR 0239 §14) — the group's. +func whoJudges(j siblingJudgement) string { + if j.Dir != "" { + return "the group's controller" + } + return "its controller" +} + // releaseMark opens each layer's summary of a release-branch check, so a status says it first. -func releaseMark(release, judgedBy string) string { - return fmt.Sprintf("[release %s, judged by its controller %s] ", release, judgedBy) +func releaseMark(release, controller, judgedBy string) string { + return fmt.Sprintf("[release %s, judged by %s %s] ", release, controller, judgedBy) } // refWords is a ref as a line says it: a commit by its first eight characters, a branch whole. @@ -1949,6 +1961,22 @@ func lastLine(s string) string { // the controller it was built from and Validator the node-engine validator in it. Both empty for any other judge. type siblingJudgement struct { From, Validator string + // Dir is the checkout of the controller that judges. + Dir string +} + +// groupDir is the checkout of the group's member in a repository of that name — the checked one (tree) or one cloned +// beside it for the group — empty when no member is in it. +func groupDir(spec CheckSpec, root, tree, repo string) string { + if spec.Repo == repo { + return tree + } + for _, g := range spec.Group { + if g.Repo == repo && safeName.MatchString(g.Repo) { + return filepath.Join(root, "group-"+g.Repo) + } + } + return "" } // judgeFromTheGroup builds the judge of a member that has siblings (novox/hq ADR 0382 §3); done says it did (or failed @@ -1974,6 +2002,21 @@ func judgeFromTheGroup(ctx context.Context, labelled, run Runner, spec CheckSpec return fmt.Sprintf("sibling %s#%d merge %s", s.Repository, s.Number, judged.Beside[dir]) } switch { + case len(spec.Group) > 0 && groupDir(spec, root, tree, "mesh-controller") != "": + // **A delivery group's composed check is judged by the group's own controller** (novox/hq ADR 0239 §14), whatever + // base it merges into — a release branch included (ADR 0358 §2 changes the judge of one pull request, not of a + // group): the member that moves mesh-controller, as merged, with the validator of the group's mesh-host member's + // merge when it has one, else the one that controller vendors. + source, validatorDir = groupDir(spec, root, tree, "mesh-controller"), groupDir(spec, root, tree, "mesh-host") + from = "the group's own controller, merge " + headAt(ctx, run, source) + validator = "the validator the group's controller vendors" + if validatorDir != "" { + validator = "the group's mesh-host merge " + headAt(ctx, run, validatorDir) + "'s validator" + } + inputs = append(inputs, "group member "+spec.Repo+" "+judged.Head+" onto "+judged.Trunk) + for _, m := range judged.Members { + inputs = append(inputs, "group member "+m.Repo+" "+m.Head+" onto "+m.Trunk) + } case spec.Judge == "self" && host != nil: source, validatorDir = tree, filepath.Join(root, "mesh-host") from = "this change's own merge " + judged.Head @@ -2004,7 +2047,7 @@ func judgeFromTheGroup(ctx context.Context, labelled, run Runner, spec CheckSpec sum := sha256.Sum256([]byte(strings.Join(inputs, "\n"))) key := "siblings-" + hex.EncodeToString(sum[:16]) bin := filepath.Join(root, "bin", "judge-of-the-group") - said.From, said.Validator = from, validator + said.From, said.Validator, said.Dir = from, validator, source if judgeKept(keptJudges(spec.JudgeCache), key, bin) { say("check", "judged by %s, with %s, built for an earlier check of the same inputs", from, validator) return bin, "", true, nil diff --git a/testdata/store-tests b/testdata/store-tests index ebafe258..8464a067 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -82,3 +82,5 @@ ./cmd/mesh-controller TestReplay867 ./cmd/mesh-controller TestReplay867Recheck ./cmd/mesh-controller TestTheRecheckVerbAsksMeshDeliveryOnABusOfItsOwn +./cmd/mesh-controller TestAGroupsCompositionIntoAReleaseBranchIsJudgedByItsOwnController +./cmd/mesh-controller TestAGroupsCompositionJudgedWithTheValidatorIntoAReleaseBranchIsJudgedByItsOwnController -- 2.54.0