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

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

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