Keep judge builds warm in the build agent's judge cache, and leave nothing on the host

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 <noreply@anthropic.com>

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.

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)
 	}
 }

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
 

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)
+	}
+}