From 4a75cc51dda6e9ce6a14d423524309a6b73f6566 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 12:48:44 +0000 Subject: [PATCH 1/8] Judge a pull request into a release branch by that branch's controller and repositories A release branch could never carry a change that needs another repository's release-branch change: its pull requests were judged by the running controller and against the other repositories' main (hq issue 704, ADR 0358). Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-builder/main.go | 17 +- cmd/mesh-builder/release_test.go | 26 ++ cmd/mesh-controller/checks.go | 168 +++++++++-- cmd/mesh-controller/release_branch_test.go | 167 +++++++++++ internal/builder/check.go | 251 ++++++++++++++++- internal/builder/release_branch_test.go | 308 +++++++++++++++++++++ internal/link/build.go | 16 ++ internal/link/events.go | 6 + 8 files changed, 924 insertions(+), 35 deletions(-) create mode 100644 cmd/mesh-builder/release_test.go create mode 100644 cmd/mesh-controller/release_branch_test.go create mode 100644 internal/builder/release_branch_test.go diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index da49ce3b..32ec20cb 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -24,6 +24,7 @@ import ( "net/url" "os" "os/signal" + "path/filepath" "sort" "strings" "syscall" @@ -313,6 +314,9 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri registry = r.Address } spec := checkSpecOf(request) + // A release branch's judge is kept beside the workspace, never in it: the check's containers mount the + // workspace, and a pull request's script must not plant the judge of the next check (novox/hq ADR 0358 §2). + spec.JudgeCache = judgeCacheOf(workspace) // What the merge would move, built by the check for the walk to take (novox/hq ADR 0350 §1): into the // scratch namespace. **Never with the package registry's credential**: this is code nobody has approved // yet, and a recipe could read the credential it is handed (issue 462). A module that resolves packages @@ -326,7 +330,8 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri if err == nil { result.Check = &link.CheckOutcome{Verdict: v.Verdict, Summary: v.Summary, Report: v.Report, Took: v.Took.Round(time.Second).String(), Gate: layerOf(v.Gate), RepoCheck: layerOf(v.Repo), - Head: v.Head, Trunk: v.Trunk, Beside: v.Beside, Built: checkBuiltOf(v.Built)} + Head: v.Head, Trunk: v.Trunk, Beside: v.Beside, Built: checkBuiltOf(v.Built), + Release: v.Release, JudgedBy: v.JudgedBy, BesideAt: v.BesideAt} // 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, @@ -456,9 +461,9 @@ 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} + Tests: c.Tests, StoreOnly: c.StoreOnly, Release: c.Release} for dir, b := range c.Beside { - spec.Beside[dir] = builder.Beside{Repository: b.Repository, Ref: b.Ref} + spec.Beside[dir] = builder.Beside{Repository: b.Repository, Ref: b.Ref, Else: b.Else} } for _, b := range c.Builds { spec.Builds = append(spec.Builds, builder.CheckBuildSpec{Module: b.Module, Repository: b.Repository, Path: b.Path, @@ -471,6 +476,12 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { return spec } +// judgeCacheOf is where a release branch's judge is kept (novox/hq ADR 0358 §2): beside the workspace, outside what +// a check's containers mount. +func judgeCacheOf(workspace string) string { + return filepath.Clean(workspace) + "-judges" +} + // triedOf is a try's outcome as its log line says it (novox/hq issue 686): the repository's own check's verdict, // or the check's where that did not run — a head that does not merge — and the lines naming what failed. func triedOf(v builder.CheckVerdict) link.Tried { diff --git a/cmd/mesh-builder/release_test.go b/cmd/mesh-builder/release_test.go new file mode 100644 index 00000000..0b678d16 --- /dev/null +++ b/cmd/mesh-builder/release_test.go @@ -0,0 +1,26 @@ +package main + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/link" +) + +// **A pull request into a release branch reaches the check as one** (novox/hq ADR 0358): the build seat hands the +// check the release branch and, beside it, the trunk each repository is read at when it has no branch of that name; +// and it keeps the release branch's judge outside the workspace a check's containers mount. +func TestTheBuildSeatHandsTheCheckItsReleaseBranch(t *testing.T) { + spec := checkSpecOf(link.BuildRequest{ID: "b", Repository: "r", Ref: "abc", Check: &link.CheckRequest{ + Owner: "novox", Repo: "mesh-catalog", Base: "feat/57", Release: "feat/57", + Beside: map[string]link.CheckedOut{"mesh-host": {Repository: "h", Ref: "feat/57", Else: "main"}}}}) + if spec.Release != "feat/57" || spec.Beside["mesh-host"].Ref != "feat/57" || spec.Beside["mesh-host"].Else != "main" { + t.Fatalf("the check was handed release %q and mesh-host %+v", spec.Release, spec.Beside["mesh-host"]) + } + workspace := filepath.Join(t.TempDir(), "mesh-builder") + cache := judgeCacheOf(workspace) + if rel, err := filepath.Rel(workspace, cache); err != nil || !strings.HasPrefix(rel, "..") { + t.Fatalf("the judge cache %s lies inside the workspace %s, which a check's containers mount", cache, workspace) + } +} diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index fbae0304..3fa4a882 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "path" "regexp" @@ -248,30 +249,14 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop if err != nil { return link.BuildRequest{}, err } - // Beside it, at what the mesh runs: each core repository by the module the mesh builds from it. - beside := map[string]link.CheckedOut{} - for _, e := range entries { - dir, core := coreModules[e.Manifest.Module] - if !core || e.Provided || e.Source.Repository == "" { - continue - } - url, err := clone(e.Source) - if err != nil { - return link.BuildRequest{}, err - } - refs := besideRefs(dir, current[e.Manifest.Module].Commit) - beside[dir] = link.CheckedOut{Repository: url, Ref: refs[dir]} - if dir == "mesh-controller" { - beside["mesh-controller-main"] = link.CheckedOut{Repository: url, Ref: refs["mesh-controller-main"]} - if e.Source.Seat != "" { - for _, sibling := range []string{"mesh-lab", "mesh-sdk"} { - if url, err := clone(inventory.Source{Seat: e.Source.Seat, Repository: siblingOf(e.Source.Repository, - sibling)}); err == nil { - beside[sibling] = link.CheckedOut{Repository: url, Ref: refs[sibling]} - } - } - } - } + // **A pull request into a release branch is judged by that branch** (novox/hq ADR 0358): asked of the forge once. + release, err := releaseBranchOf(ctx, p, scope, entries) + if err != nil { + return link.BuildRequest{}, err + } + beside, err := besideOf(entries, func(module string) string { return current[module].Commit }, clone, release) + if err != nil { + return link.BuildRequest{}, err } builds := checkBuildsOf(p, scope, entries, clone) return link.BuildRequest{ @@ -283,10 +268,142 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop Source: sourceOnSeat(source), Check: &link.CheckRequest{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Base: p.Base, Paths: p.Paths, Beside: beside, Modules: scope.Modules, Dependents: scope.Dependents, New: scope.New, - Manifests: scope.Manifests, Judge: scope.Judge, Builds: builds, Named: namedForTheCheck(ctx, p, builds)}, + Manifests: scope.Manifests, Judge: scope.Judge, Builds: builds, Named: namedForTheCheck(ctx, p, builds), + Release: release}, }, nil } +// besideOf is what a check reads beside its own repository, by the directory it is found under: each core repository +// by the module the mesh builds from it, at besideRefs — or, for a pull request into a release branch (novox/hq ADR +// 0358 §3), at its branch of that name where it has one and at its trunk otherwise, the controller's main kept for +// its judge. running is the commit the mesh runs of a module. +func besideOf(entries []inventory.Entry, running func(module string) string, + clone func(inventory.Source) (string, error), release string) (map[string]link.CheckedOut, error) { + beside := map[string]link.CheckedOut{} + for _, e := range entries { + dir, core := coreModules[e.Manifest.Module] + if !core || e.Provided || e.Source.Repository == "" { + continue + } + url, err := clone(e.Source) + if err != nil { + return nil, err + } + refs := besideRefs(dir, running(e.Manifest.Module)) + at := func(dir, trunk string) link.CheckedOut { + if release == "" { + return link.CheckedOut{Repository: url, Ref: refs[dir]} + } + if trunk == "" { + trunk = "main" + } + return link.CheckedOut{Repository: url, Ref: release, Else: trunk} + } + beside[dir] = at(dir, followedBranch(e.Source.Ref)) + if dir == "mesh-controller" { + // The controller's main stays its main: it judges only a check the running controller predates. + beside["mesh-controller-main"] = link.CheckedOut{Repository: url, Ref: refs["mesh-controller-main"]} + if release != "" { + // mesh-controller has the release branch: that is what makes it one (ADR 0358 §1). + beside[dir] = link.CheckedOut{Repository: url, Ref: release} + } + if e.Source.Seat != "" { + for _, sibling := range []string{"mesh-lab", "mesh-sdk"} { + if url, err := clone(inventory.Source{Seat: e.Source.Seat, Repository: siblingOf(e.Source.Repository, + sibling)}); err == nil { + b := link.CheckedOut{Repository: url, Ref: refs[sibling]} + if release != "" { + b = link.CheckedOut{Repository: url, Ref: release, Else: "main"} + } + beside[sibling] = b + } + } + } + } + } + return beside, nil +} + +// releaseBranchOf is the release branch a pull request merges into (novox/hq ADR 0358 §1): its base, when that is not +// its repository's trunk and mesh-controller has a branch of that name; empty otherwise, and the pull request is +// checked as before it. **A base that is a trunk asks nothing**: main, the branch a module built from the repository +// follows, or mesh-controller's own trunk. Any other base is asked of the forge once, and an answer it cannot give is +// an error, never read as "no release branch". +func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope, entries []inventory.Entry) (string, error) { + base := p.Base + if base == "" || base == "main" { + return "", nil + } + if scope.From != nil && base == followedBranch(scope.From.Source.Ref) { + return "", nil + } + var controller *inventory.Entry + for i, e := range entries { + if e.Manifest.Module == "mesh-controller" && !e.Provided && e.Source.Repository != "" { + controller = &entries[i] + break + } + } + if controller == nil || base == followedBranch(controller.Source.Ref) { + return "", nil + } + if !tryRef.MatchString(base) || strings.Contains(base, "..") { + return "", nil // no branch the forge could hold: checked as before + } + owner, repo := sourceOwner(controller.Source.Repository), sourceName(controller.Source.Repository) + if owner == "" || repo == "" { + return "", nil + } + has, err := forgeHasBranch(ctx, owner, repo, base) + if err != nil { + return "", fmt.Errorf("whether %s/%s#%d merges into a release branch cannot be told — the forge did not say "+ + "whether %s/%s has a branch %s (novox/hq ADR 0358): %w", p.Owner, p.Repo, p.Number, owner, repo, base, err) + } + if !has { + return "", nil + } + return base, nil +} + +// sourceName is the name of a recorded repository, a path on the git seat or a URL: novox/mesh-host → mesh-host. +func sourceName(repository string) string { + parts := strings.Split(strings.Trim(strings.TrimSuffix(repository, ".git"), "/"), "/") + if len(parts) < 2 { + return "" + } + return parts[len(parts)-1] +} + +// forgeHasBranch asks the forge, through its module's tools on the bus, whether a repository has a branch. A +// variable so a test needs no forge. +var forgeHasBranch = func(ctx context.Context, owner, repo, branch string) (bool, error) { + js, err := aBus() + if err != nil { + return false, err + } + defer js.Close() + raw, _ := json.Marshal(map[string]any{"path": "/repos/" + owner + "/" + repo + "/branches/" + branch}) + answer, err := link.Ask(ctx, link.OverNATS{Conn: js.Conn()}, "gitea", "gitea_api", raw, 30*time.Second) + if err != nil { + return false, err + } + if answer.Error != "" { + if strings.Contains(answer.Error, " 404 ") { + return false, nil + } + return false, errors.New(answer.Error) + } + var found struct { + Result struct { + Name string `json:"name"` + } `json:"result"` + } + if err := json.Unmarshal(answer.Result, &found); err != nil { + return false, err + } + return found.Result.Name == branch, nil +} + // namedForTheCheck are the builds earlier checks of the same pull request at the same head named, read once for the // check (novox/hq issue 703): asked again because only the trunk moved, the check keeps what they built of a key // still the same. Another pull request's build, or one of an earlier head, is never kept. None when they cannot be @@ -458,6 +575,7 @@ func checkedOf(result link.BuildResult) link.Checked { // repositories read beside it. c.Trunk, c.Beside, c.Took = result.Check.Trunk, result.Check.Beside, result.Check.Took c.Built = result.Check.Built + c.Release, c.JudgedBy, c.BesideAt = result.Check.Release, result.Check.JudgedBy, result.Check.BesideAt 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/release_branch_test.go b/cmd/mesh-controller/release_branch_test.go new file mode 100644 index 00000000..3147b3d5 --- /dev/null +++ b/cmd/mesh-controller/release_branch_test.go @@ -0,0 +1,167 @@ +package main + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// **A pull request into a release branch is judged by that branch** (novox/hq ADR 0358, issue 704): a base that is +// not its repository's trunk, where mesh-controller has a branch of that name, is a release branch; the check then +// reads every repository beside it at its branch of that name where it has one, else at its trunk. Any other base is +// checked exactly as before. + +const aRelease = "feat/57-config-and-provisioning" + +// theCore is the mesh's core as a check reads it beside: the controller, the node-engine, the tool runner and the +// catalogue's bus, on the git seat. +func theCore() []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" + return []inventory.Entry{onSeat("mesh-controller", "novox/mesh-controller"), onSeat("mesh-host", "novox/mesh-host"), + onSeat("node-tools", "novox/mesh-tools"), nats, onSeat("gitea", "novox/mesh-catalog")} +} + +// aForgeWithBranches stands in for the forge: which branches each repository has, and how often it was asked. +func aForgeWithBranches(t *testing.T, branches map[string][]string, failing error) *int { + t.Helper() + asked := 0 + was := forgeHasBranch + t.Cleanup(func() { forgeHasBranch = was }) + forgeHasBranch = func(_ context.Context, owner, repo, branch string) (bool, error) { + asked++ + if failing != nil { + return false, failing + } + for _, b := range branches[owner+"/"+repo] { + if b == branch { + return true, nil + } + } + return false, nil + } + return &asked +} + +func TestABaseIsAReleaseBranchWhenTheControllerHasABranchOfThatName(t *testing.T) { + entries := theCore() + scopeOf := func(p link.PullUpdated) checkScope { return pullScope(p, entries, nil, nil) } + pull := func(repo, base string) link.PullUpdated { + return link.PullUpdated{Owner: "novox", Repo: repo, Number: 286, Base: base, Commit: "abc", + Paths: []string{"modules/gitea/index.ts"}} + } + + asked := aForgeWithBranches(t, map[string][]string{"novox/mesh-controller": {"main", aRelease}}, nil) + for _, c := range []struct { + what string + p link.PullUpdated + release string + asks int + }{ + {"a pull request into the trunk asks nothing and is no release", pull("mesh-catalog", "main"), "", 0}, + {"a catalogue pull request into the release branch", pull("mesh-catalog", aRelease), aRelease, 1}, + {"a controller pull request into the release branch", pull("mesh-controller", aRelease), aRelease, 1}, + {"a base mesh-controller has no branch of is checked as today", pull("mesh-catalog", "feat/other"), "", 1}, + {"no base: as today", pull("mesh-catalog", ""), "", 0}, + } { + *asked = 0 + got, err := releaseBranchOf(t.Context(), c.p, scopeOf(c.p), entries) + if err != nil { + t.Fatalf("%s: %v", c.what, err) + } + if got != c.release || *asked != c.asks { + t.Errorf("%s: release %q after %d question(s) to the forge, not %q after %d", c.what, got, *asked, + c.release, c.asks) + } + } + + // The branch a module built from the repository follows is its trunk: never asked about. + photos := fromRepo("photos", "novox/photos", "") + photos.Source.Ref = "nox-mesh" + withPhotos := append(theCore(), photos) + p := link.PullUpdated{Owner: "novox", Repo: "photos", Base: "nox-mesh", Commit: "abc", Paths: []string{"x.js"}} + *asked = 0 + if got, err := releaseBranchOf(t.Context(), p, pullScope(p, withPhotos, nil, nil), withPhotos); err != nil || + got != "" || *asked != 0 { + t.Fatalf("a module's own trunk read as release %q (%v) after %d question(s)", got, err, *asked) + } + + // A forge that cannot answer is an error, never "no release branch". + aForgeWithBranches(t, nil, errors.New("nothing serves gitea.gitea_api")) + q := pull("mesh-catalog", aRelease) + if _, err := releaseBranchOf(t.Context(), q, scopeOf(q), entries); err == nil || + !strings.Contains(err.Error(), "cannot be told") { + t.Fatalf("an unanswered forge was read as an answer: %v", err) + } +} + +func TestBesideAReleaseBranchCheckEachRepositoryIsReadAtThatBranchElseItsTrunk(t *testing.T) { + entries := theCore() + clone := func(s inventory.Source) (string, error) { return "http://git/" + s.Repository + ".git", nil } + running := func(module string) string { return "running-" + module } + + beside, err := besideOf(entries, running, clone, aRelease) + if err != nil { + t.Fatal(err) + } + at := func(repo string, ref, orElse string) link.CheckedOut { + return link.CheckedOut{Repository: "http://git/novox/" + repo + ".git", Ref: ref, Else: orElse} + } + want := map[string]link.CheckedOut{ + // The judge: mesh-controller's release branch, which it has. + "mesh-controller": at("mesh-controller", aRelease, ""), + "mesh-controller-main": at("mesh-controller", "main", ""), + // A controller pull request into the release branch reads mesh-host at that branch where it has one. + "mesh-host": at("mesh-host", aRelease, "main"), + "mesh-tools": at("mesh-tools", aRelease, "main"), + "mesh-catalog": at("mesh-catalog", aRelease, "main"), + "mesh-lab": at("mesh-lab", aRelease, "main"), + "mesh-sdk": at("mesh-sdk", aRelease, "main"), + } + if !reflect.DeepEqual(beside, want) { + t.Fatalf("beside a release-branch check:\n got %v\nwant %v", beside, want) + } + + // Any other base: exactly as before — what the mesh runs, the catalogue's and the lab's main. + beside, err = besideOf(entries, running, clone, "") + if err != nil { + t.Fatal(err) + } + want = map[string]link.CheckedOut{ + "mesh-controller": at("mesh-controller", "running-mesh-controller", ""), + "mesh-controller-main": at("mesh-controller", "main", ""), + "mesh-host": at("mesh-host", "running-mesh-host", ""), + "mesh-tools": at("mesh-tools", "running-node-tools", ""), + "mesh-catalog": at("mesh-catalog", "main", ""), + "mesh-lab": at("mesh-lab", "main", ""), + "mesh-sdk": at("mesh-sdk", sdkPinned(), ""), + } + if !reflect.DeepEqual(beside, want) { + t.Fatalf("beside a trunk check:\n got %v\nwant %v", beside, want) + } +} + +func TestAReleaseBranchVerdictNamesTheBranchTheJudgeAndWhatItRead(t *testing.T) { + asked := &link.CheckRequest{Owner: "novox", Repo: "mesh-catalog", Number: 286, Base: aRelease, Release: aRelease} + c := checkedOf(link.BuildResult{ID: "b", Ref: "aaaaaaaaaaaa0000", Checked: asked, Check: &link.CheckOutcome{ + Verdict: "pass", Head: "aaaaaaaaaaaa", Trunk: "111111111111", Gate: &link.CheckLayer{Verdict: "pass"}, + RepoCheck: &link.CheckLayer{Verdict: "pass"}, Release: aRelease, JudgedBy: "fcdecb436117", + Beside: map[string]string{"mesh-host": "222222222222", "mesh-lab": "333333333333"}, + BesideAt: map[string]string{"mesh-host": aRelease, "mesh-lab": "main"}}}) + if c.Release != aRelease || c.JudgedBy != "fcdecb436117" { + t.Fatalf("the verdict names release %q judged by %q", c.Release, c.JudgedBy) + } + if c.BesideAt["mesh-host"] != aRelease || c.BesideAt["mesh-lab"] != "main" || c.Beside["mesh-host"] != "222222222222" { + t.Fatalf("the verdict names what it read beside it as %v at %v", c.Beside, c.BesideAt) + } +} diff --git a/internal/builder/check.go b/internal/builder/check.go index 1b2648fe..1a9d32bb 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -13,6 +13,7 @@ import ( "os/exec" "path/filepath" "regexp" + "sort" "strings" "time" @@ -87,6 +88,16 @@ type CheckSpec struct { // Judge is who judges the gate: "" the controller the mesh runs, "self" the change's own, "validator" // the running one with the change's validator. Judge string + // Release is the release branch the pull request merges into (novox/hq ADR 0358), empty for any other base. The + // gate is then judged by the controller beside it as mesh-controller — mesh-controller's release branch, never + // the controller the mesh runs nor its main — with mesh-host's validator from beside it when mesh-host was read at + // the release branch too; a change to the controller still judges itself and a change to the node-engine brings + // its own validator. The verdict names the branch, the judge's commit and the branch each beside was read at. + Release string + // JudgeCache is a directory outside the workspace where a release branch's judge is kept by the commits it is + // built from, so it is built once per release-branch head (ADR 0358 §2). Outside: the check's containers mount + // the workspace and a pull request's own script must not plant a judge for the next check. Empty keeps none. + JudgeCache string // Toolchain is the image a check's Go runs in: the mesh's own Go toolchain, as it holds it. // Toolchains is every toolchain the mesh holds, by language, for a script that declares another. Toolchain string @@ -153,10 +164,12 @@ func ToolchainsOf(held map[string]string) map[string]string { return out } -// Beside is one repository cloned next to the one checked. +// Beside is one repository cloned next to the one checked: at Ref, or at Else when Else is said and the repository +// has no branch Ref (novox/hq ADR 0358 §3). type Beside struct { Repository string Ref string + Else string } // CheckVerdict is what came of one check. Verdict and Summary are the gate's; Gate and Repo each layer. @@ -179,6 +192,12 @@ type CheckVerdict struct { Members []JudgedHead // Built are the builds it made for its merge (novox/hq ADR 0350 §1), each with its key, or why not. Built []CheckBuilt + // Release is the release branch it was judged for (novox/hq ADR 0358 §4), JudgedBy the commit of the controller + // that judged its gate (twelve characters), and BesideAt the branch each repository beside it was read at, by + // directory. Empty for a check of any other base. + Release string + JudgedBy string + BesideAt map[string]string } // Layer is one layer of a check, judged. @@ -303,13 +322,23 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if err := forgetLeftCredentials(workspace); err != nil { return CheckVerdict{}, err } - clone := func(repository, ref, dir string) error { + // cloneAt clones a repository and checks out ref — or, when the repository has no branch ref and orElse is said, + // orElse (novox/hq ADR 0358 §3); readAt keeps which, by directory. + readAt := map[string]string{} + cloneAt := func(repository, ref, orElse, dir string) error { if _, err := run(ctx, root, "git", cloneWith(credentials, "clone", "--quiet", repository, dir)...); err != nil { return fmt.Errorf("cannot clone %s: %s", redact.redact(repository), redact.redact(err.Error())) } if err := recordedWithoutUserinfo(ctx, run, filepath.Join(root, dir), repository); err != nil { return err } + if orElse != "" && ref != "" { + if _, err := run(ctx, filepath.Join(root, dir), "git", "rev-parse", "--verify", "--quiet", + "refs/remotes/origin/"+ref); err != nil { + ref = orElse + } + } + readAt[dir] = ref if ref != "" { if _, err := run(ctx, filepath.Join(root, dir), "git", "checkout", "--quiet", ref); err != nil { return fmt.Errorf("%s has no %s: %w", redact.redact(repository), ref, err) @@ -317,6 +346,9 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry } return nil } + clone := func(repository, ref, dir string) error { + return cloneAt(repository, ref, "", dir) + } name := spec.Repo if name == "" || !safeName.MatchString(name) { name = "checked" @@ -352,6 +384,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 = spec.Release, judged.JudgedBy, judged.BesideAt return v } // **What the merge would move is built from a checkout nothing of the check touches** (novox/hq issue 703): the @@ -392,9 +425,16 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if dir == name || !safeName.MatchString(dir) { continue } - if err := clone(b.Repository, b.Ref, dir); err != nil { + if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil { return CheckVerdict{}, fmt.Errorf("beside it, %w", err) } + if spec.Release != "" { + // The branch each was read at is named with its commit (ADR 0358 §4). + if judged.BesideAt == nil { + judged.BesideAt = map[string]string{} + } + judged.BesideAt[dir] = readAt[dir] + } // The commit each one read is named in the verdict (ADR 0323 §3): its move is issue 568's question. if at := headAt(ctx, run, filepath.Join(root, dir)); at != "" { if judged.Beside == nil { @@ -402,7 +442,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry } judged.Beside[dir] = at } - say("check", "beside it %s at %s", dir, short(b.Ref)) + 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. @@ -542,10 +582,21 @@ 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, inToolchain, say) + gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, inToolchain, say) if err != nil { return CheckVerdict{}, err } + 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. + judgedBy := filepath.Join(root, "mesh-controller") + if spec.Judge == "self" { + judgedBy = tree + } + judged.JudgedBy = headAt(ctx, run, judgedBy) + fmt.Fprintf(&out, "--- %s\n", releaseWords(spec.Release, judged.JudgedBy, judged.BesideAt, judged.Beside)) + say("check", "%s", releaseWords(spec.Release, judged.JudgedBy, judged.BesideAt, judged.Beside)) + } verdictFile := filepath.Join(root, "verdict.json") env := []string{EnvFacts + "=" + factsFile, EnvGate + "=" + gate, EnvGateStore + "=" + storeURL, EnvTestStore + "=" + storeURL, EnvTestBus + "=nats://" + bus, EnvRepository + "=" + spec.Owner + "/" + spec.Repo, @@ -611,6 +662,11 @@ 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) + 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. v.Gate.Summary, v.Repo.Summary, v.Repo.Failed = redact.redact(v.Gate.Summary), redact.redact(v.Repo.Summary), redact.redact(v.Repo.Failed) @@ -968,7 +1024,12 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil // the mesh runs. It answers the judge's path; or, when the change makes its own judge unbuildable, why — // the change's fault, a failing gate; or an error when the check cannot build a judge at all. A check // whose gate does not run builds a judge only to hand its scripts one, and goes on without when it cannot. -func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, tree string, +// +// **A pull request into a release branch is judged by that branch's controller** (novox/hq ADR 0358 §2): the one +// beside it as mesh-controller, read at the release branch — with mesh-host's validator from beside it when mesh-host +// 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, inToolchain func(string, []string, ...string) []string, say func(step, format string, args ...any)) (string, string, error) { gated := spec.Gated() switch spec.Judge { @@ -982,7 +1043,14 @@ func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, t say("check", "judged by the change's own controller") return bin, "", nil } - bin, judgeTree, err := judge(ctx, labelled, run, root, inToolchain, say) + var bin, judgeTree string + var err error + if spec.Release != "" { + withHost := spec.Judge != "validator" && besideAt["mesh-host"] == spec.Release + bin, judgeTree, err = releaseJudge(ctx, labelled, run, spec, root, withHost, inToolchain, say) + } else { + bin, judgeTree, err = judge(ctx, labelled, run, root, inToolchain, say) + } if err != nil { if !gated { say("check", "no judge for its script: %v", err) @@ -1004,12 +1072,181 @@ func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, t if _, err := labelled(ctx, root, "docker", inToolchain(filepath.Join(root, judgeTree), []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")}, "go", "build", "-o", withValidator, "./cmd/mesh-controller")...); err != nil { + if spec.Release != "" { + return "", spec.Release + "'s controller does not build with this change's validator: " + + firstLine(err.Error()), nil + } return "", "the controller the mesh runs does not build with this change's validator: " + firstLine(err.Error()), nil } + if spec.Release != "" { + say("check", "judged by %s's controller, with this change's validator", spec.Release) + return withValidator, "", nil + } say("check", "judged by the controller the mesh runs, with this change's validator") return withValidator, "", nil } +// releaseJudge builds the controller that judges a pull request into a release branch (novox/hq ADR 0358 §2): the +// one beside the check as mesh-controller, read at that branch — with mesh-host's validator from beside it in place +// of the one it vendors when withHost. **Built once per release-branch head**: kept in the spec's judge cache by the +// commits it is built from and taken from there while they stand. It answers the binary and the directory it was +// built from. +func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root string, withHost bool, + inToolchain func(string, []string, ...string) []string, say func(step, format string, args ...any)) (string, string, error) { + const dir = "mesh-controller" + if _, err := os.Stat(filepath.Join(root, dir)); err != nil { + return "", "", fmt.Errorf("no controller of %s beside the check to judge it with", spec.Release) + } + key := fullHeadAt(ctx, plain, filepath.Join(root, dir)) + if key == "" { + return "", "", fmt.Errorf("the controller of %s beside the check names no commit", spec.Release) + } + with := "" + if withHost { + host := fullHeadAt(ctx, plain, filepath.Join(root, "mesh-host")) + if host == "" { + return "", "", fmt.Errorf("mesh-host of %s beside the check names no commit", spec.Release) + } + key += "-with-mesh-host-" + host + with = ", with mesh-host's validator of " + spec.Release + " at " + twelve(host) + } + bin := filepath.Join(root, "bin", "mesh-controller") + if took := judgeKept(spec.JudgeCache, key, bin); took { + say("check", "judged by %s's controller at %s%s, built for an earlier check of that head", spec.Release, + twelve(key), with) + return bin, dir, nil + } + if withHost { + vendored := filepath.Join(root, dir, "vendor", "github.com", "novox", "mesh-host") + for _, pkg := range []string{"validate", filepath.Join("internal", "declaration")} { + if err := replaceGoFiles(filepath.Join(root, "mesh-host", pkg), filepath.Join(vendored, pkg)); err != nil { + return "", "", fmt.Errorf("mesh-host's validator of %s could not be put in its controller: %w", spec.Release, err) + } + } + } + if _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), + []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")}, + "go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil { + return "", "", fmt.Errorf("the controller of %s does not build%s: %w", spec.Release, with, err) + } + // Kept before anything of the change runs: nothing a pull request's script writes can reach the cache. + keepJudge(spec.JudgeCache, key, bin, say) + say("check", "judged by %s's controller at %s%s", spec.Release, twelve(key), with) + return bin, dir, nil +} + +// judgesKept is how many release-branch judges a judge cache keeps: a head moved past is not built from again. +const judgesKept = 4 + +// judgeKept puts the judge kept under key in the cache at bin, and says whether there was one. +func judgeKept(cache, key, bin string) bool { + if cache == "" { + return false + } + body, err := os.ReadFile(filepath.Join(cache, key, "mesh-controller")) + if err != nil { + return false + } + if err := os.MkdirAll(filepath.Dir(bin), 0o755); err != nil { + return false + } + if err := os.WriteFile(bin, body, 0o755); err != nil { + return false + } + now := time.Now() + _ = os.Chtimes(filepath.Join(cache, key), now, now) + return true +} + +// keepJudge keeps a judge just built under key, and lets go of all but the judgesKept most recently used. A judge +// that cannot be kept is said and built again next time: the cache only saves time. +func keepJudge(cache, key, bin string, say func(step, format string, args ...any)) { + if cache == "" { + return + } + body, err := os.ReadFile(bin) + if err == nil { + err = os.MkdirAll(filepath.Join(cache, key), 0o755) + } + if err == nil { + kept := filepath.Join(cache, key, "mesh-controller") + if err = os.WriteFile(kept+".part", body, 0o755); err == nil { + err = os.Rename(kept+".part", kept) + } + } + if err != nil { + say("check", "the judge could not be kept for the next check, which builds it again: %v", err) + return + } + entries, err := os.ReadDir(cache) + if err != nil || len(entries) <= judgesKept { + return + } + type used struct { + name string + at time.Time + } + var all []used + for _, e := range entries { + if info, err := e.Info(); err == nil && e.IsDir() { + all = append(all, used{e.Name(), info.ModTime()}) + } + } + sort.Slice(all, func(i, j int) bool { return all[i].at.After(all[j].at) }) + for _, u := range all[min(judgesKept, len(all)):] { + if u.name != key { + _ = os.RemoveAll(filepath.Join(cache, u.name)) + } + } +} + +// fullHeadAt is the whole commit a checkout stands at, empty when it names none. +func fullHeadAt(ctx context.Context, run Runner, tree string) string { + out, err := run(ctx, tree, "git", "rev-parse", "--verify", "--quiet", "HEAD^{commit}") + if err != nil { + return "" + } + return strings.TrimSpace(out) +} + +// releaseWords is how a release-branch check says what judged it and what it read (novox/hq ADR 0358 §4). +func releaseWords(release, judgedBy 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", release, judgedBy) + if len(read) > 0 { + said += "; read beside it: " + strings.Join(read, ", ") + } + return said +} + +// 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) +} + +// refWords is a ref as a line says it: a commit by its first eight characters, a branch whole. +func refWords(ref string) string { + if commitLike.MatchString(ref) { + return short(ref) + } + return ref +} + +// commitLike is a ref that names a commit. +var commitLike = regexp.MustCompile(`^[0-9a-f]{7,40}$`) + +func sortedKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + // replaceGoFiles puts a package's Go files — never its tests — in place of another copy's. func replaceGoFiles(from, into string) error { old, err := filepath.Glob(filepath.Join(into, "*.go")) diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go new file mode 100644 index 00000000..a30406cd --- /dev/null +++ b/internal/builder/release_branch_test.go @@ -0,0 +1,308 @@ +package builder + +import ( + "net" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// **A pull request into a release branch is judged by the controller of that branch** (novox/hq ADR 0358, issue +// 704): the build seat's check builds the judge from mesh-controller read at the release branch — with mesh-host's +// validator of that branch when mesh-host has one — reads every repository beside it at that branch where it has one +// and at its trunk otherwise, and names the branch, the judge's commit and each repository read with its commit. + +const releaseBranch = "feat/57-config-and-provisioning" + +// gitInRepo runs git in a repository made for a test. +func gitInRepo(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)) +} + +// commitFiles writes files into a repository and commits them on the branch checked out. +func commitFiles(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) + } + } + gitInRepo(t, dir, "add", "-A") + gitInRepo(t, dir, "commit", "--quiet", "-m", "x") + return gitInRepo(t, dir, "rev-parse", "HEAD") +} + +// aRepositoryWithARelease is a repository whose main holds mainFiles and, when releaseFiles is not nil, a release +// branch over them holding releaseFiles. It answers the repository, main's head and the release branch's head. +func aRepositoryWithARelease(t *testing.T, mainFiles, releaseFiles map[string]string) (string, string, string) { + t.Helper() + dir := t.TempDir() + gitInRepo(t, dir, "init", "--quiet", "-b", "main") + main := commitFiles(t, dir, mainFiles) + if releaseFiles == nil { + return dir, main, "" + } + gitInRepo(t, dir, "checkout", "--quiet", "-b", releaseBranch) + head := commitFiles(t, dir, releaseFiles) + gitInRepo(t, dir, "checkout", "--quiet", "main") + return dir, main, head +} + +// aJudgeOf is a controller that judges as told: its gate passes, saying who it is and the word of the node-engine +// validator it vendors — the one it was built with. +func aJudgeOf(who 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) +} +`, + } +} + +// aHostSaying is the node-engine's validator, saying a word. +func aHostSaying(word string) map[string]string { + return map[string]string{ + "validate/v.go": "package validate\n\nconst Word = \"" + word + "\"\n", + "internal/declaration/d.go": "package declaration\n", + "validate/v_test.go": "package validate\n", + "internal/declaration/x_test.go": "package declaration\n", + } +} + +// aRuntimeThatBuildsTheJudge 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 — the judge's build too — and a store and a bus that +// answer. It answers the file each `go` the check runs is written to. +func aRuntimeThatBuildsTheJudge(t *testing.T) string { + t.Helper() + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no Go toolchain on this machine to build a judge with") + } + 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() + built := filepath.Join(dir, "built") + runtime := `#!/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 + if [ "$1" = go ]; then echo "$*" >> "` + built + `"; fi + cd "$workdir" && exec "$@" ;; +esac +exit 1 +` + if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(runtime), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("GOTOOLCHAIN", "local") + return built +} + +// builds is how many times the check ran go to build a judge. +func builds(t *testing.T, file string) int { + t.Helper() + body, err := os.ReadFile(file) + if err != nil { + return 0 + } + return strings.Count(string(body), "go build") +} + +// theReleasedMesh is the core beside a release-branch check: a controller whose main and release branch judge +// differently, a node-engine with a validator of each, and a lab with no release branch. +func theReleasedMesh(t *testing.T) (map[string]Beside, string, string, string) { + t.Helper() + controller, _, controllerRelease := aRepositoryWithARelease(t, aJudgeOf("main's controller"), + aJudgeOf("the release branch's controller")) + host, _, hostRelease := aRepositoryWithARelease(t, aHostSaying("main's validator"), + aHostSaying("the release branch's validator")) + lab, labMain, _ := aRepositoryWithARelease(t, map[string]string{"README": "the lab"}, nil) + return map[string]Beside{ + "mesh-controller": {Repository: controller, Ref: releaseBranch}, + "mesh-controller-main": {Repository: controller, Ref: "main"}, + "mesh-host": {Repository: host, Ref: releaseBranch, Else: "main"}, + "mesh-lab": {Repository: lab, Ref: releaseBranch, Else: "main"}, + }, controllerRelease, hostRelease, labMain +} + +// aPullRequestInto is a repository whose pull request's head is cut from base, with a change. +func aPullRequestInto(t *testing.T, files map[string]string, base string, change map[string]string) (string, string) { + t.Helper() + repo, _, _ := aRepositoryWithARelease(t, files, map[string]string{"released": "r"}) + gitInRepo(t, repo, "checkout", "--quiet", "-b", "change", base) + head := commitFiles(t, repo, change) + gitInRepo(t, repo, "checkout", "--quiet", "main") + return repo, head +} + +func TestACataloguePullRequestIntoAReleaseBranchIsJudgedByThatBranchsController(t *testing.T) { + built := aRuntimeThatBuildsTheJudge(t) + registry := aFactsRegistry(t) + beside, controllerRelease, hostRelease, labMain := theReleasedMesh(t) + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, releaseBranch, map[string]string{"modules/gitea/index.ts": "x"}) + cache := t.TempDir() + check := func(id string) CheckVerdict { + t.Helper() + v, err := Check(t.Context(), Command, CheckSpec{ID: id, Repository: catalogue, Ref: head, Owner: "novox", + Repo: "mesh-catalog", Number: 286, Base: releaseBranch, Release: releaseBranch, JudgeCache: cache, + Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Beside: beside, Toolchain: "golang"}, + t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + return v + } + v := check("check-704-catalogue") + // The release branch's controller judged, with mesh-host's validator of the release branch. + if v.Gate == nil || v.Gate.Verdict != "pass" || + !strings.HasSuffix(v.Gate.Summary, "the release branch's controller with the release branch's validator") { + t.Fatalf("the gate was not judged by the release branch's controller with its node-engine's validator: %+v\n%s", + v.Gate, v.Report) + } + // It names the release branch, the judge's commit, and each repository read beside it at its branch and commit. + mark := "[release " + releaseBranch + ", judged by its controller " + controllerRelease[:12] + "] " + if !strings.HasPrefix(v.Gate.Summary, mark) || !strings.HasPrefix(v.Repo.Summary, mark) { + t.Fatalf("the statuses do not name the release branch and its judge first:\n%s\n%s", v.Gate.Summary, v.Repo.Summary) + } + if v.Release != releaseBranch || v.JudgedBy != controllerRelease[:12] { + t.Fatalf("the verdict names release %q judged by %q, not %s by %s", v.Release, v.JudgedBy, releaseBranch, + controllerRelease[:12]) + } + if v.BesideAt["mesh-host"] != releaseBranch || v.Beside["mesh-host"] != hostRelease[:12] { + t.Fatalf("mesh-host was read at %q (%s), not its release branch at %s", v.BesideAt["mesh-host"], + v.Beside["mesh-host"], hostRelease[:12]) + } + // A repository with no release branch is read at its trunk, and says so. + if v.BesideAt["mesh-lab"] != "main" || v.Beside["mesh-lab"] != labMain[:12] { + t.Fatalf("the lab, with no release branch, was read at %q (%s), not main at %s", v.BesideAt["mesh-lab"], + v.Beside["mesh-lab"], labMain[:12]) + } + if !strings.Contains(v.Report, "read beside it: ") || !strings.Contains(v.Report, "mesh-host "+releaseBranch+" at "+hostRelease[:12]) { + t.Fatalf("the report does not say what was read beside it:\n%s", v.Report) + } + if n := builds(t, built); n != 1 { + t.Fatalf("the judge was built %d times for one check", n) + } + + // **Built once per release-branch head**: a second check while that head stands takes the judge kept. + v = check("check-704-catalogue-again") + if !strings.HasSuffix(v.Gate.Summary, "the release branch's controller with the release branch's validator") { + t.Fatalf("the kept judge did not judge: %+v\n%s", v.Gate, v.Report) + } + if n := builds(t, built); n != 1 { + t.Fatalf("the judge of the same release-branch head was built again: %d builds", n) + } +} + +func TestAControllerPullRequestIntoAReleaseBranchReadsTheNodeEngineAtThatBranch(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + registry := aFactsRegistry(t) + beside, _, hostRelease, _ := theReleasedMesh(t) + delete(beside, "mesh-controller") + controller, head := aPullRequestInto(t, aJudgeOf("the change's controller"), releaseBranch, + map[string]string{"cmd/x.txt": "x"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-controller", Repository: controller, Ref: head, + Owner: "novox", Repo: "mesh-controller", Number: 315, Base: releaseBranch, Release: releaseBranch, + JudgeCache: t.TempDir(), Paths: []string{"cmd/x.txt"}, Modules: []string{"mesh-controller"}, Judge: "self", + Beside: beside, Toolchain: "golang"}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || !strings.HasSuffix(v.Gate.Summary, "the change's controller with the vendored validator") { + t.Fatalf("a controller change into a release branch did not judge itself: %+v\n%s", v.Gate, v.Report) + } + if v.BesideAt["mesh-host"] != releaseBranch || v.Beside["mesh-host"] != hostRelease[:12] { + t.Fatalf("mesh-host was read beside it at %q (%s), not at the release branch's %s", v.BesideAt["mesh-host"], + v.Beside["mesh-host"], hostRelease[:12]) + } + if v.Release != releaseBranch || v.JudgedBy == "" || v.JudgedBy != v.Merge[:12] { + t.Fatalf("the verdict names release %q judged by %q, not its own merge %s", v.Release, v.JudgedBy, v.Merge) + } +} + +func TestAPullRequestIntoTheTrunkIsJudgedAsBefore(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + registry := aFactsRegistry(t) + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("main's controller"), + aJudgeOf("the release branch's controller")) + host, hostMain, _ := aRepositoryWithARelease(t, aHostSaying("main's validator"), + aHostSaying("the release branch's validator")) + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, "main", map[string]string{"modules/gitea/index.ts": "x"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-trunk", Repository: catalogue, Ref: head, + Owner: "novox", Repo: "mesh-catalog", Number: 1, Base: "main", JudgeCache: t.TempDir(), + Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Toolchain: "golang", + Beside: map[string]Beside{"mesh-controller": {Repository: controller, Ref: controllerMain}, + "mesh-host": {Repository: host, Ref: hostMain}}}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Summary != "main's controller with the vendored validator" { + t.Fatalf("a trunk pull request was not judged by the controller beside it as before: %+v\n%s", v.Gate, v.Report) + } + if v.Release != "" || v.JudgedBy != "" || v.BesideAt != nil || strings.Contains(v.Report, "release branch") { + t.Fatalf("a trunk pull request's verdict speaks of a release branch: %q %q %v", v.Release, v.JudgedBy, v.BesideAt) + } + if v.Beside["mesh-host"] != hostMain[:12] { + t.Fatalf("mesh-host was read at %s, not the commit asked %s", v.Beside["mesh-host"], hostMain[:12]) + } +} diff --git a/internal/link/build.go b/internal/link/build.go index 1522ddaf..e5ddafc4 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -167,6 +167,12 @@ type CheckRequest struct { // the controller, judged by itself; JudgeValidator for a change to the node-engine, judged by the // running controller built with the change's validator in place of the one it vendors. Judge string `json:"judge,omitempty"` + // Release is the release branch the pull request merges into (novox/hq ADR 0358): its base, when that is not its + // repository's trunk and mesh-controller has a branch of that name. The gate is then judged by the controller + // built from mesh-controller's release branch — a change to the controller still by itself, a change to the + // 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"` // 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. @@ -257,6 +263,10 @@ const ( type CheckedOut struct { Repository string `json:"repository"` Ref string `json:"ref,omitempty"` + // Else is the ref read when the repository has no branch Ref (novox/hq ADR 0358 §3): beside a check of a pull + // request into a release branch, Ref is that branch and Else the repository's trunk. Empty: Ref is read, or the + // check cannot run. + Else string `json:"else,omitempty"` } // CheckOutcome is a merge check's verdict. @@ -281,6 +291,12 @@ type CheckOutcome struct { Members []CheckedMember `json:"members,omitempty"` // Built are the builds it made for its merge (novox/hq ADR 0350 §1). Built []CheckBuilt `json:"built,omitempty"` + // Release is the release branch the check was judged for (novox/hq ADR 0358 §4), JudgedBy the commit of the + // controller that judged its gate, as twelve characters, and BesideAt the branch each repository beside it was + // read at, by directory. Empty from a check of any other base, and from a build seat from before it. + Release string `json:"release,omitempty"` + JudgedBy string `json:"judged-by,omitempty"` + BesideAt map[string]string `json:"beside-at,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 edbcfbc3..30265231 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -293,6 +293,12 @@ type Checked struct { Trunk string `json:"trunk,omitempty"` Beside map[string]string `json:"beside,omitempty"` Took string `json:"took,omitempty"` + // Release is the release branch the check judged for (novox/hq ADR 0358 §4), JudgedBy the commit of the + // controller that judged its gate (twelve characters), and BesideAt the branch each repository beside it was + // read at, by directory: so a reader sees a release-branch judgement apart from a trunk one. Empty otherwise. + Release string `json:"release,omitempty"` + JudgedBy string `json:"judged-by,omitempty"` + BesideAt map[string]string `json:"beside-at,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"` -- 2.54.0 From ea7d91bff190b4af7ab6baed27dcf4ace02d0377 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 12:56:55 +0000 Subject: [PATCH 2/8] Say which validator judged a release-branch check, and prove clause 5 ADR 0358 version 4 asks the verdict to say whether mesh-host's release-branch validator or the vendored one judged, and checks that a release branch's own pull request into its trunk is judged as any trunk pull request (issue 704). Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-builder/main.go | 2 +- cmd/mesh-controller/checks.go | 1 + cmd/mesh-controller/release_branch_test.go | 34 +++++++++- internal/builder/check.go | 30 +++++++-- internal/builder/release_branch_test.go | 75 ++++++++++++++++++++++ internal/link/build.go | 2 + internal/link/events.go | 2 + 7 files changed, 137 insertions(+), 9 deletions(-) diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index 32ec20cb..fc418367 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -331,7 +331,7 @@ 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} + Release: v.Release, JudgedBy: v.JudgedBy, BesideAt: v.BesideAt, Validator: v.Validator} // 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, diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 3fa4a882..e51c97c5 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -576,6 +576,7 @@ func checkedOf(result link.BuildResult) link.Checked { c.Trunk, c.Beside, c.Took = result.Check.Trunk, result.Check.Beside, result.Check.Took 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 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/release_branch_test.go b/cmd/mesh-controller/release_branch_test.go index 3147b3d5..e6ddf276 100644 --- a/cmd/mesh-controller/release_branch_test.go +++ b/cmd/mesh-controller/release_branch_test.go @@ -156,12 +156,40 @@ func TestAReleaseBranchVerdictNamesTheBranchTheJudgeAndWhatItRead(t *testing.T) c := checkedOf(link.BuildResult{ID: "b", Ref: "aaaaaaaaaaaa0000", Checked: asked, Check: &link.CheckOutcome{ Verdict: "pass", Head: "aaaaaaaaaaaa", Trunk: "111111111111", Gate: &link.CheckLayer{Verdict: "pass"}, RepoCheck: &link.CheckLayer{Verdict: "pass"}, Release: aRelease, JudgedBy: "fcdecb436117", - Beside: map[string]string{"mesh-host": "222222222222", "mesh-lab": "333333333333"}, - BesideAt: map[string]string{"mesh-host": aRelease, "mesh-lab": "main"}}}) + Beside: map[string]string{"mesh-host": "222222222222", "mesh-lab": "333333333333"}, + BesideAt: map[string]string{"mesh-host": aRelease, "mesh-lab": "main"}, + Validator: "mesh-host's validator of " + aRelease + " at 222222222222"}}) if c.Release != aRelease || c.JudgedBy != "fcdecb436117" { t.Fatalf("the verdict names release %q judged by %q", c.Release, c.JudgedBy) } - if c.BesideAt["mesh-host"] != aRelease || c.BesideAt["mesh-lab"] != "main" || c.Beside["mesh-host"] != "222222222222" { + if c.BesideAt["mesh-host"] != aRelease || c.BesideAt["mesh-lab"] != "main" || c.Beside["mesh-host"] != "222222222222" || + c.Validator != "mesh-host's validator of "+aRelease+" at 222222222222" { t.Fatalf("the verdict names what it read beside it as %v at %v", c.Beside, c.BesideAt) } } + +// **Clause 5** (ADR 0358 §5): a release branch's own pull request into its trunk is no release-branch check: the forge +// is asked nothing, the request names no release branch, and the repositories beside it are read as for any trunk +// pull request. Its delivery group's composed check is asked as for any other (askGroupCheck, unchanged). +func TestAReleaseBranchsPullRequestIntoItsTrunkIsCheckedAsAnyTrunkPullRequest(t *testing.T) { + entries := theCore() + asked := aForgeWithBranches(t, map[string][]string{"novox/mesh-controller": {"main", aRelease}}, nil) + for _, repo := range []string{"mesh-catalog", "mesh-controller", "mesh-host"} { + p := link.PullUpdated{Owner: "novox", Repo: repo, Number: 400, Head: aRelease, Base: "main", Commit: "abc", + Paths: []string{"modules/gitea/index.ts"}} + got, err := releaseBranchOf(t.Context(), p, pullScope(p, entries, nil, nil), entries) + if err != nil || got != "" || *asked != 0 { + t.Fatalf("%s's release branch into main read as release %q (%v) after %d question(s)", repo, got, err, *asked) + } + } + clone := func(s inventory.Source) (string, error) { return "http://git/" + s.Repository + ".git", nil } + beside, err := besideOf(entries, func(module string) string { return "running-" + module }, clone, "") + if err != nil { + t.Fatal(err) + } + for dir, b := range beside { + if b.Ref == aRelease || b.Else != "" { + t.Fatalf("beside a trunk pull request %s is read at %+v", dir, b) + } + } +} diff --git a/internal/builder/check.go b/internal/builder/check.go index 1a9d32bb..ce95febc 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -198,6 +198,10 @@ type CheckVerdict struct { Release string JudgedBy string BesideAt map[string]string + // Validator says which node-engine validator the judge of a release-branch check was built with (ADR 0358 §2): + // 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 } // Layer is one layer of a check, judged. @@ -384,7 +388,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 = spec.Release, judged.JudgedBy, judged.BesideAt + v.Release, v.JudgedBy, v.BesideAt, v.Validator = spec.Release, judged.JudgedBy, judged.BesideAt, judged.Validator return v } // **What the merge would move is built from a checkout nothing of the check touches** (novox/hq issue 703): the @@ -594,8 +598,10 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry judgedBy = tree } judged.JudgedBy = headAt(ctx, run, judgedBy) - fmt.Fprintf(&out, "--- %s\n", releaseWords(spec.Release, judged.JudgedBy, judged.BesideAt, judged.Beside)) - say("check", "%s", releaseWords(spec.Release, judged.JudgedBy, judged.BesideAt, judged.Beside)) + 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) } verdictFile := filepath.Join(root, "verdict.json") env := []string{EnvFacts + "=" + factsFile, EnvGate + "=" + gate, EnvGateStore + "=" + storeURL, @@ -1209,13 +1215,27 @@ func fullHeadAt(ctx context.Context, run Runner, tree string) string { return strings.TrimSpace(out) } +// releaseValidator says which node-engine validator a release-branch check's judge is built with (novox/hq ADR 0358 +// §2), by the rule judgeFor builds it by. +func releaseValidator(spec CheckSpec, at, commits map[string]string) string { + switch { + case spec.Judge == "self": + return "the validator the change's own controller vendors" + case spec.Judge == "validator": + return "the change's own validator" + case at["mesh-host"] == spec.Release: + return fmt.Sprintf("mesh-host's validator of %s at %s", spec.Release, commits["mesh-host"]) + } + return fmt.Sprintf("the validator %s's controller vendors (mesh-host has no branch %s)", spec.Release, spec.Release) +} + // releaseWords is how a release-branch check says what judged it and what it read (novox/hq ADR 0358 §4). -func releaseWords(release, judgedBy string, at, commits map[string]string) string { +func releaseWords(release, 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", release, judgedBy) + said := fmt.Sprintf("judged for the release branch %s by its controller at %s, with %s", release, judgedBy, validator) if len(read) > 0 { said += "; read beside it: " + strings.Join(read, ", ") } diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index a30406cd..14e97a4f 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -239,6 +239,19 @@ func TestACataloguePullRequestIntoAReleaseBranchIsJudgedByThatBranchsController( if !strings.Contains(v.Report, "read beside it: ") || !strings.Contains(v.Report, "mesh-host "+releaseBranch+" at "+hostRelease[:12]) { t.Fatalf("the report does not say what was read beside it:\n%s", v.Report) } + // It says which validator judged: mesh-host's of the release branch, with its commit. + if want := "mesh-host's validator of " + releaseBranch + " at " + hostRelease[:12]; v.Validator != want || + !strings.Contains(v.Report, want) { + t.Fatalf("the verdict says the validator was %q, not %q:\n%s", v.Validator, want, v.Report) + } + // Each repository read beside it is named with the branch and the commit read. + for dir := range beside { + if v.BesideAt[dir] == "" || v.Beside[dir] == "" || + !strings.Contains(v.Report, dir+" "+v.BesideAt[dir]+" at "+v.Beside[dir]) { + t.Fatalf("%s is not named with its branch and commit: at %q, commit %q\n%s", dir, v.BesideAt[dir], + v.Beside[dir], v.Report) + } + } if n := builds(t, built); n != 1 { t.Fatalf("the judge was built %d times for one check", n) } @@ -306,3 +319,65 @@ func TestAPullRequestIntoTheTrunkIsJudgedAsBefore(t *testing.T) { t.Fatalf("mesh-host was read at %s, not the commit asked %s", v.Beside["mesh-host"], hostMain[:12]) } } + +// **Without mesh-host's release branch, the release branch's controller keeps the validator it vendors** (ADR 0358 +// §2), and the verdict says so. +func TestWithoutTheNodeEnginesReleaseBranchTheJudgeKeepsItsVendoredValidator(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + registry := aFactsRegistry(t) + beside, controllerRelease, _, _ := theReleasedMesh(t) + host, hostMain, _ := aRepositoryWithARelease(t, aHostSaying("main's validator"), nil) + beside["mesh-host"] = Beside{Repository: host, Ref: releaseBranch, Else: "main"} + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, releaseBranch, map[string]string{"modules/gitea/index.ts": "x"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-vendored", Repository: catalogue, Ref: head, + Owner: "novox", Repo: "mesh-catalog", Number: 286, Base: releaseBranch, Release: releaseBranch, + JudgeCache: t.TempDir(), Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Beside: beside, + Toolchain: "golang"}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || !strings.HasSuffix(v.Gate.Summary, "the release branch's controller with the vendored validator") { + t.Fatalf("without mesh-host's release branch the judge did not keep its vendored validator: %+v\n%s", v.Gate, v.Report) + } + if v.JudgedBy != controllerRelease[:12] || v.BesideAt["mesh-host"] != "main" || v.Beside["mesh-host"] != hostMain[:12] { + t.Fatalf("judged by %q, mesh-host read at %q (%s)", v.JudgedBy, v.BesideAt["mesh-host"], v.Beside["mesh-host"]) + } + if !strings.Contains(v.Validator, "controller vendors") || !strings.Contains(v.Report, v.Validator) { + t.Fatalf("the verdict does not say the vendored validator judged: %q\n%s", v.Validator, v.Report) + } +} + +// **Clause 5** (ADR 0358 §5): a release branch's own pull request into its trunk is checked as any pull request into a +// trunk — by the controller beside it as the mesh runs it, the repositories at what a trunk check reads — and its +// verdict names no release branch. +func TestAReleaseBranchsPullRequestIntoItsTrunkIsJudgedByTheControllerTheMeshRuns(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + registry := aFactsRegistry(t) + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("the controller the mesh runs"), + aJudgeOf("the release branch's controller")) + host, hostMain, _ := aRepositoryWithARelease(t, aHostSaying("main's validator"), + aHostSaying("the release branch's validator")) + catalogue, _, releaseHead := aRepositoryWithARelease(t, map[string]string{ + "modules/gitea/module.json": `{"module":"gitea"}`, CheckScript: "true\n"}, + map[string]string{"modules/gitea/index.ts": "released"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-release-into-trunk", Repository: catalogue, + Ref: releaseHead, Owner: "novox", Repo: "mesh-catalog", Number: 300, Base: "main", JudgeCache: t.TempDir(), + Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Toolchain: "golang", + Beside: map[string]Beside{"mesh-controller": {Repository: controller, Ref: controllerMain}, + "mesh-host": {Repository: host, Ref: hostMain}}}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Summary != "the controller the mesh runs with the vendored validator" { + t.Fatalf("the release branch's pull request into its trunk was not judged by the controller the mesh runs: %+v\n%s", + v.Gate, v.Report) + } + if v.Release != "" || v.JudgedBy != "" || v.BesideAt != nil || v.Validator != "" || + strings.Contains(v.Report, "release branch") || strings.Contains(v.Gate.Summary, "[release") { + t.Fatalf("its verdict names a release branch: %q %q %v %q", v.Release, v.JudgedBy, v.BesideAt, v.Validator) + } + if v.Beside["mesh-host"] != hostMain[:12] || v.Beside["mesh-controller"] != controllerMain[:12] { + t.Fatalf("beside it were read %v, not the trunk's %s and %s", v.Beside, controllerMain[:12], hostMain[:12]) + } +} diff --git a/internal/link/build.go b/internal/link/build.go index e5ddafc4..fa837d3f 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -297,6 +297,8 @@ type CheckOutcome struct { Release string `json:"release,omitempty"` JudgedBy string `json:"judged-by,omitempty"` 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"` } // 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 30265231..83ba26c6 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -299,6 +299,8 @@ type Checked struct { Release string `json:"release,omitempty"` JudgedBy string `json:"judged-by,omitempty"` 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"` // 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"` -- 2.54.0 From 48b2906b5ce4f826fab760e834320ef5c7273b24 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 13:13:30 +0000 Subject: [PATCH 3/8] Keep mesh-lab at its trunk and build the release judge with a cache of its own Review of #329: mesh-lab's replays run with the container runtime's socket, so a same-named branch must never be what runs there; the shared Go cache and HOME are writable by a pull request's own script, so the judge kept for a head must not be built from them (issue 704, issue 482). A forge answer naming no branch is an error, and a trunk is the branch a repository follows, not a name. Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-controller/checks.go | 41 +++++++++--- cmd/mesh-controller/release_branch_test.go | 72 +++++++++++++++++++++- internal/builder/check.go | 15 ++++- internal/builder/release_branch_test.go | 33 +++++++++- 4 files changed, 147 insertions(+), 14 deletions(-) diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index e51c97c5..866c2988 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -276,7 +276,8 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop // besideOf is what a check reads beside its own repository, by the directory it is found under: each core repository // by the module the mesh builds from it, at besideRefs — or, for a pull request into a release branch (novox/hq ADR // 0358 §3), at its branch of that name where it has one and at its trunk otherwise, the controller's main kept for -// its judge. running is the commit the mesh runs of a module. +// its judge, and mesh-lab at its trunk whatever the base: its replays run with the container runtime's socket. running +// is the commit the mesh runs of a module. func besideOf(entries []inventory.Entry, running func(module string) string, clone func(inventory.Source) (string, error), release string) (map[string]link.CheckedOut, error) { beside := map[string]link.CheckedOut{} @@ -312,7 +313,9 @@ func besideOf(entries []inventory.Entry, running func(module string) string, if url, err := clone(inventory.Source{Seat: e.Source.Seat, Repository: siblingOf(e.Source.Repository, sibling)}); err == nil { b := link.CheckedOut{Repository: url, Ref: refs[sibling]} - if release != "" { + // **mesh-lab is read at its trunk, always** (ADR 0358): its replays run with the container + // runtime's socket, so they are only ever reviewed code — never a branch anyone may create. + if release != "" && sibling != "mesh-lab" { b = link.CheckedOut{Repository: url, Ref: release, Else: "main"} } beside[sibling] = b @@ -326,15 +329,15 @@ func besideOf(entries []inventory.Entry, running func(module string) string, // releaseBranchOf is the release branch a pull request merges into (novox/hq ADR 0358 §1): its base, when that is not // its repository's trunk and mesh-controller has a branch of that name; empty otherwise, and the pull request is -// checked as before it. **A base that is a trunk asks nothing**: main, the branch a module built from the repository -// follows, or mesh-controller's own trunk. Any other base is asked of the forge once, and an answer it cannot give is -// an error, never read as "no release branch". +// checked as before it. **A base that is a trunk asks nothing**: the branch a module built from the repository follows +// (main when it records none), or mesh-controller's own trunk. Any other base is asked of the forge once, and an +// answer it cannot give is an error, never read as "no release branch". func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope, entries []inventory.Entry) (string, error) { base := p.Base - if base == "" || base == "main" { + if base == "" { return "", nil } - if scope.From != nil && base == followedBranch(scope.From.Source.Ref) { + if scope.From != nil && base == trunkOf(scope.From.Source) { return "", nil } var controller *inventory.Entry @@ -344,7 +347,7 @@ func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope, break } } - if controller == nil || base == followedBranch(controller.Source.Ref) { + if controller == nil || base == trunkOf(controller.Source) { return "", nil } if !tryRef.MatchString(base) || strings.Contains(base, "..") { @@ -365,6 +368,14 @@ func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope, return base, nil } +// trunkOf is the branch a module's source follows: main when it records none, or records a commit. +func trunkOf(s inventory.Source) string { + if t := followedBranch(s.Ref); t != "" { + return t + } + return "main" +} + // sourceName is the name of a recorded repository, a path on the git seat or a URL: novox/mesh-host → mesh-host. func sourceName(repository string) string { parts := strings.Split(strings.Trim(strings.TrimSuffix(repository, ".git"), "/"), "/") @@ -387,6 +398,13 @@ var forgeHasBranch = func(ctx context.Context, owner, repo, branch string) (bool if err != nil { return false, err } + return branchAnswered(answer, branch) +} + +// branchAnswered reads the forge's answer about a branch: a 404 is "no such branch"; an answer naming the branch is +// "it has it"; anything else — another error, an unreadable answer, one naming no branch or another — is an error, +// never read as "no release branch". +func branchAnswered(answer link.Answer, branch string) (bool, error) { if answer.Error != "" { if strings.Contains(answer.Error, " 404 ") { return false, nil @@ -399,9 +417,12 @@ var forgeHasBranch = func(ctx context.Context, owner, repo, branch string) (bool } `json:"result"` } if err := json.Unmarshal(answer.Result, &found); err != nil { - return false, err + return false, fmt.Errorf("the forge's answer about branch %s cannot be read: %w", branch, err) } - return found.Result.Name == branch, nil + if found.Result.Name != branch { + return false, fmt.Errorf("the forge answered about branch %s naming %q", branch, found.Result.Name) + } + return true, nil } // namedForTheCheck are the builds earlier checks of the same pull request at the same head named, read once for the diff --git a/cmd/mesh-controller/release_branch_test.go b/cmd/mesh-controller/release_branch_test.go index e6ddf276..52cf43d8 100644 --- a/cmd/mesh-controller/release_branch_test.go +++ b/cmd/mesh-controller/release_branch_test.go @@ -125,8 +125,9 @@ func TestBesideAReleaseBranchCheckEachRepositoryIsReadAtThatBranchElseItsTrunk(t "mesh-host": at("mesh-host", aRelease, "main"), "mesh-tools": at("mesh-tools", aRelease, "main"), "mesh-catalog": at("mesh-catalog", aRelease, "main"), - "mesh-lab": at("mesh-lab", aRelease, "main"), - "mesh-sdk": at("mesh-sdk", aRelease, "main"), + // mesh-lab at its trunk, always: its replays run with the container runtime's socket. + "mesh-lab": at("mesh-lab", "main", ""), + "mesh-sdk": at("mesh-sdk", aRelease, "main"), } if !reflect.DeepEqual(beside, want) { t.Fatalf("beside a release-branch check:\n got %v\nwant %v", beside, want) @@ -193,3 +194,70 @@ func TestAReleaseBranchsPullRequestIntoItsTrunkIsCheckedAsAnyTrunkPullRequest(t } } } + +// **A forge answer that names no branch is an error**, never "no release branch"; a 404 is "no such branch". +func TestTheForgesAnswerAboutABranchIsReadStrictly(t *testing.T) { + for _, c := range []struct { + what string + answer link.Answer + has bool + fails bool + }{ + {"the branch named", link.Answer{Result: []byte(`{"result":{"name":"` + aRelease + `"}}`)}, true, false}, + {"not found", link.Answer{Error: "Gitea API /repos/novox/mesh-controller/branches/x: 404 {\"message\":\"not found\"}"}, + false, false}, + {"an empty branch name", link.Answer{Result: []byte(`{"result":{"name":""}}`)}, false, true}, + {"no result", link.Answer{Result: []byte(`{}`)}, false, true}, + {"another branch named", link.Answer{Result: []byte(`{"result":{"name":"main"}}`)}, false, true}, + {"unreadable", link.Answer{Result: []byte(`not json`)}, false, true}, + {"another error", link.Answer{Error: "Gitea API: 500 internal"}, false, true}, + } { + has, err := branchAnswered(c.answer, aRelease) + if has != c.has || (err != nil) != c.fails { + t.Errorf("%s: has %v, error %v", c.what, has, err) + } + } +} + +// **The trunk is the repository's own, not a name**: a repository whose trunk is not main, one that follows main, and +// a controller whose trunk is not main. +func TestATrunkIsTheBranchTheRepositoryFollows(t *testing.T) { + asked := aForgeWithBranches(t, map[string][]string{"novox/mesh-controller": {"main", aRelease, "master"}}, nil) + legacy := fromRepo("legacy", "novox/legacy", "") + legacy.Source.Ref = "master" + onMain := fromRepo("photos", "novox/photos", "") + onMain.Source.Ref = "main" + entries := append(theCore(), legacy, onMain) + for _, c := range []struct { + what, repo, base, release string + asks int + }{ + {"a repository whose trunk is master, into master", "legacy", "master", "", 0}, + {"a repository whose trunk is master, into the release branch", "legacy", aRelease, aRelease, 1}, + {"a repository whose module follows main, into main", "photos", "main", "", 0}, + {"a repository whose module follows main, into the release branch", "photos", aRelease, aRelease, 1}, + } { + *asked = 0 + p := link.PullUpdated{Owner: "novox", Repo: c.repo, Base: c.base, Commit: "abc", Paths: []string{"x"}} + got, err := releaseBranchOf(t.Context(), p, pullScope(p, entries, nil, nil), entries) + if err != nil || got != c.release || *asked != c.asks { + t.Errorf("%s: release %q (%v) after %d question(s), not %q after %d", c.what, got, err, *asked, c.release, c.asks) + } + } + + // A controller whose trunk is not main: main may be a release branch there, and its own trunk never is. + core := theCore() + core[0].Source.Ref = "trunk" + for _, c := range []struct { + base, release string + asks int + }{{"trunk", "", 0}, {"main", "main", 1}} { + *asked = 0 + p := link.PullUpdated{Owner: "novox", Repo: "hq", Base: c.base, Commit: "abc", Paths: []string{"x"}} + got, err := releaseBranchOf(t.Context(), p, pullScope(p, core, nil, nil), core) + if err != nil || got != c.release || *asked != c.asks { + t.Errorf("hq into %s, the controller following trunk: release %q (%v) after %d question(s)", c.base, got, err, + *asked) + } + } +} diff --git a/internal/builder/check.go b/internal/builder/check.go index ce95febc..240a0901 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -429,6 +429,11 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if dir == name || !safeName.MatchString(dir) { continue } + if dir == "mesh-lab" && b.Else != "" { + // **mesh-lab is read at its trunk, always** (novox/hq ADR 0358): its replays run with the container + // runtime's socket, so a branch of a release's name — which anyone may create — is never what runs there. + b.Ref, b.Else = b.Else, "" + } if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil { return CheckVerdict{}, fmt.Errorf("beside it, %w", err) } @@ -1130,8 +1135,12 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s } } } + // **Built with a cache of its own, never the workspace's** (novox/hq issue 482): the workspace's Go cache and HOME are + // written by every pull request's own script, so a poisoned cache entry or Go environment file would otherwise be + // built into the judge kept for this head. A fresh cache under this check's root, made before anything of the + // change runs, and no Go environment file. if _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), - []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")}, + []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOENV=off", "GOCACHE=" + judgeGoCache(root)}, "go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil { return "", "", fmt.Errorf("the controller of %s does not build%s: %w", spec.Release, with, err) } @@ -1141,6 +1150,10 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s return bin, dir, nil } +// judgeGoCache is the Go cache a release branch's judge is built with: fresh for each check, under its root, which is +// emptied before the check clones anything — never the workspace's shared cache. +func judgeGoCache(root string) string { return filepath.Join(root, "judge-go-cache") } + // judgesKept is how many release-branch judges a judge cache keeps: a head moved past is not built from again. const judgesKept = 4 diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index 14e97a4f..1ca1609b 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -143,7 +143,7 @@ run) done if [ -n "$detached" ]; then echo a-container; exit 0; fi shift - if [ "$1" = go ]; then echo "$*" >> "` + built + `"; fi + if [ "$1" = go ]; then echo "GOCACHE=$GOCACHE GOENV=$GOENV $*" >> "` + built + `"; fi cd "$workdir" && exec "$@" ;; esac exit 1 @@ -255,6 +255,13 @@ func TestACataloguePullRequestIntoAReleaseBranchIsJudgedByThatBranchsController( if n := builds(t, built); n != 1 { t.Fatalf("the judge was built %d times for one check", n) } + // **The judge is built with a cache no pull request's script can write** (issue 482): a fresh one under the + // check's own root, never the workspace's shared go-cache, and no Go environment file from the workspace's HOME. + said, _ := os.ReadFile(built) + if !strings.Contains(string(said), "/check/judge-go-cache GOENV=off go build") || + strings.Contains(string(said), "/go-cache GOENV") { + t.Fatalf("the release judge was built with the workspace's cache or Go environment:\n%s", said) + } // **Built once per release-branch head**: a second check while that head stands takes the judge kept. v = check("check-704-catalogue-again") @@ -381,3 +388,27 @@ func TestAReleaseBranchsPullRequestIntoItsTrunkIsJudgedByTheControllerTheMeshRun t.Fatalf("beside it were read %v, not the trunk's %s and %s", v.Beside, controllerMain[:12], hostMain[:12]) } } + +// **mesh-lab is read at its trunk, always** (ADR 0358): its replays run with the container runtime's socket, so a +// branch of the release's name in mesh-lab — which anyone may create — is never what a check reads. +func TestAReleaseBranchCheckReadsMeshLabAtItsTrunkEvenWhenItHasThatBranch(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + registry := aFactsRegistry(t) + beside, _, _, _ := theReleasedMesh(t) + lab, labMain, labRelease := aRepositoryWithARelease(t, map[string]string{"README": "the lab"}, + map[string]string{"README": "a branch anyone could push"}) + beside["mesh-lab"] = Beside{Repository: lab, Ref: releaseBranch, Else: "main"} + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, releaseBranch, map[string]string{"modules/gitea/index.ts": "x"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-lab", Repository: catalogue, Ref: head, + Owner: "novox", Repo: "mesh-catalog", Number: 286, Base: releaseBranch, Release: releaseBranch, + JudgeCache: t.TempDir(), Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Beside: beside, + Toolchain: "golang"}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.BesideAt["mesh-lab"] != "main" || v.Beside["mesh-lab"] != labMain[:12] || v.Beside["mesh-lab"] == labRelease[:12] { + t.Fatalf("mesh-lab was read at %q (%s), not its trunk main at %s", v.BesideAt["mesh-lab"], v.Beside["mesh-lab"], + labMain[:12]) + } +} -- 2.54.0 From b7084a19e9bf1547766503ebe85b44db8e70448d Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 13:30:46 +0000 Subject: [PATCH 4/8] Build every judge where no pull request can reach, and never read mesh-lab at a release Review of #329: a judge built with HOME in the workspace runs git under a configuration a pull request's script can plant, so a hook could rewrite the judge's sources before they compile and the result be kept under an honest key (issue 482, issue 704). Every judge now builds with its own HOME, configuration and Go cache outside the workspace, no git configuration and no VCS stamping. Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-controller/checks.go | 2 +- cmd/mesh-controller/release_branch_test.go | 10 ++ internal/builder/check.go | 76 ++++++++---- internal/builder/release_branch_test.go | 137 ++++++++++++++++++++- 4 files changed, 200 insertions(+), 25 deletions(-) diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 866c2988..1c7eefda 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -370,7 +370,7 @@ func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope, // trunkOf is the branch a module's source follows: main when it records none, or records a commit. func trunkOf(s inventory.Source) string { - if t := followedBranch(s.Ref); t != "" { + if t := strings.TrimSpace(followedBranch(s.Ref)); t != "" { return t } return "main" diff --git a/cmd/mesh-controller/release_branch_test.go b/cmd/mesh-controller/release_branch_test.go index 52cf43d8..298b1588 100644 --- a/cmd/mesh-controller/release_branch_test.go +++ b/cmd/mesh-controller/release_branch_test.go @@ -261,3 +261,13 @@ func TestATrunkIsTheBranchTheRepositoryFollows(t *testing.T) { } } } + +// trunkOf: the branch a source follows, and main when it records none or records a commit (issue 215's rule). +func TestATrunkIsMainWhenTheSourceRecordsNone(t *testing.T) { + for ref, want := range map[string]string{"": "main", "main": "main", "master": "master", "nox-mesh": "nox-mesh", + "0123456789abcdef0123456789abcdef01234567": "main", " ": "main"} { + if got := trunkOf(inventory.Source{Ref: ref}); got != want { + t.Errorf("a source recording %q follows %q, not %q", ref, got, want) + } + } +} diff --git a/internal/builder/check.go b/internal/builder/check.go index 240a0901..202a9abf 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -429,10 +429,17 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if dir == name || !safeName.MatchString(dir) { continue } - if dir == "mesh-lab" && b.Else != "" { - // **mesh-lab is read at its trunk, always** (novox/hq ADR 0358): its replays run with the container - // runtime's socket, so a branch of a release's name — which anyone may create — is never what runs there. - b.Ref, b.Else = b.Else, "" + if dir == "mesh-lab" { + // **mesh-lab is never read at a release branch** (novox/hq ADR 0358): its replays run with the container + // runtime's socket, so a branch of a release's name — which anyone may create — is never what runs there, + // whether it is asked with a fallback or without. + if b.Else != "" || (spec.Release != "" && b.Ref == spec.Release) { + b.Ref = b.Else + if b.Ref == "" { + b.Ref = "main" + } + } + b.Else = "" } if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil { return CheckVerdict{}, fmt.Errorf("beside it, %w", err) @@ -565,21 +572,47 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry user = fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid()) } in := func(image, dir string, env []string, command ...string) []string { + home := workspace + for _, e := range env { + if strings.HasPrefix(e, "HOME=") { + home = strings.TrimPrefix(e, "HOME=") + } + } // As the builder itself: what a check writes into the workspace is the builder's to remove. args := []string{"run", "--rm", "--network", "host", "--volume", workspace + ":" + workspace, "--workdir", dir, - "--user", user, "--env", "HOME=" + workspace, + "--user", user, "--env", "HOME=" + home, // The check's own checkouts, whoever cloned them: git in a container of another user than the // one that cloned refuses a repository it does not own ("dubious ownership"), and Go's build // stamps the version from git — a judge that would not build for want of it. "--env", "GIT_CONFIG_COUNT=1", "--env", "GIT_CONFIG_KEY_0=safe.directory", "--env", "GIT_CONFIG_VALUE_0=*"} for _, e := range env { - args = append(args, "--env", e) + if !strings.HasPrefix(e, "HOME=") { + args = append(args, "--env", e) + } } return append(append(args, image), command...) } inToolchain := func(dir string, env []string, command ...string) []string { return in(spec.Toolchain, dir, env, command...) } + // **Every judge is built where no pull request can reach** (novox/hq issue 482, ADR 0358 review): the workspace's + // Go cache, HOME and git configuration are written by pull requests' own scripts, and git runs during a Go build + // to stamp its version — a planted hook would rewrite the judge's sources before they compile. So a judge is built + // with a HOME, configuration and Go cache of its own, outside the workspace and made fresh for this check, no + // global or system git configuration, no Go environment file, and no version stamping from git. + judgeHome, err := os.MkdirTemp("", "mesh-check-judge-") + if err != nil { + return CheckVerdict{}, err + } + defer os.RemoveAll(judgeHome) + if withinDir(workspace, judgeHome) { + return CheckVerdict{}, fmt.Errorf("the temporary directory %s is inside the workspace %s, which a check's "+ + "containers mount: no judge is built where a pull request could reach it", judgeHome, workspace) + } + inJudgeBuild := func(dir string, _ []string, command ...string) []string { + args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome), command...) + return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome}, args[1:]...) + } var out tail // running runs one container of the check, its output into the report, in a process group of its own. running := func(args []string) error { @@ -591,7 +624,7 @@ 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, inToolchain, say) + gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, inJudgeBuild, say) if err != nil { return CheckVerdict{}, err } @@ -1046,8 +1079,7 @@ func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, t switch spec.Judge { case "self": bin := filepath.Join(root, "bin", "judge-of-itself") - if _, err := labelled(ctx, root, "docker", inToolchain(tree, - []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")}, + if _, err := labelled(ctx, root, "docker", inToolchain(tree, nil, "go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil { return "", "the change's controller, which judges itself, does not build: " + firstLine(err.Error()), nil } @@ -1080,8 +1112,7 @@ func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, t } } withValidator := filepath.Join(root, "bin", "judge-with-this-validator") - if _, err := labelled(ctx, root, "docker", inToolchain(filepath.Join(root, judgeTree), - []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")}, + if _, err := labelled(ctx, root, "docker", inToolchain(filepath.Join(root, judgeTree), nil, "go", "build", "-o", withValidator, "./cmd/mesh-controller")...); err != nil { if spec.Release != "" { return "", spec.Release + "'s controller does not build with this change's validator: " + @@ -1135,12 +1166,8 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s } } } - // **Built with a cache of its own, never the workspace's** (novox/hq issue 482): the workspace's Go cache and HOME are - // written by every pull request's own script, so a poisoned cache entry or Go environment file would otherwise be - // built into the judge kept for this head. A fresh cache under this check's root, made before anything of the - // change runs, and no Go environment file. - if _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), - []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOENV=off", "GOCACHE=" + judgeGoCache(root)}, + // Built where no pull request reaches (JudgeBuildEnv): a judge kept for this head is the head's, nothing else. + if _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), nil, "go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil { return "", "", fmt.Errorf("the controller of %s does not build%s: %w", spec.Release, with, err) } @@ -1150,9 +1177,15 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s return bin, dir, nil } -// judgeGoCache is the Go cache a release branch's judge is built with: fresh for each check, under its root, which is -// emptied before the check clones anything — never the workspace's shared cache. -func judgeGoCache(root string) string { return filepath.Join(root, "judge-go-cache") } +// JudgeBuildEnv is the environment every judge is built in (novox/hq issue 482): home, a directory outside the +// workspace made fresh for the check, as HOME, configuration and Go cache; no global or system git configuration; no +// Go environment file; and no version stamped from git, so no git runs during the build at all. +func JudgeBuildEnv(home string) []string { + return []string{"HOME=" + home, "XDG_CONFIG_HOME=" + filepath.Join(home, ".config"), + "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "GOENV=off", + "GOCACHE=" + filepath.Join(home, "go-cache"), "GOPATH=" + filepath.Join(home, "go"), + "CGO_ENABLED=0", "GOFLAGS=-mod=vendor -buildvcs=false", "GOPROXY=off"} +} // judgesKept is how many release-branch judges a judge cache keeps: a head moved past is not built from again. const judgesKept = 4 @@ -1514,8 +1547,7 @@ func judge(ctx context.Context, run, plain Runner, root string, inToolchain func say func(step, format string, args ...any)) (string, string, error) { bin := filepath.Join(root, "bin", "mesh-controller") build := func(dir string) error { - _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), - []string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")}, + _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), nil, "go", "build", "-o", bin, "./cmd/mesh-controller")...) return err } diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index 1ca1609b..d5509687 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -144,6 +144,7 @@ run) if [ -n "$detached" ]; then echo a-container; exit 0; fi shift if [ "$1" = go ]; then echo "GOCACHE=$GOCACHE GOENV=$GOENV $*" >> "` + built + `"; fi + if [ "$1" = go ] && [ "$2" = build ] && [ -n "$JUDGE_HOMES" ]; then echo "$HOME" >> "$JUDGE_HOMES"; fi cd "$workdir" && exec "$@" ;; esac exit 1 @@ -258,8 +259,8 @@ func TestACataloguePullRequestIntoAReleaseBranchIsJudgedByThatBranchsController( // **The judge is built with a cache no pull request's script can write** (issue 482): a fresh one under the // check's own root, never the workspace's shared go-cache, and no Go environment file from the workspace's HOME. said, _ := os.ReadFile(built) - if !strings.Contains(string(said), "/check/judge-go-cache GOENV=off go build") || - strings.Contains(string(said), "/go-cache GOENV") { + if !strings.Contains(string(said), "/mesh-check-judge-") || !strings.Contains(string(said), "GOENV=off go build") || + strings.Contains(string(said), "/check/../go-cache") { t.Fatalf("the release judge was built with the workspace's cache or Go environment:\n%s", said) } @@ -412,3 +413,135 @@ func TestAReleaseBranchCheckReadsMeshLabAtItsTrunkEvenWhenItHasThatBranch(t *tes labMain[:12]) } } + +// aPlantedWorkspace is a workspace a pull request's own script has written into, as it can: a git configuration in its +// HOME whose fsmonitor hook says each directory git runs in, a git configuration under XDG_CONFIG_HOME doing the same, +// and a Go environment file that would hand the judge's build a tool of its own. It answers the workspace and the +// file the hook writes to. +func aPlantedWorkspace(t *testing.T) (string, string) { + t.Helper() + workspace, logs := t.TempDir(), t.TempDir() + ran := filepath.Join(logs, "hook-ran") + hook := filepath.Join(workspace, "hook.sh") + files := map[string]string{ + hook: "#!/bin/sh\necho \"$PWD\" >> " + ran + "\n", + filepath.Join(workspace, ".gitconfig"): "[core]\n\tfsmonitor = " + hook + "\n", + filepath.Join(workspace, ".config", "git", "config"): "[core]\n\tfsmonitor = " + hook + "\n", + filepath.Join(workspace, ".config", "go", "env"): "GOFLAGS=-toolexec=" + hook + "\n", + } + for name, body := range files { + if err := os.MkdirAll(filepath.Dir(name), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + return workspace, ran +} + +// judgeRanTheHook says whether the planted hook ran in a judge's tree: the controller beside the check, or the +// change's own when it judges itself. +func judgeRanTheHook(t *testing.T, ran string) string { + t.Helper() + body, _ := os.ReadFile(ran) + for _, line := range strings.Split(string(body), "\n") { + if strings.HasSuffix(line, "/check/mesh-controller") { + return string(body) + } + } + return "" +} + +// **Every judge is built where no pull request reaches** (issue 482, ADR 0358 review): a git configuration with a +// hook, and a Go environment file, planted in the workspace by an earlier pull request's script, do not run during +// the release judge's build — while the pull request's own script, in the workspace, still meets them. +func TestAHookPlantedInTheWorkspaceDoesNotRunDuringTheReleaseJudgesBuild(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + homes := filepath.Join(t.TempDir(), "homes") + t.Setenv("JUDGE_HOMES", homes) + registry := aFactsRegistry(t) + workspace, ran := aPlantedWorkspace(t) + beside, _, _, _ := theReleasedMesh(t) + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "git status >/dev/null 2>&1; true\n"}, releaseBranch, map[string]string{"modules/gitea/index.ts": "x"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-planted", Repository: catalogue, Ref: head, + Owner: "novox", Repo: "mesh-catalog", Number: 286, Base: releaseBranch, Release: releaseBranch, + JudgeCache: t.TempDir(), Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Beside: beside, + Toolchain: "golang"}, workspace, registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Verdict != "pass" { + t.Fatalf("the check did not judge: %+v\n%s", v.Gate, v.Report) + } + if body, _ := os.ReadFile(ran); !strings.Contains(string(body), "/check/mesh-catalog") { + t.Fatalf("the planted hook never ran, so this test proves nothing: %q", body) + } + if said := judgeRanTheHook(t, ran); said != "" { + t.Fatalf("the hook planted in the workspace ran during the judge's build:\n%s", said) + } + if body, _ := os.ReadFile(homes); len(body) == 0 || strings.Contains(string(body), workspace) { + t.Fatalf("the judge was built with a HOME in the workspace: %q", body) + } +} + +// The same for the judge built with a node-engine change's own validator, into a release branch (ADR 0358 §2). +func TestAHookPlantedInTheWorkspaceDoesNotRunDuringTheValidatorJudgesBuild(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + homes := filepath.Join(t.TempDir(), "homes") + t.Setenv("JUDGE_HOMES", homes) + registry := aFactsRegistry(t) + workspace, ran := aPlantedWorkspace(t) + beside, _, _, _ := theReleasedMesh(t) + delete(beside, "mesh-host") + files := aHostSaying("the release branch's validator") + files["module.json"] = `{"module":"mesh-host"}` + files[CheckScript] = "git status >/dev/null 2>&1; true\n" + host, head := aPullRequestInto(t, files, releaseBranch, aHostSaying("this change's validator")) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-planted-validator", Repository: host, Ref: head, + Owner: "novox", Repo: "mesh-host", Number: 111, Base: releaseBranch, Release: releaseBranch, Judge: "validator", + JudgeCache: t.TempDir(), Paths: []string{"validate/v.go"}, Modules: []string{"mesh-host"}, Beside: beside, + Toolchain: "golang"}, workspace, registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || !strings.HasSuffix(v.Gate.Summary, "the release branch's controller with this change's validator") { + t.Fatalf("a node-engine change into a release branch was not judged by that branch's controller with its own "+ + "validator: %+v\n%s", v.Gate, v.Report) + } + if body, _ := os.ReadFile(ran); !strings.Contains(string(body), "/check/mesh-host") { + t.Fatalf("the planted hook never ran, so this test proves nothing: %q", body) + } + if said := judgeRanTheHook(t, ran); said != "" { + t.Fatalf("the hook planted in the workspace ran during a judge's build:\n%s", said) + } + if body, _ := os.ReadFile(homes); strings.Count(string(body), "\n") != 2 || strings.Contains(string(body), workspace) { + t.Fatalf("the judges were not both built with a HOME outside the workspace: %q", body) + } +} + +// **mesh-lab is never read at a release branch**, asked with a fallback or without (ADR 0358). +func TestTheBuildSeatNeverReadsMeshLabAtAReleaseBranch(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + registry := aFactsRegistry(t) + lab, labMain, _ := aRepositoryWithARelease(t, map[string]string{"README": "the lab"}, + map[string]string{"README": "a branch anyone could push"}) + for _, asked := range []Beside{{Repository: lab, Ref: releaseBranch}, {Repository: lab, Ref: releaseBranch, Else: "main"}} { + beside, _, _, _ := theReleasedMesh(t) + beside["mesh-lab"] = asked + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, releaseBranch, map[string]string{"modules/gitea/index.ts": "x"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-lab-asked", Repository: catalogue, Ref: head, + Owner: "novox", Repo: "mesh-catalog", Number: 286, Base: releaseBranch, Release: releaseBranch, + JudgeCache: t.TempDir(), Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Beside: beside, + Toolchain: "golang"}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.BesideAt["mesh-lab"] != "main" || v.Beside["mesh-lab"] != labMain[:12] { + t.Fatalf("mesh-lab asked as %+v was read at %q (%s), not main at %s", asked, v.BesideAt["mesh-lab"], + v.Beside["mesh-lab"], labMain[:12]) + } + } +} -- 2.54.0 From 2bee290eea56dc94dd67fe379c59ad35e4e64ef5 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 13:53:05 +0000 Subject: [PATCH 5/8] Keep judge builds warm in the build agent's judge cache, and leave nothing on the host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #329: a judge's home made with a temporary directory inside the build agent is resolved on the host by the container runtime, leaving a cold cache behind on every check, and a kept judge was lost on every redeploy. Judge builds now use the judge cache the build agent declares (MESH_JUDGE_CACHE), mounted into judge builds only: one warm Go cache, the kept judges, and a fresh home per check removed after it (hq ADR 0358 §6, issue 704, issue 482). Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-builder/main.go | 16 ++-- cmd/mesh-builder/release_test.go | 15 +-- internal/builder/check.go | 84 +++++++++++++---- internal/builder/release_branch_test.go | 117 +++++++++++++++++++++++- 4 files changed, 192 insertions(+), 40 deletions(-) diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index fc418367..51dcd1b4 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -24,7 +24,6 @@ import ( "net/url" "os" "os/signal" - "path/filepath" "sort" "strings" "syscall" @@ -314,9 +313,9 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri registry = r.Address } spec := checkSpecOf(request) - // A release branch's judge is kept beside the workspace, never in it: the check's containers mount the - // workspace, and a pull request's script must not plant the judge of the next check (novox/hq ADR 0358 §2). - spec.JudgeCache = judgeCacheOf(workspace) + // The judge cache the build agent declares as its own data, never in the workspace: only judge builds mount + // it, so no pull request's script can write what a judge is built with or kept as (novox/hq ADR 0358 §6). + spec.JudgeCache = judgeCache() // What the merge would move, built by the check for the walk to take (novox/hq ADR 0350 §1): into the // scratch namespace. **Never with the package registry's credential**: this is code nobody has approved // yet, and a recipe could read the credential it is handed (issue 462). A module that resolves packages @@ -476,11 +475,10 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { return spec } -// judgeCacheOf is where a release branch's judge is kept (novox/hq ADR 0358 §2): beside the workspace, outside what -// a check's containers mount. -func judgeCacheOf(workspace string) string { - return filepath.Clean(workspace) + "-judges" -} +// 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. +func judgeCache() string { return os.Getenv("MESH_JUDGE_CACHE") } // triedOf is a try's outcome as its log line says it (novox/hq issue 686): the repository's own check's verdict, // or the check's where that did not run — a head that does not merge — and the lines naming what failed. diff --git a/cmd/mesh-builder/release_test.go b/cmd/mesh-builder/release_test.go index 0b678d16..2eb9c8f3 100644 --- a/cmd/mesh-builder/release_test.go +++ b/cmd/mesh-builder/release_test.go @@ -1,8 +1,6 @@ package main import ( - "path/filepath" - "strings" "testing" "github.com/novox/mesh-controller/internal/link" @@ -10,7 +8,7 @@ import ( // **A pull request into a release branch reaches the check as one** (novox/hq ADR 0358): the build seat hands the // check the release branch and, beside it, the trunk each repository is read at when it has no branch of that name; -// and it keeps the release branch's judge outside the workspace a check's containers mount. +// and it hands the check the judge cache the build agent declares (MESH_JUDGE_CACHE), never a path of its own. func TestTheBuildSeatHandsTheCheckItsReleaseBranch(t *testing.T) { spec := checkSpecOf(link.BuildRequest{ID: "b", Repository: "r", Ref: "abc", Check: &link.CheckRequest{ Owner: "novox", Repo: "mesh-catalog", Base: "feat/57", Release: "feat/57", @@ -18,9 +16,12 @@ func TestTheBuildSeatHandsTheCheckItsReleaseBranch(t *testing.T) { if spec.Release != "feat/57" || spec.Beside["mesh-host"].Ref != "feat/57" || spec.Beside["mesh-host"].Else != "main" { t.Fatalf("the check was handed release %q and mesh-host %+v", spec.Release, spec.Beside["mesh-host"]) } - workspace := filepath.Join(t.TempDir(), "mesh-builder") - cache := judgeCacheOf(workspace) - if rel, err := filepath.Rel(workspace, cache); err != nil || !strings.HasPrefix(rel, "..") { - t.Fatalf("the judge cache %s lies inside the workspace %s, which a check's containers mount", cache, workspace) + t.Setenv("MESH_JUDGE_CACHE", "/var/lib/mesh/build-agent/judge-cache") + if got := judgeCache(); got != "/var/lib/mesh/build-agent/judge-cache" { + t.Fatalf("the judge cache the manifest declares is read as %q", got) + } + t.Setenv("MESH_JUDGE_CACHE", "") + if got := judgeCache(); got != "" { + t.Fatalf("with no judge cache declared, %q is used", got) } } diff --git a/internal/builder/check.go b/internal/builder/check.go index 202a9abf..dcd0f5c2 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -94,9 +94,12 @@ type CheckSpec struct { // the release branch too; a change to the controller still judges itself and a change to the node-engine brings // its own validator. The verdict names the branch, the judge's commit and the branch each beside was read at. Release string - // JudgeCache is a directory outside the workspace where a release branch's judge is kept by the commits it is - // built from, so it is built once per release-branch head (ADR 0358 §2). Outside: the check's containers mount - // the workspace and a pull request's own script must not plant a judge for the next check. Empty keeps none. + // JudgeCache is the judge cache (novox/hq ADR 0358 §6): a directory of the build agent's own, outside the + // workspace, that only judge builds mount — never a check's containers, so no pull request's script can write it. + // It holds the Go build cache every judge is built with (judges stay warm), the release-branch judges kept by the + // commits they are built from (judges/, one per release-branch head), and each judge build's fresh home (homes/, + // removed after the build). Empty: each judge is built cold under the check's own root, which every check empties + // before it clones, and nothing is kept. JudgeCache string // Toolchain is the image a check's Go runs in: the mesh's own Go toolchain, as it holds it. // Toolchains is every toolchain the mesh holds, by language, for a script that declares another. @@ -598,20 +601,21 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry // **Every judge is built where no pull request can reach** (novox/hq issue 482, ADR 0358 review): the workspace's // Go cache, HOME and git configuration are written by pull requests' own scripts, and git runs during a Go build // to stamp its version — a planted hook would rewrite the judge's sources before they compile. So a judge is built - // with a HOME, configuration and Go cache of its own, outside the workspace and made fresh for this check, no - // global or system git configuration, no Go environment file, and no version stamping from git. - judgeHome, err := os.MkdirTemp("", "mesh-check-judge-") + // with a HOME and configuration made fresh for this check and a Go cache only judge builds mount (judgeBuildDirs), + // no global or system git configuration, no Go environment file, and no version stamping from git. + judgeHome, judgeGoCache, err := judgeBuildDirs(spec.JudgeCache, workspace, root, spec.ID) if err != nil { return CheckVerdict{}, err } defer os.RemoveAll(judgeHome) - if withinDir(workspace, judgeHome) { - return CheckVerdict{}, fmt.Errorf("the temporary directory %s is inside the workspace %s, which a check's "+ - "containers mount: no judge is built where a pull request could reach it", judgeHome, workspace) - } inJudgeBuild := func(dir string, _ []string, command ...string) []string { - args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome), command...) - return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome}, args[1:]...) + args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome, judgeGoCache), command...) + if spec.JudgeCache == "" { + return args // under the check's root, in the workspace mounted already + } + // The judge cache's parts this build needs, mounted into judge builds alone (ADR 0358 §6). + return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome, "--volume", + judgeGoCache + ":" + judgeGoCache}, args[1:]...) } var out tail // running runs one container of the check, its output into the report, in a process group of its own. @@ -1153,7 +1157,7 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s with = ", with mesh-host's validator of " + spec.Release + " at " + twelve(host) } bin := filepath.Join(root, "bin", "mesh-controller") - if took := judgeKept(spec.JudgeCache, key, bin); took { + if took := judgeKept(keptJudges(spec.JudgeCache), key, bin); took { say("check", "judged by %s's controller at %s%s, built for an earlier check of that head", spec.Release, twelve(key), with) return bin, dir, nil @@ -1172,21 +1176,63 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s return "", "", fmt.Errorf("the controller of %s does not build%s: %w", spec.Release, with, err) } // Kept before anything of the change runs: nothing a pull request's script writes can reach the cache. - keepJudge(spec.JudgeCache, key, bin, say) + keepJudge(keptJudges(spec.JudgeCache), key, bin, say) say("check", "judged by %s's controller at %s%s", spec.Release, twelve(key), with) return bin, dir, nil } -// JudgeBuildEnv is the environment every judge is built in (novox/hq issue 482): home, a directory outside the -// workspace made fresh for the check, as HOME, configuration and Go cache; no global or system git configuration; no -// Go environment file; and no version stamped from git, so no git runs during the build at all. -func JudgeBuildEnv(home string) []string { +// 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. +func JudgeBuildEnv(home, goCache string) []string { return []string{"HOME=" + home, "XDG_CONFIG_HOME=" + filepath.Join(home, ".config"), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "GOENV=off", - "GOCACHE=" + filepath.Join(home, "go-cache"), "GOPATH=" + filepath.Join(home, "go"), + "GOCACHE=" + goCache, "GOPATH=" + filepath.Join(home, "go"), "CGO_ENABLED=0", "GOFLAGS=-mod=vendor -buildvcs=false", "GOPROXY=off"} } +// judgeBuildDirs are the home and the Go cache a check's judge builds use (ADR 0358 §6). With a judge cache: a home +// made fresh under its homes/ for this check, and its go-cache/, kept warm between checks — both on the host path the +// build agent declares, so the container runtime mounts what the agent sees, and nothing is left in a temporary +// directory of the host. Without one: both under the check's root, which every check empties before it clones. A +// judge cache inside the workspace is refused: a check's containers mount the workspace. +func judgeBuildDirs(cache, workspace, root, id string) (string, string, error) { + if cache == "" { + home, goCache := filepath.Join(root, "judge-home"), filepath.Join(root, "judge-go-cache") + for _, d := range []string{home, goCache} { + if err := os.MkdirAll(d, 0o700); err != nil { + return "", "", err + } + } + return home, goCache, nil + } + if withinDir(workspace, cache) { + return "", "", fmt.Errorf("the judge cache %s is inside the workspace %s, which a check's containers mount: "+ + "no judge is built where a pull request could reach it", cache, workspace) + } + if !safeName.MatchString(id) { + id = "check" + } + home, goCache := filepath.Join(cache, "homes", id), filepath.Join(cache, "go-cache") + if err := os.RemoveAll(home); err != nil { + return "", "", err + } + for _, d := range []string{home, goCache} { + if err := os.MkdirAll(d, 0o700); err != nil { + return "", "", err + } + } + return home, goCache, nil +} + +// keptJudges is where a judge cache keeps the release-branch judges, empty when there is no judge cache. +func keptJudges(cache string) string { + if cache == "" { + return "" + } + return filepath.Join(cache, "judges") +} + // judgesKept is how many release-branch judges a judge cache keeps: a head moved past is not built from again. const judgesKept = 4 diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index d5509687..123c13c0 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -1,6 +1,7 @@ package builder import ( + "fmt" "net" "os" "os/exec" @@ -130,6 +131,7 @@ case "$cmd" in ps|rm|exec) exit 0 ;; port) echo "` + bus.Addr().String() + `"; exit 0 ;; run) + if [ -n "$RUNS" ]; then echo "$*" >> "$RUNS"; fi workdir=""; detached="" while [ $# -gt 0 ]; do case "$1" in @@ -256,12 +258,16 @@ func TestACataloguePullRequestIntoAReleaseBranchIsJudgedByThatBranchsController( if n := builds(t, built); n != 1 { t.Fatalf("the judge was built %d times for one check", n) } - // **The judge is built with a cache no pull request's script can write** (issue 482): a fresh one under the - // check's own root, never the workspace's shared go-cache, and no Go environment file from the workspace's HOME. + // **The judge is built with the judge cache's Go cache** (ADR 0358 §6), which no check's container mounts, and no + // Go environment file. said, _ := os.ReadFile(built) - if !strings.Contains(string(said), "/mesh-check-judge-") || !strings.Contains(string(said), "GOENV=off go build") || - strings.Contains(string(said), "/check/../go-cache") { - t.Fatalf("the release judge was built with the workspace's cache or Go environment:\n%s", said) + if !strings.Contains(string(said), "GOCACHE="+filepath.Join(cache, "go-cache")+" GOENV=off go build") { + t.Fatalf("the release judge was not built with the judge cache's Go cache:\n%s", said) + } + // **Clause 5**: the head was merged onto the release branch's head, which the verdict names as its trunk commit, + // so the release branch's move puts it out of date as a trunk's move does (ADR 0323 §4, the forge's holder). + if releaseHead := gitInRepo(t, catalogue, "rev-parse", releaseBranch); v.Trunk != releaseHead[:12] { + t.Fatalf("the verdict names trunk %q, not the release branch's head %s", v.Trunk, releaseHead[:12]) } // **Built once per release-branch head**: a second check while that head stands takes the judge kept. @@ -545,3 +551,104 @@ func TestTheBuildSeatNeverReadsMeshLabAtAReleaseBranch(t *testing.T) { } } } + +// **Judges are built warm and where no pull request reaches** (ADR 0358 §6): every judge build uses the judge cache's +// one Go cache, kept between checks; its home is made fresh under the judge cache and removed after; nothing is left in +// a temporary directory of the host; and no container a check runs for the pull request mounts the judge cache. +func TestJudgesAreBuiltWarmFromAJudgeCacheNoCheckContainerMounts(t *testing.T) { + built := aRuntimeThatBuildsTheJudge(t) + runs := filepath.Join(t.TempDir(), "runs") + t.Setenv("RUNS", runs) + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + registry := aFactsRegistry(t) + cache := t.TempDir() + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("the controller the mesh runs"), nil) + beside := map[string]Beside{"mesh-controller": {Repository: controller, Ref: controllerMain}} + for i := range 2 { + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, "main", map[string]string{"modules/gitea/index.ts": fmt.Sprint(i)}) + v, err := Check(t.Context(), Command, CheckSpec{ID: fmt.Sprintf("check-704-warm-%d", i), Repository: catalogue, + Ref: head, Owner: "novox", Repo: "mesh-catalog", Number: 1, Base: "main", JudgeCache: cache, + Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Toolchain: "golang", Beside: beside}, + t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Summary != "the controller the mesh runs with the vendored validator" { + t.Fatalf("check %d was not judged: %+v\n%s", i, v.Gate, v.Report) + } + if i == 0 { + if entries, _ := os.ReadDir(filepath.Join(cache, "go-cache")); len(entries) == 0 { + t.Fatal("the judge's build left nothing in the judge cache's Go cache to build the next judge warm") + } + } + } + // Both judges built with the one Go cache of the judge cache. + said, _ := os.ReadFile(built) + if n := strings.Count(string(said), "GOCACHE="+filepath.Join(cache, "go-cache")+" GOENV=off go build"); n != 2 { + t.Fatalf("%d of 2 judge builds used the judge cache's Go cache:\n%s", n, said) + } + // Nothing left behind: no home under the judge cache, nothing in the host's temporary directory. + if left, _ := os.ReadDir(filepath.Join(cache, "homes")); len(left) != 0 { + t.Fatalf("judge builds left %d home(s) under the judge cache", len(left)) + } + if left, _ := os.ReadDir(tmp); len(left) != 0 { + t.Fatalf("a check left %d entr(ies) in the temporary directory, e.g. %s", len(left), left[0].Name()) + } + // Only a judge build mounts the judge cache: no container the check runs for the pull request does. + body, _ := os.ReadFile(runs) + judgeBuilds := 0 + for _, line := range strings.Split(strings.TrimSpace(string(body)), "\n") { + judging := strings.Contains(line, " go build ") + if judging { + judgeBuilds++ + if !strings.Contains(line, "--volume "+filepath.Join(cache, "go-cache")+":") { + t.Fatalf("a judge build did not mount the judge cache's Go cache: %s", line) + } + continue + } + if strings.Contains(line, cache) { + t.Fatalf("a container the check runs for the pull request names the judge cache: %s", line) + } + } + if judgeBuilds != 2 { + t.Fatalf("%d judge builds were run, not 2", judgeBuilds) + } +} + +// Without a judge cache, a judge is built cold under the check's own root — never in a temporary directory of the +// host — and a judge cache inside the workspace is refused. +func TestWithoutAJudgeCacheAJudgeIsBuiltColdUnderTheChecksRoot(t *testing.T) { + built := aRuntimeThatBuildsTheJudge(t) + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + registry := aFactsRegistry(t) + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("the controller the mesh runs"), nil) + beside := map[string]Beside{"mesh-controller": {Repository: controller, Ref: controllerMain}} + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, "main", map[string]string{"modules/gitea/index.ts": "x"}) + workspace := t.TempDir() + spec := CheckSpec{ID: "check-704-cold", Repository: catalogue, Ref: head, Owner: "novox", Repo: "mesh-catalog", + Number: 1, Base: "main", Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, + Toolchain: "golang", Beside: beside} + v, err := Check(t.Context(), Command, spec, workspace, registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Verdict != "pass" { + t.Fatalf("the check was not judged: %+v\n%s", v.Gate, v.Report) + } + said, _ := os.ReadFile(built) + if !strings.Contains(string(said), "GOCACHE="+filepath.Join(workspace, "check", "judge-go-cache")+" GOENV=off go build") { + t.Fatalf("without a judge cache the judge was not built under the check's root:\n%s", said) + } + if left, _ := os.ReadDir(tmp); len(left) != 0 { + t.Fatalf("a check left %d entr(ies) in the temporary directory", len(left)) + } + spec.JudgeCache = filepath.Join(workspace, "judge-cache") + if _, err := Check(t.Context(), Command, spec, workspace, registry, GitCredential{}, nil); err == nil || + !strings.Contains(err.Error(), "inside the workspace") { + t.Fatalf("a judge cache inside the workspace was taken: %v", err) + } +} -- 2.54.0 From 0214b45469ecaf44e883936f4550ce78d24560c5 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 14:15:19 +0000 Subject: [PATCH 6/8] Run mesh-lab's replays where no pull request reaches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The replays hold the container runtime's socket, yet took their HOME, Go environment, build cache and module cache from the workspace a pull request's own script writes, so a planted -toolexec hook ran as root on the build node (hq issue 709). They now run from a copy of mesh-lab made before anything of the change runs, with a fresh home and the judge cache's caches, as a judge is built (ADR 0358 §6). Co-Authored-By: Claude Opus 5.5 --- internal/builder/check.go | 114 ++++++++++++++++++++++-- internal/builder/release_branch_test.go | 74 ++++++++++++++- 2 files changed, 179 insertions(+), 9 deletions(-) diff --git a/internal/builder/check.go b/internal/builder/check.go index dcd0f5c2..898e1d37 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -329,6 +329,13 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if err := forgetLeftCredentials(workspace); err != nil { return CheckVerdict{}, err } + // **mesh-lab's replays run where no pull request reaches** (novox/hq issue 709, ADR 0358 §6): they hold the + // container runtime's socket, so their sources, home, Go cache and module cache are none of the workspace's. + replay, err := replayDirs(spec.JudgeCache, workspace, root, spec.ID) + if err != nil { + return CheckVerdict{}, err + } + defer os.RemoveAll(replay.base) // cloneAt clones a repository and checks out ref — or, when the repository has no branch ref and orElse is said, // orElse (novox/hq ADR 0358 §3); readAt keeps which, by directory. readAt := map[string]string{} @@ -447,6 +454,12 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil { return CheckVerdict{}, fmt.Errorf("beside it, %w", err) } + if dir == "mesh-lab" && replay.lab != "" { + // The copy the replays run from, made before anything of the change runs, outside every check container. + if err := copyCheckout(ctx, run, filepath.Join(root, dir), replay.lab); err != nil { + return CheckVerdict{}, fmt.Errorf("mesh-lab could not be copied for its replays: %w", err) + } + } if spec.Release != "" { // The branch each was read at is named with its commit (ADR 0358 §4). if judged.BesideAt == nil { @@ -617,6 +630,21 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome, "--volume", judgeGoCache + ":" + judgeGoCache}, args[1:]...) } + replayLab := filepath.Join(root, "mesh-lab") + if replay.lab != "" { + replayLab = replay.lab + } + inReplays := func(env []string, command ...string) []string { + args := in(spec.Toolchain, filepath.Join(replayLab, "replays"), append(ReplayEnv(replay.home, replay.goCache, + replay.modCache), env...), command...) + // The socket goes to the replays alone, never to the change's own code. + mounts := []string{"--volume", "/var/run/docker.sock:/var/run/docker.sock"} + if spec.JudgeCache != "" { + mounts = append(mounts, "--volume", replay.base+":"+replay.base, "--volume", replay.goCache+":"+replay.goCache, + "--volume", replay.modCache+":"+replay.modCache) + } + return append(append([]string{args[0]}, mounts...), args[1:]...) + } var out tail // running runs one container of the check, its output into the report, in a process group of its own. running := func(args []string) error { @@ -670,7 +698,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry say("check", "the gate: %d module(s) of the graph touched — %s", len(modules), strings.Join(modules, ", ")) fmt.Fprintf(&out, "--- the gate: %s\n", strings.Join(modules, ", ")) v.Gate.Verdict, v.Gate.Summary = gateLayer(ctx, spec, tree, root, gate, verdictFile, env, inToolchain, - running, &out, bus, workspace, name, say) + running, &out, bus, name, replayLab, inReplays, say) } if timedOut() { v.Gate.Verdict, v.Gate.Summary = "error", fmt.Sprintf("the check ran past %s and was ended", CheckTimeout) @@ -947,8 +975,9 @@ const SomeManifestsEnv = "MESH_MODULE_CHECK_SOME=1" // gateLayer runs the gate: the touched manifests through `module check`, every machine composed with the // change, and the replays of what the mesh runs. It answers the gate's verdict and summary. func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFile string, env []string, - inToolchain func(string, []string, ...string) []string, running func([]string) error, out *tail, bus, workspace, - name string, say func(step, format string, args ...any)) (string, string) { + inToolchain func(string, []string, ...string) []string, running func([]string) error, out *tail, bus, + name, replayLab string, inReplays func([]string, ...string) []string, + say func(step, format string, args ...any)) (string, string) { // 1. The manifests the change touches, as the judge reads them: a manifest it cannot read, or one with a // problem the change brings, fails here. **What was already so on the base branch is said and fails // nothing** — the gate's own rule: a module whose manifest the running controller already finds fault @@ -1040,7 +1069,7 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil // 3. **The replays of what the mesh runs** (to-be 45 §9, M9): mesh-lab's, from its main — reviewed code, // so given the container runtime the resolver replay raises containers with — against the bus of the // release the mesh runs and the change's own catalogue when the change is to the catalogue. - if lab := filepath.Join(root, "mesh-lab", "replays"); ctx.Err() == nil { + if lab := filepath.Join(replayLab, "replays"); ctx.Err() == nil { if _, err := os.Stat(lab); err == nil { catalogue := filepath.Join(root, "mesh-catalog") if name == "mesh-catalog" { @@ -1048,11 +1077,10 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil } say("check", "the replays of what the mesh runs, from mesh-lab") fmt.Fprintln(out, "--- the replays (mesh-lab replays/)") - args := inToolchain(lab, []string{EnvTestBus + "=nats://" + bus, "MESH_REPLAY_CATALOGUE=" + catalogue, - "GOCACHE=" + filepath.Join(workspace, "go-cache"), "GOMODCACHE=" + filepath.Join(workspace, "go-modules")}, + // In the replays' own environment (inReplays): reviewed code, the socket, and nothing the workspace holds + // but the catalogue they read. + args := inReplays([]string{EnvTestBus + "=nats://" + bus, "MESH_REPLAY_CATALOGUE=" + catalogue}, "go", "test", "-count=1", "./...") - // The socket goes to the replays alone, never to the change's own code above. - args = append([]string{args[0], "--volume", "/var/run/docker.sock:/var/run/docker.sock"}, args[1:]...) if err := running(args); err != nil { if ctx.Err() != nil { return "error", "the check was ended during the replays" @@ -1225,6 +1253,76 @@ func judgeBuildDirs(cache, workspace, root, id string) (string, string, error) { return home, goCache, nil } +// replayPlace is where mesh-lab's replays run from and with (novox/hq issue 709): base, the directory made for this +// check and removed after it; lab, the copy of mesh-lab they run (empty: the checkout beside the check); home, their +// HOME and configuration; and the Go build and module caches. +type replayPlace struct { + base, lab, home, goCache, modCache string +} + +// replayDirs makes the replays' place (issue 709, ADR 0358 §6). With a judge cache: a directory under its homes/ for +// this check, holding a copy of mesh-lab made before anything of the change runs and a fresh home, and the judge +// cache's Go build and module caches — none of which a check's container mounts. The replays' build is as trusted as a +// judge's: it compiles mesh-lab's trunk, reviewed code, and nothing of the pull request. Without a judge cache: a home +// and caches under the check's root, which every check empties before it clones, and the replays run from the +// checkout beside the check, as before. +func replayDirs(cache, workspace, root, id string) (replayPlace, error) { + if cache == "" { + p := replayPlace{base: filepath.Join(root, "replay-home"), goCache: filepath.Join(root, "judge-go-cache"), + modCache: filepath.Join(root, "replay-go-modules")} + p.home = p.base + for _, d := range []string{p.home, p.goCache, p.modCache} { + if err := os.MkdirAll(d, 0o700); err != nil { + return replayPlace{}, err + } + } + return p, nil + } + if withinDir(workspace, cache) { + return replayPlace{}, fmt.Errorf("the judge cache %s is inside the workspace %s, which a check's containers "+ + "mount: no replay is run where a pull request could reach it", cache, workspace) + } + if !safeName.MatchString(id) { + id = "check" + } + base := filepath.Join(cache, "homes", id+"-replays") + p := replayPlace{base: base, lab: filepath.Join(base, "mesh-lab"), home: filepath.Join(base, "home"), + goCache: filepath.Join(cache, "go-cache"), modCache: filepath.Join(cache, "go-modules")} + if err := os.RemoveAll(base); err != nil { + return replayPlace{}, err + } + for _, d := range []string{p.home, p.goCache, p.modCache} { + if err := os.MkdirAll(d, 0o700); err != nil { + return replayPlace{}, err + } + } + return p, nil +} + +// ReplayEnv is the environment mesh-lab's replays run in (novox/hq issue 709): home as HOME and configuration, the Go +// build and module caches given, no global or system git configuration, no Go environment file, and no version +// stamped from git — as a judge is built (JudgeBuildEnv), with the modules the replays fetch. +func ReplayEnv(home, goCache, modCache string) []string { + return []string{"HOME=" + home, "XDG_CONFIG_HOME=" + filepath.Join(home, ".config"), + "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "GOENV=off", + "GOCACHE=" + goCache, "GOMODCACHE=" + modCache, "GOPATH=" + filepath.Join(home, "go"), + "GOFLAGS=-buildvcs=false"} +} + +// copyCheckout copies a checkout at the commit it stands at into dir, over git's own transport — never by linking +// objects — so what is written into the source afterwards does not reach the copy. +func copyCheckout(ctx context.Context, run Runner, from, dir string) error { + commit := fullHeadAt(ctx, run, from) + if commit == "" { + return fmt.Errorf("%s names no commit", from) + } + if _, err := run(ctx, filepath.Dir(dir), "git", "clone", "--quiet", "--no-local", "--no-checkout", from, dir); err != nil { + return err + } + _, err := run(ctx, dir, "git", "checkout", "--quiet", "--detach", commit) + return err +} + // keptJudges is where a judge cache keeps the release-branch judges, empty when there is no judge cache. func keptJudges(cache string) string { if cache == "" { diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index 123c13c0..d1ea2c27 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -147,6 +147,9 @@ run) shift if [ "$1" = go ]; then echo "GOCACHE=$GOCACHE GOENV=$GOENV $*" >> "` + built + `"; fi if [ "$1" = go ] && [ "$2" = build ] && [ -n "$JUDGE_HOMES" ]; then echo "$HOME" >> "$JUDGE_HOMES"; fi + if [ "$1" = go ] && [ "$2" = test ] && [ -n "$REPLAYS" ]; then + echo "HOME=$HOME GOCACHE=$GOCACHE GOMODCACHE=$GOMODCACHE GOENV=$GOENV GOFLAGS=$GOFLAGS DIR=$workdir" >> "$REPLAYS" + fi cd "$workdir" && exec "$@" ;; esac exit 1 @@ -362,7 +365,7 @@ func TestWithoutTheNodeEnginesReleaseBranchTheJudgeKeepsItsVendoredValidator(t * } } -// **Clause 5** (ADR 0358 §5): a release branch's own pull request into its trunk is checked as any pull request into a +// **Clause 7** (ADR 0358 §7): a release branch's own pull request into its trunk is checked as any pull request into a // trunk — by the controller beside it as the mesh runs it, the repositories at what a trunk check reads — and its // verdict names no release branch. func TestAReleaseBranchsPullRequestIntoItsTrunkIsJudgedByTheControllerTheMeshRuns(t *testing.T) { @@ -652,3 +655,72 @@ func TestWithoutAJudgeCacheAJudgeIsBuiltColdUnderTheChecksRoot(t *testing.T) { t.Fatalf("a judge cache inside the workspace was taken: %v", err) } } + +// aLabWithReplays is mesh-lab with replays: a Go module of its own whose one test passes, needing nothing fetched. +func aLabWithReplays(t *testing.T) (string, string) { + t.Helper() + lab, labMain, _ := aRepositoryWithARelease(t, map[string]string{ + "replays/go.mod": "module example.org/replays\n\ngo 1.22\n", + "replays/replay_test.go": "package replays\n\nimport \"testing\"\n\nfunc TestAReplay(t *testing.T) {}\n", + }, nil) + return lab, labMain +} + +// **mesh-lab's replays run where no pull request reaches** (novox/hq issue 709, ADR 0358 §6): they hold the container +// runtime's socket, so a Go environment file with a -toolexec hook and a Go cache and module cache planted in the +// workspace by a pull request's script are neither run nor read — the replays run from a copy of mesh-lab outside +// the workspace with a fresh home and the judge cache's caches, all removed or kept outside every check container. +func TestTheReplaysRunWhereNoPullRequestReaches(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + replays := filepath.Join(t.TempDir(), "replays") + t.Setenv("REPLAYS", replays) + registry := aFactsRegistry(t) + workspace, ran := aPlantedWorkspace(t) + for _, poisoned := range []string{filepath.Join(workspace, "go-cache", "README"), + filepath.Join(workspace, "go-modules", "cache", "download", "planted")} { + if err := os.MkdirAll(filepath.Dir(poisoned), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(poisoned, []byte("planted by a pull request"), 0o644); err != nil { + t.Fatal(err) + } + } + cache := t.TempDir() + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("the controller the mesh runs"), nil) + lab, labMain := aLabWithReplays(t) + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, "main", map[string]string{"modules/gitea/index.ts": "x"}) + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-709", Repository: catalogue, Ref: head, Owner: "novox", + Repo: "mesh-catalog", Number: 1, Base: "main", JudgeCache: cache, Paths: []string{"modules/gitea/index.ts"}, + Modules: []string{"gitea"}, Toolchain: "golang", Beside: map[string]Beside{ + "mesh-controller": {Repository: controller, Ref: controllerMain}, "mesh-lab": {Repository: lab, Ref: labMain}}}, + workspace, registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Verdict != "pass" || !strings.Contains(v.Report, "--- the replays") { + t.Fatalf("the replays did not run and pass: %+v\n%s", v.Gate, v.Report) + } + body, _ := os.ReadFile(replays) + said := strings.TrimSpace(string(body)) + if said == "" || strings.Count(said, "\n") != 0 { + t.Fatalf("the replays ran %q, not once", said) + } + for _, want := range []string{"GOCACHE=" + filepath.Join(cache, "go-cache") + " ", + "GOMODCACHE=" + filepath.Join(cache, "go-modules") + " ", "GOENV=off ", "GOFLAGS=-buildvcs=false ", + "HOME=" + filepath.Join(cache, "homes", "check-709-replays", "home") + " ", + "DIR=" + filepath.Join(cache, "homes", "check-709-replays", "mesh-lab", "replays")} { + if !strings.Contains(said, want) { + t.Fatalf("the replays did not run with %q: %s", want, said) + } + } + if strings.Contains(said, workspace) { + t.Fatalf("the replays ran with something of the workspace: %s", said) + } + if body, _ := os.ReadFile(ran); strings.Contains(string(body), "replays") || strings.Contains(string(body), "mesh-lab") { + t.Fatalf("a hook planted in the workspace ran during the replays:\n%s", body) + } + if left, _ := os.ReadDir(filepath.Join(cache, "homes")); len(left) != 0 { + t.Fatalf("the replays left %d entr(ies) under the judge cache's homes", len(left)) + } +} -- 2.54.0 From 847ffd6d1285a35d5e9fe572d05b55b8dbe18fd2 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 14:34:00 +0000 Subject: [PATCH 7/8] Never run the replays from the workspace, with or without a judge cache Without a judge cache the replays still ran mesh-lab's checkout in the workspace with the container runtime's socket, so a self-judging pull request could write a test into them (hq issue 709, review 12303). They now run from a copy in a container volume of the check's own, made before anything of the change runs, with a fresh home and throwaway caches, removed after the check. Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-builder/main.go | 5 +- internal/builder/check.go | 111 ++++++++++++++++-------- internal/builder/release_branch_test.go | 100 +++++++++++++++++++++ 3 files changed, 178 insertions(+), 38 deletions(-) diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index 51dcd1b4..4ad26abc 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -313,8 +313,9 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri registry = r.Address } spec := checkSpecOf(request) - // The judge cache the build agent declares as its own data, never in the workspace: only judge builds mount - // it, so no pull request's script can write what a judge is built with or kept as (novox/hq ADR 0358 §6). + // The judge cache the build agent declares as its own data, never in the workspace: only judge builds and + // mesh-lab's replays mount it, never a check's containers, so no pull request's script can write what a judge + // is built with or kept as, or what the replays run (novox/hq ADR 0358 §6, issue 709). spec.JudgeCache = judgeCache() // What the merge would move, built by the check for the walk to take (novox/hq ADR 0350 §1): into the // scratch namespace. **Never with the package registry's credential**: this is code nobody has approved diff --git a/internal/builder/check.go b/internal/builder/check.go index 898e1d37..fc368569 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -335,7 +335,16 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if err != nil { return CheckVerdict{}, err } - defer os.RemoveAll(replay.base) + if spec.JudgeCache != "" { + defer os.RemoveAll(replay.base) + } + defer func() { + if replay.volumeMade { + removing, done := context.WithTimeout(context.Background(), time.Minute) + defer done() + _, _ = run(removing, root, "docker", "volume", "rm", "--force", replay.volume) + } + }() // cloneAt clones a repository and checks out ref — or, when the repository has no branch ref and orElse is said, // orElse (novox/hq ADR 0358 §3); readAt keeps which, by directory. readAt := map[string]string{} @@ -454,9 +463,10 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil { return CheckVerdict{}, fmt.Errorf("beside it, %w", err) } - if dir == "mesh-lab" && replay.lab != "" { - // The copy the replays run from, made before anything of the change runs, outside every check container. - if err := copyCheckout(ctx, run, filepath.Join(root, dir), replay.lab); err != nil { + if dir == "mesh-lab" { + // The copy the replays run from, made before anything of the change runs, outside every check container + // (novox/hq issue 709) — and whether it holds replays, read now, so nothing written later decides it. + if err := replay.copyLab(ctx, run, spec, root, workspace, filepath.Join(root, dir)); err != nil { return CheckVerdict{}, fmt.Errorf("mesh-lab could not be copied for its replays: %w", err) } } @@ -630,16 +640,14 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome, "--volume", judgeGoCache + ":" + judgeGoCache}, args[1:]...) } - replayLab := filepath.Join(root, "mesh-lab") - if replay.lab != "" { - replayLab = replay.lab - } inReplays := func(env []string, command ...string) []string { - args := in(spec.Toolchain, filepath.Join(replayLab, "replays"), append(ReplayEnv(replay.home, replay.goCache, + args := in(spec.Toolchain, filepath.Join(replay.lab, "replays"), append(ReplayEnv(replay.home, replay.goCache, replay.modCache), env...), command...) // The socket goes to the replays alone, never to the change's own code. mounts := []string{"--volume", "/var/run/docker.sock:/var/run/docker.sock"} - if spec.JudgeCache != "" { + if replay.volume != "" { + mounts = append(mounts, "--volume", replay.volume+":"+replay.base) + } else { mounts = append(mounts, "--volume", replay.base+":"+replay.base, "--volume", replay.goCache+":"+replay.goCache, "--volume", replay.modCache+":"+replay.modCache) } @@ -698,7 +706,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry say("check", "the gate: %d module(s) of the graph touched — %s", len(modules), strings.Join(modules, ", ")) fmt.Fprintf(&out, "--- the gate: %s\n", strings.Join(modules, ", ")) v.Gate.Verdict, v.Gate.Summary = gateLayer(ctx, spec, tree, root, gate, verdictFile, env, inToolchain, - running, &out, bus, name, replayLab, inReplays, say) + running, &out, bus, name, replay.hasReplays, inReplays, say) } if timedOut() { v.Gate.Verdict, v.Gate.Summary = "error", fmt.Sprintf("the check ran past %s and was ended", CheckTimeout) @@ -976,7 +984,7 @@ const SomeManifestsEnv = "MESH_MODULE_CHECK_SOME=1" // change, and the replays of what the mesh runs. It answers the gate's verdict and summary. func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFile string, env []string, inToolchain func(string, []string, ...string) []string, running func([]string) error, out *tail, bus, - name, replayLab string, inReplays func([]string, ...string) []string, + name string, hasReplays bool, inReplays func([]string, ...string) []string, say func(step, format string, args ...any)) (string, string) { // 1. The manifests the change touches, as the judge reads them: a manifest it cannot read, or one with a // problem the change brings, fails here. **What was already so on the base branch is said and fails @@ -1069,8 +1077,8 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil // 3. **The replays of what the mesh runs** (to-be 45 §9, M9): mesh-lab's, from its main — reviewed code, // so given the container runtime the resolver replay raises containers with — against the bus of the // release the mesh runs and the change's own catalogue when the change is to the catalogue. - if lab := filepath.Join(replayLab, "replays"); ctx.Err() == nil { - if _, err := os.Stat(lab); err == nil { + if ctx.Err() == nil { + if hasReplays { catalogue := filepath.Join(root, "mesh-catalog") if name == "mesh-catalog" { catalogue = tree @@ -1253,38 +1261,39 @@ func judgeBuildDirs(cache, workspace, root, id string) (string, string, error) { return home, goCache, nil } -// replayPlace is where mesh-lab's replays run from and with (novox/hq issue 709): base, the directory made for this -// check and removed after it; lab, the copy of mesh-lab they run (empty: the checkout beside the check); home, their -// HOME and configuration; and the Go build and module caches. +// replayPlace is where mesh-lab's replays run from and with (novox/hq issue 709), never anything a pull request can +// write: base, the place made for this check; lab, the copy of mesh-lab they run; home, their HOME and configuration; +// and their Go build and module caches. With a judge cache, base is a directory under its homes/ (the paths the same +// on the host and in the build agent) and the caches are the judge cache's. Without one, base is where a container +// volume of this check's own is mounted (volume), holding the copy, the home and throwaway caches, removed after the +// check: the build agent declares no other host path outside the workspace. hasReplays says whether mesh-lab held +// replays when it was copied, before anything of the change ran. type replayPlace struct { base, lab, home, goCache, modCache string + volume string + volumeMade, hasReplays bool } -// replayDirs makes the replays' place (issue 709, ADR 0358 §6). With a judge cache: a directory under its homes/ for -// this check, holding a copy of mesh-lab made before anything of the change runs and a fresh home, and the judge -// cache's Go build and module caches — none of which a check's container mounts. The replays' build is as trusted as a -// judge's: it compiles mesh-lab's trunk, reviewed code, and nothing of the pull request. Without a judge cache: a home -// and caches under the check's root, which every check empties before it clones, and the replays run from the -// checkout beside the check, as before. +// ReplayMount is where a check's own replay volume is mounted in the replays' container when there is no judge cache. +var ReplayMount = "/mesh-replays" + +// replayDirs makes the replays' place (issue 709, ADR 0358 §6). The replays' build is as trusted as a judge's: it +// compiles mesh-lab's trunk, reviewed code, and nothing of the pull request, which hands them only its catalogue, as +// data. Never the workspace: the gate runs before the replays, a pull request's own code among its containers. func replayDirs(cache, workspace, root, id string) (replayPlace, error) { + if !safeName.MatchString(id) { + id = "check" + } if cache == "" { - p := replayPlace{base: filepath.Join(root, "replay-home"), goCache: filepath.Join(root, "judge-go-cache"), - modCache: filepath.Join(root, "replay-go-modules")} - p.home = p.base - for _, d := range []string{p.home, p.goCache, p.modCache} { - if err := os.MkdirAll(d, 0o700); err != nil { - return replayPlace{}, err - } - } - return p, nil + base := ReplayMount + return replayPlace{base: base, lab: filepath.Join(base, "mesh-lab"), home: filepath.Join(base, "home"), + goCache: filepath.Join(base, "go-cache"), modCache: filepath.Join(base, "go-modules"), + volume: "mesh-check-" + strings.ToLower(id) + "-replays"}, nil } if withinDir(workspace, cache) { return replayPlace{}, fmt.Errorf("the judge cache %s is inside the workspace %s, which a check's containers "+ "mount: no replay is run where a pull request could reach it", cache, workspace) } - if !safeName.MatchString(id) { - id = "check" - } base := filepath.Join(cache, "homes", id+"-replays") p := replayPlace{base: base, lab: filepath.Join(base, "mesh-lab"), home: filepath.Join(base, "home"), goCache: filepath.Join(cache, "go-cache"), modCache: filepath.Join(cache, "go-modules")} @@ -1299,6 +1308,36 @@ func replayDirs(cache, workspace, root, id string) (replayPlace, error) { return p, nil } +// copyLab copies mesh-lab's checkout, freshly cloned and before anything of the change runs, to where the replays +// run it. With a judge cache, over git's own transport into its directory. Without one, into a volume of this check's +// own, made now: a container reads the checkout from the workspace, mounted read-only, and copies it with a fresh home +// and empty caches, owned by the user the check's containers run as. +func (p *replayPlace) copyLab(ctx context.Context, run Runner, spec CheckSpec, root, workspace, from string) error { + _, err := os.Stat(filepath.Join(from, "replays")) + p.hasReplays = err == nil + if p.volume == "" { + return copyCheckout(ctx, run, from, p.lab) + } + if spec.Toolchain == "" { + return errors.New("the mesh holds no Go toolchain to copy mesh-lab with") + } + user := spec.User + if user == "" { + user = fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid()) + } + _, _ = run(ctx, root, "docker", "volume", "rm", "--force", p.volume) // one an earlier delivery left + if _, err := run(ctx, root, "docker", "volume", "create", "--label", BuildLabel+"="+spec.ID, p.volume); err != nil { + return err + } + p.volumeMade = true + script := fmt.Sprintf("mkdir -p %s %s %s %s && cp -a %s/. %s/ && chown -R %s %s", + p.lab, p.home, p.goCache, p.modCache, from, p.lab, user, p.base) + _, err = run(ctx, root, "docker", LabelledArgs("docker", []string{"run", "--rm", "--network", "none", + "--volume", p.volume + ":" + p.base, "--volume", workspace + ":" + workspace + ":ro", "--workdir", "/", + spec.Toolchain, "sh", "-c", script}, spec.ID)...) + return err +} + // ReplayEnv is the environment mesh-lab's replays run in (novox/hq issue 709): home as HOME and configuration, the Go // build and module caches given, no global or system git configuration, no Go environment file, and no version // stamped from git — as a judge is built (JudgeBuildEnv), with the modules the replays fetch. @@ -1306,7 +1345,7 @@ func ReplayEnv(home, goCache, modCache string) []string { return []string{"HOME=" + home, "XDG_CONFIG_HOME=" + filepath.Join(home, ".config"), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "GOENV=off", "GOCACHE=" + goCache, "GOMODCACHE=" + modCache, "GOPATH=" + filepath.Join(home, "go"), - "GOFLAGS=-buildvcs=false"} + "GOFLAGS=-buildvcs=false", "GOWORK=off"} } // copyCheckout copies a checkout at the commit it stands at into dir, over git's own transport — never by linking diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index d1ea2c27..1e560d36 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -129,6 +129,10 @@ func aRuntimeThatBuildsTheJudge(t *testing.T) string { cmd="$1"; shift case "$cmd" in ps|rm|exec) exit 0 ;; +volume) + if [ -n "$VOLUMES" ]; then echo "$*" >> "$VOLUMES"; fi + if [ "$1" = rm ] && [ -n "$REPLAY_MOUNT" ]; then rm -rf "$REPLAY_MOUNT"; fi + exit 0 ;; port) echo "` + bus.Addr().String() + `"; exit 0 ;; run) if [ -n "$RUNS" ]; then echo "$*" >> "$RUNS"; fi @@ -724,3 +728,99 @@ func TestTheReplaysRunWhereNoPullRequestReaches(t *testing.T) { t.Fatalf("the replays left %d entr(ies) under the judge cache's homes", len(left)) } } + +// aSelfJudgingControllerThatWritesIntoTheReplays is a controller pull request whose own judge, while the gate runs it, +// writes a test into the replays beside the check — the reviewer's case for issue 709: code of the pull request's, +// run before the replays, reaching what they run. +func aSelfJudgingControllerThatWritesIntoTheReplays(t *testing.T, marker string) (string, string) { + t.Helper() + files := aJudgeOf("the change's controller") + evil := "package replays\n\nimport (\n\t\"os\"\n\t\"testing\"\n)\n\nfunc TestEvil(t *testing.T) {\n\t_ = os.WriteFile(" + + fmt.Sprintf("%q", marker) + ", []byte(\"ran\"), 0o644)\n}\n" + files["cmd/mesh-controller/main.go"] = strings.Replace(files["cmd/mesh-controller/main.go"], "func main() {", + "func main() {\n\tif len(os.Args) > 2 && os.Args[1] == \"merge-gate\" {\n\t\t_ = os.WriteFile(os.Getenv(\"MESH_CHECK_BESIDE\")+"+ + "\"/mesh-lab/replays/evil_test.go\", []byte("+fmt.Sprintf("%q", evil)+"), 0o644)\n\t\t_ = os.WriteFile("+ + fmt.Sprintf("%q", marker+".wrote")+", nil, 0o644)\n\t}", 1) + return aPullRequestInto(t, files, "main", map[string]string{"cmd/x.txt": "x"}) +} + +// **Without a judge cache, the replays still never read what a pull request wrote** (issue 709, review 12303): the +// copy of mesh-lab, its home and throwaway caches are in a container volume of the check's own, made before anything +// of the change runs and removed after — never the workspace. With a judge cache, under it. In both: a self-judging +// controller pull request writing a test into the replays beside the check, a planted GOFLAGS hook and a poisoned +// cache entry in the workspace neither run nor are read. +func TestTheReplaysNeverReadWhatAPullRequestWroteWithOrWithoutAJudgeCache(t *testing.T) { + for _, withCache := range []bool{false, true} { + t.Run(fmt.Sprintf("judge cache %v", withCache), func(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + replays, volumes := filepath.Join(t.TempDir(), "replays"), filepath.Join(t.TempDir(), "volumes") + t.Setenv("REPLAYS", replays) + t.Setenv("VOLUMES", volumes) + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + mount := filepath.Join(t.TempDir(), "mesh-replays") + t.Setenv("REPLAY_MOUNT", mount) + was := ReplayMount + ReplayMount = mount + t.Cleanup(func() { ReplayMount = was }) + registry := aFactsRegistry(t) + workspace, ran := aPlantedWorkspace(t) + if err := os.MkdirAll(filepath.Join(workspace, "go-cache"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, "go-cache", "README"), []byte("planted"), 0o644); err != nil { + t.Fatal(err) + } + evilRan := filepath.Join(t.TempDir(), "evil-ran") + controller, head := aSelfJudgingControllerThatWritesIntoTheReplays(t, evilRan) + lab, labMain := aLabWithReplays(t) + cache := "" + if withCache { + cache = t.TempDir() + } + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-709-self", Repository: controller, Ref: head, + Owner: "novox", Repo: "mesh-controller", Number: 330, Base: "main", Judge: "self", JudgeCache: cache, + Paths: []string{"cmd/x.txt"}, Modules: []string{"mesh-controller"}, Toolchain: "golang", + Beside: map[string]Beside{"mesh-lab": {Repository: lab, Ref: labMain}}}, workspace, registry, + GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Verdict != "pass" || !strings.Contains(v.Report, "--- the replays") { + t.Fatalf("the replays did not run and pass: %+v\n%s", v.Gate, v.Report) + } + if _, err := os.Stat(evilRan + ".wrote"); err != nil { + t.Fatalf("the self-judging pull request did not write into the replays beside it, so this proves nothing: %v", err) + } + if _, err := os.Stat(evilRan); err == nil { + t.Fatal("the test the pull request wrote into the replays ran with the socket") + } + if body, _ := os.ReadFile(ran); strings.Contains(string(body), "replays") || strings.Contains(string(body), "mesh-lab") { + t.Fatalf("a hook planted in the workspace ran during the replays:\n%s", body) + } + body, _ := os.ReadFile(replays) + said := strings.TrimSpace(string(body)) + if said == "" || strings.Contains(said, workspace) || !strings.Contains(said, "GOENV=off ") { + t.Fatalf("the replays ran with something of the workspace, or not at all: %q", said) + } + base := mount + if withCache { + base = filepath.Join(cache, "homes", "check-709-self-replays") + } + if !strings.Contains(said, "DIR="+filepath.Join(base, "mesh-lab", "replays")) { + t.Fatalf("the replays did not run from the copy under %s: %s", base, said) + } + vols, _ := os.ReadFile(volumes) + if !withCache && (!strings.Contains(string(vols), "create --label "+BuildLabel+"=check-709-self") || + !strings.Contains(string(vols), "rm --force mesh-check-check-709-self-replays")) { + t.Fatalf("the check's own replay volume was not made and removed: %q", vols) + } + if _, err := os.Stat(base); err == nil { + t.Fatalf("the replays' place %s was left after the check", base) + } + if left, _ := os.ReadDir(tmp); len(left) != 0 { + t.Fatalf("a check left %d entr(ies) in the temporary directory", len(left)) + } + }) + } +} -- 2.54.0 From 5d5bdf9e32d15b4a430cd53cbba6078af33f6c63 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 17:58:07 +0000 Subject: [PATCH 8/8] Refuse the replays without a judge cache rather than run them from a volume A check's own volume is no place for the replays: the store replay binds a directory it makes there into other containers by path, which the container runtime would create on the build node's own disk (review 12308). Without the build agent's judge cache the replays are refused and the gate says so as an error; replays the runtime cannot start are the check's error, not the change's; and an id that cannot name a place in the judge cache is refused (hq issue 709, issue 704). Co-Authored-By: Claude Opus 5.5 --- internal/builder/check.go | 122 ++++++++++++------------ internal/builder/release_branch_test.go | 103 +++++++++++++------- 2 files changed, 130 insertions(+), 95 deletions(-) diff --git a/internal/builder/check.go b/internal/builder/check.go index fc368569..0e10a85f 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -335,16 +335,9 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if err != nil { return CheckVerdict{}, err } - if spec.JudgeCache != "" { + if replay.base != "" { defer os.RemoveAll(replay.base) } - defer func() { - if replay.volumeMade { - removing, done := context.WithTimeout(context.Background(), time.Minute) - defer done() - _, _ = run(removing, root, "docker", "volume", "rm", "--force", replay.volume) - } - }() // cloneAt clones a repository and checks out ref — or, when the repository has no branch ref and orElse is said, // orElse (novox/hq ADR 0358 §3); readAt keeps which, by directory. readAt := map[string]string{} @@ -466,7 +459,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if dir == "mesh-lab" { // The copy the replays run from, made before anything of the change runs, outside every check container // (novox/hq issue 709) — and whether it holds replays, read now, so nothing written later decides it. - if err := replay.copyLab(ctx, run, spec, root, workspace, filepath.Join(root, dir)); err != nil { + if err := replay.copyLab(ctx, run, filepath.Join(root, dir)); err != nil { return CheckVerdict{}, fmt.Errorf("mesh-lab could not be copied for its replays: %w", err) } } @@ -644,13 +637,8 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry args := in(spec.Toolchain, filepath.Join(replay.lab, "replays"), append(ReplayEnv(replay.home, replay.goCache, replay.modCache), env...), command...) // The socket goes to the replays alone, never to the change's own code. - mounts := []string{"--volume", "/var/run/docker.sock:/var/run/docker.sock"} - if replay.volume != "" { - mounts = append(mounts, "--volume", replay.volume+":"+replay.base) - } else { - mounts = append(mounts, "--volume", replay.base+":"+replay.base, "--volume", replay.goCache+":"+replay.goCache, - "--volume", replay.modCache+":"+replay.modCache) - } + mounts := []string{"--volume", "/var/run/docker.sock:/var/run/docker.sock", "--volume", replay.base + ":" + + replay.base, "--volume", replay.goCache + ":" + replay.goCache, "--volume", replay.modCache + ":" + replay.modCache} return append(append([]string{args[0]}, mounts...), args[1:]...) } var out tail @@ -706,7 +694,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry say("check", "the gate: %d module(s) of the graph touched — %s", len(modules), strings.Join(modules, ", ")) fmt.Fprintf(&out, "--- the gate: %s\n", strings.Join(modules, ", ")) v.Gate.Verdict, v.Gate.Summary = gateLayer(ctx, spec, tree, root, gate, verdictFile, env, inToolchain, - running, &out, bus, name, replay.hasReplays, inReplays, say) + running, &out, bus, name, replay.hasReplays, replay.refused(), inReplays, say) } if timedOut() { v.Gate.Verdict, v.Gate.Summary = "error", fmt.Sprintf("the check ran past %s and was ended", CheckTimeout) @@ -984,7 +972,7 @@ const SomeManifestsEnv = "MESH_MODULE_CHECK_SOME=1" // change, and the replays of what the mesh runs. It answers the gate's verdict and summary. func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFile string, env []string, inToolchain func(string, []string, ...string) []string, running func([]string) error, out *tail, bus, - name string, hasReplays bool, inReplays func([]string, ...string) []string, + name string, hasReplays bool, replaysRefused string, inReplays func([]string, ...string) []string, say func(step, format string, args ...any)) (string, string) { // 1. The manifests the change touches, as the judge reads them: a manifest it cannot read, or one with a // problem the change brings, fails here. **What was already so on the base branch is said and fails @@ -1077,6 +1065,12 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil // 3. **The replays of what the mesh runs** (to-be 45 §9, M9): mesh-lab's, from its main — reviewed code, // so given the container runtime the resolver replay raises containers with — against the bus of the // release the mesh runs and the change's own catalogue when the change is to the catalogue. + if ctx.Err() == nil && hasReplays && replaysRefused != "" { + // **Never from the workspace** (novox/hq issue 709): without the judge cache there is no place outside it. + say("check", "%s", replaysRefused) + fmt.Fprintf(out, "--- the replays: %s\n", replaysRefused) + return "error", replaysRefused + } if ctx.Err() == nil { if hasReplays { catalogue := filepath.Join(root, "mesh-catalog") @@ -1093,7 +1087,15 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil if ctx.Err() != nil { return "error", "the check was ended during the replays" } - return "fail", "a replay of a core incident fails with this change: " + lastLine(out.String()) + if replaysNotStarted(err) { + // The container runtime could not start them (a mount, an image): the check's fault, never the change's. + said := "the replays could not be started, so the change is not judged by them: " + lastLine(out.String()) + say("check", "%s", said) + return "error", said + } + said := "a replay of a core incident fails with this change: " + lastLine(out.String()) + say("check", "%s", said) + return "fail", said } } } @@ -1247,7 +1249,7 @@ func judgeBuildDirs(cache, workspace, root, id string) (string, string, error) { "no judge is built where a pull request could reach it", cache, workspace) } if !safeName.MatchString(id) { - id = "check" + return "", "", fmt.Errorf("the check's id %q cannot name its place in the judge cache", id) } home, goCache := filepath.Join(cache, "homes", id), filepath.Join(cache, "go-cache") if err := os.RemoveAll(home); err != nil { @@ -1262,38 +1264,42 @@ func judgeBuildDirs(cache, workspace, root, id string) (string, string, error) { } // replayPlace is where mesh-lab's replays run from and with (novox/hq issue 709), never anything a pull request can -// write: base, the place made for this check; lab, the copy of mesh-lab they run; home, their HOME and configuration; -// and their Go build and module caches. With a judge cache, base is a directory under its homes/ (the paths the same -// on the host and in the build agent) and the caches are the judge cache's. Without one, base is where a container -// volume of this check's own is mounted (volume), holding the copy, the home and throwaway caches, removed after the -// check: the build agent declares no other host path outside the workspace. hasReplays says whether mesh-lab held -// replays when it was copied, before anything of the change ran. +// write: base, the directory made for this check under the judge cache's homes/ (the same path on the host and in the +// build agent), removed after it; lab, the copy of mesh-lab they run; home, their HOME and configuration; and the judge +// cache's Go build and module caches. Empty without a judge cache: the replays are then refused. hasReplays says whether +// mesh-lab held replays when it was cloned, before anything of the change ran. type replayPlace struct { base, lab, home, goCache, modCache string - volume string - volumeMade, hasReplays bool + hasReplays bool } -// ReplayMount is where a check's own replay volume is mounted in the replays' container when there is no judge cache. -var ReplayMount = "/mesh-replays" +// replaysNeedTheJudgeCache is the gate's error when mesh-lab holds replays and the build agent has no judge cache. +const replaysNeedTheJudgeCache = "the replays need the build agent's judge cache (mesh-catalog#297); refused rather " + + "than run from the workspace" + +// refused is why the replays cannot run here, empty when they can. +func (p replayPlace) refused() string { + if p.base == "" { + return replaysNeedTheJudgeCache + } + return "" +} // replayDirs makes the replays' place (issue 709, ADR 0358 §6). The replays' build is as trusted as a judge's: it // compiles mesh-lab's trunk, reviewed code, and nothing of the pull request, which hands them only its catalogue, as -// data. Never the workspace: the gate runs before the replays, a pull request's own code among its containers. +// data. Never the workspace: the gate runs before the replays, a pull request's own code among its containers. Without +// a judge cache there is no place: the build agent declares no other host path, and the replays are refused. func replayDirs(cache, workspace, root, id string) (replayPlace, error) { - if !safeName.MatchString(id) { - id = "check" - } if cache == "" { - base := ReplayMount - return replayPlace{base: base, lab: filepath.Join(base, "mesh-lab"), home: filepath.Join(base, "home"), - goCache: filepath.Join(base, "go-cache"), modCache: filepath.Join(base, "go-modules"), - volume: "mesh-check-" + strings.ToLower(id) + "-replays"}, nil + return replayPlace{}, nil } if withinDir(workspace, cache) { return replayPlace{}, fmt.Errorf("the judge cache %s is inside the workspace %s, which a check's containers "+ "mount: no replay is run where a pull request could reach it", cache, workspace) } + if !safeName.MatchString(id) { + return replayPlace{}, fmt.Errorf("the check's id %q cannot name its place in the judge cache", id) + } base := filepath.Join(cache, "homes", id+"-replays") p := replayPlace{base: base, lab: filepath.Join(base, "mesh-lab"), home: filepath.Join(base, "home"), goCache: filepath.Join(cache, "go-cache"), modCache: filepath.Join(cache, "go-modules")} @@ -1308,34 +1314,26 @@ func replayDirs(cache, workspace, root, id string) (replayPlace, error) { return p, nil } -// copyLab copies mesh-lab's checkout, freshly cloned and before anything of the change runs, to where the replays -// run it. With a judge cache, over git's own transport into its directory. Without one, into a volume of this check's -// own, made now: a container reads the checkout from the workspace, mounted read-only, and copies it with a fresh home -// and empty caches, owned by the user the check's containers run as. -func (p *replayPlace) copyLab(ctx context.Context, run Runner, spec CheckSpec, root, workspace, from string) error { +// copyLab reads whether mesh-lab's checkout, freshly cloned and before anything of the change runs, holds replays, and +// copies it over git's own transport to where the replays run it — when there is such a place. +func (p *replayPlace) copyLab(ctx context.Context, run Runner, from string) error { _, err := os.Stat(filepath.Join(from, "replays")) p.hasReplays = err == nil - if p.volume == "" { - return copyCheckout(ctx, run, from, p.lab) + if p.lab == "" { + return nil } - if spec.Toolchain == "" { - return errors.New("the mesh holds no Go toolchain to copy mesh-lab with") + return copyCheckout(ctx, run, from, p.lab) +} + +// replaysNotStarted says the container runtime could not start the replays' container — its own exit status 125, or +// 126 and 127 for a command it could not run — rather than that a replay failed. +func replaysNotStarted(err error) bool { + var exit *exec.ExitError + if !errors.As(err, &exit) { + return true } - user := spec.User - if user == "" { - user = fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid()) - } - _, _ = run(ctx, root, "docker", "volume", "rm", "--force", p.volume) // one an earlier delivery left - if _, err := run(ctx, root, "docker", "volume", "create", "--label", BuildLabel+"="+spec.ID, p.volume); err != nil { - return err - } - p.volumeMade = true - script := fmt.Sprintf("mkdir -p %s %s %s %s && cp -a %s/. %s/ && chown -R %s %s", - p.lab, p.home, p.goCache, p.modCache, from, p.lab, user, p.base) - _, err = run(ctx, root, "docker", LabelledArgs("docker", []string{"run", "--rm", "--network", "none", - "--volume", p.volume + ":" + p.base, "--volume", workspace + ":" + workspace + ":ro", "--workdir", "/", - spec.Toolchain, "sh", "-c", script}, spec.ID)...) - return err + code := exit.ExitCode() + return code == 125 || code == 126 || code == 127 } // ReplayEnv is the environment mesh-lab's replays run in (novox/hq issue 709): home as HOME and configuration, the Go diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index 1e560d36..6547ac0e 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -131,7 +131,6 @@ case "$cmd" in ps|rm|exec) exit 0 ;; volume) if [ -n "$VOLUMES" ]; then echo "$*" >> "$VOLUMES"; fi - if [ "$1" = rm ] && [ -n "$REPLAY_MOUNT" ]; then rm -rf "$REPLAY_MOUNT"; fi exit 0 ;; port) echo "` + bus.Addr().String() + `"; exit 0 ;; run) @@ -151,6 +150,9 @@ run) shift if [ "$1" = go ]; then echo "GOCACHE=$GOCACHE GOENV=$GOENV $*" >> "` + built + `"; fi if [ "$1" = go ] && [ "$2" = build ] && [ -n "$JUDGE_HOMES" ]; then echo "$HOME" >> "$JUDGE_HOMES"; fi + if [ "$1" = go ] && [ "$2" = test ] && [ -n "$REPLAYS_CANNOT_START" ]; then + echo "docker: Error response from daemon: invalid mount config for type bind" >&2; exit 125 + fi if [ "$1" = go ] && [ "$2" = test ] && [ -n "$REPLAYS" ]; then echo "HOME=$HOME GOCACHE=$GOCACHE GOMODCACHE=$GOMODCACHE GOENV=$GOENV GOFLAGS=$GOFLAGS DIR=$workdir" >> "$REPLAYS" fi @@ -744,13 +746,13 @@ func aSelfJudgingControllerThatWritesIntoTheReplays(t *testing.T, marker string) return aPullRequestInto(t, files, "main", map[string]string{"cmd/x.txt": "x"}) } -// **Without a judge cache, the replays still never read what a pull request wrote** (issue 709, review 12303): the -// copy of mesh-lab, its home and throwaway caches are in a container volume of the check's own, made before anything -// of the change runs and removed after — never the workspace. With a judge cache, under it. In both: a self-judging -// controller pull request writing a test into the replays beside the check, a planted GOFLAGS hook and a poisoned -// cache entry in the workspace neither run nor are read. -func TestTheReplaysNeverReadWhatAPullRequestWroteWithOrWithoutAJudgeCache(t *testing.T) { - for _, withCache := range []bool{false, true} { +// **The replays never read what a pull request wrote** (issue 709, reviews 12303 and 12308): with a judge cache they +// run from a copy under it, and a self-judging controller pull request writing a test into the replays beside the +// check, a planted GOFLAGS hook and a poisoned cache entry in the workspace neither run nor are read. Without one the +// replays are refused — the gate an error that says why, never a pass, never the workspace — they never start, and +// nothing is made on the host. +func TestTheReplaysNeverReadWhatAPullRequestWroteAndNeedAJudgeCache(t *testing.T) { + for _, withCache := range []bool{true, false} { t.Run(fmt.Sprintf("judge cache %v", withCache), func(t *testing.T) { aRuntimeThatBuildsTheJudge(t) replays, volumes := filepath.Join(t.TempDir(), "replays"), filepath.Join(t.TempDir(), "volumes") @@ -758,11 +760,6 @@ func TestTheReplaysNeverReadWhatAPullRequestWroteWithOrWithoutAJudgeCache(t *tes t.Setenv("VOLUMES", volumes) tmp := t.TempDir() t.Setenv("TMPDIR", tmp) - mount := filepath.Join(t.TempDir(), "mesh-replays") - t.Setenv("REPLAY_MOUNT", mount) - was := ReplayMount - ReplayMount = mount - t.Cleanup(func() { ReplayMount = was }) registry := aFactsRegistry(t) workspace, ran := aPlantedWorkspace(t) if err := os.MkdirAll(filepath.Join(workspace, "go-cache"), 0o755); err != nil { @@ -786,9 +783,6 @@ func TestTheReplaysNeverReadWhatAPullRequestWroteWithOrWithoutAJudgeCache(t *tes if err != nil { t.Fatal(err) } - if v.Gate == nil || v.Gate.Verdict != "pass" || !strings.Contains(v.Report, "--- the replays") { - t.Fatalf("the replays did not run and pass: %+v\n%s", v.Gate, v.Report) - } if _, err := os.Stat(evilRan + ".wrote"); err != nil { t.Fatalf("the self-judging pull request did not write into the replays beside it, so this proves nothing: %v", err) } @@ -800,27 +794,70 @@ func TestTheReplaysNeverReadWhatAPullRequestWroteWithOrWithoutAJudgeCache(t *tes } body, _ := os.ReadFile(replays) said := strings.TrimSpace(string(body)) - if said == "" || strings.Contains(said, workspace) || !strings.Contains(said, "GOENV=off ") { - t.Fatalf("the replays ran with something of the workspace, or not at all: %q", said) - } - base := mount - if withCache { - base = filepath.Join(cache, "homes", "check-709-self-replays") - } - if !strings.Contains(said, "DIR="+filepath.Join(base, "mesh-lab", "replays")) { - t.Fatalf("the replays did not run from the copy under %s: %s", base, said) - } - vols, _ := os.ReadFile(volumes) - if !withCache && (!strings.Contains(string(vols), "create --label "+BuildLabel+"=check-709-self") || - !strings.Contains(string(vols), "rm --force mesh-check-check-709-self-replays")) { - t.Fatalf("the check's own replay volume was not made and removed: %q", vols) - } - if _, err := os.Stat(base); err == nil { - t.Fatalf("the replays' place %s was left after the check", base) + if vols, _ := os.ReadFile(volumes); len(vols) != 0 { + t.Fatalf("the check made a container volume: %q", vols) } if left, _ := os.ReadDir(tmp); len(left) != 0 { t.Fatalf("a check left %d entr(ies) in the temporary directory", len(left)) } + if !withCache { + if v.Gate == nil || v.Gate.Verdict != "error" || !strings.Contains(v.Gate.Summary, replaysNeedTheJudgeCache) || + v.Verdict != "error" { + t.Fatalf("without a judge cache the gate said %+v, not an error saying the replays need it", v.Gate) + } + if said != "" { + t.Fatalf("without a judge cache the replays started: %s", said) + } + return + } + if v.Gate == nil || v.Gate.Verdict != "pass" || !strings.Contains(v.Report, "--- the replays") { + t.Fatalf("the replays did not run and pass: %+v\n%s", v.Gate, v.Report) + } + base := filepath.Join(cache, "homes", "check-709-self-replays") + if said == "" || strings.Contains(said, workspace) || !strings.Contains(said, "GOENV=off ") || + !strings.Contains(said, "DIR="+filepath.Join(base, "mesh-lab", "replays")) { + t.Fatalf("the replays did not run from the copy under %s, apart from the workspace: %q", base, said) + } + if _, err := os.Stat(base); err == nil { + t.Fatalf("the replays' place %s was left after the check", base) + } }) } } + +// **Replays the container runtime could not start are the check's error, never the change's failure**, said in the +// build's log (review 12308); and a check whose id cannot name a place in the judge cache is refused, never given a +// shared one. +func TestReplaysThatCannotStartAreAnErrorAndAnUnsafeIDIsRefused(t *testing.T) { + aRuntimeThatBuildsTheJudge(t) + t.Setenv("REPLAYS_CANNOT_START", "1") + registry := aFactsRegistry(t) + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("the controller the mesh runs"), nil) + lab, labMain := aLabWithReplays(t) + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, "main", map[string]string{"modules/gitea/index.ts": "x"}) + spec := CheckSpec{ID: "check-709-start", Repository: catalogue, Ref: head, Owner: "novox", Repo: "mesh-catalog", + Number: 1, Base: "main", JudgeCache: t.TempDir(), Paths: []string{"modules/gitea/index.ts"}, + Modules: []string{"gitea"}, Toolchain: "golang", Beside: map[string]Beside{ + "mesh-controller": {Repository: controller, Ref: controllerMain}, "mesh-lab": {Repository: lab, Ref: labMain}}} + var logged []string + v, err := Check(t.Context(), Command, spec, t.TempDir(), registry, GitCredential{}, func(step, message string) { + logged = append(logged, message) + }) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Verdict != "error" || !strings.Contains(v.Gate.Summary, "could not be started") || + !strings.Contains(v.Gate.Summary, "invalid mount config") { + t.Fatalf("replays that could not start were said %+v, not an error naming why", v.Gate) + } + if !strings.Contains(strings.Join(logged, "\n"), "the replays could not be started") { + t.Fatalf("the build's log does not say what failed:\n%s", strings.Join(logged, "\n")) + } + + spec.ID = "Check 1/../../x" + if _, err := Check(t.Context(), Command, spec, t.TempDir(), registry, GitCredential{}, nil); err == nil || + !strings.Contains(err.Error(), "cannot name its place in the judge cache") { + t.Fatalf("a check with an unsafe id was taken: %v", err) + } +} -- 2.54.0