Build a check's builds for its merge where the host's container engine sees them (hq issue 713)

The check built in a temporary directory of the build holder's container, which the
host's engine mounted empty: every check build failed and every walk built again.

internal/builder/check.go

@@ -357,13 +357,16 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 	// **What the merge would move is built from a checkout nothing of the check touches** (novox/hq issue 703): the
 	// gate and the repository's own check run in containers that mount the workspace read-write, so they can change
 	// its tree, its objects and its caches. The builds are made from a copy of the merge's objects, cloned before
-	// anything of the change runs, outside the workspace — so what is kept under a key is what the key describes.
-	building, buildingTree := "", ""
+	// anything of the change runs, outside the workspace — so what is kept under a key is what the key describes. Each
+	// build's working copy is cloned from it into the workspace only after the check has ended (forTheMerge): the
+	// host's container engine sees no other directory of the holder's (novox/hq issue 713).
+	buildingTree := ""
 	if len(spec.Builds) > 0 && spec.BuildWith != nil && len(spec.Group) == 0 {
 		at := judged.Merge
 		if at == "" {
 			at = spec.Ref
 		}
+		var building string
 		building, buildingTree, err = checkoutForTheBuilds(ctx, run, tree, at, workspace)
 		if err != nil {
 			return CheckVerdict{}, err
@@ -622,23 +625,35 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 	// **What the merge would move, built once, here** (novox/hq ADR 0350 §1): after the judging, from the merge it
 	// judged, into the scratch namespace — so the walk of the merge commit takes it rather than building again.
 	if len(spec.Builds) > 0 && spec.BuildWith != nil && len(spec.Group) == 0 {
-		if why := skipsTheBuilds(v); why != "" {
-			say("check", "%s", why)
-		} else {
-			began := time.Now()
-			v.Built = buildForTheMerge(ctx, run, spec, *spec.BuildWith, building, buildingTree, judged.Merge, judged, forge,
-				func(step, message string) {
-					if log != nil {
-						log(step, redact.redact(message))
-					}
-				}, say)
-			say("check", "built for the merge: %d of %d module(s) (%s)", builtCount(v.Built), len(v.Built),
-				time.Since(began).Round(time.Second))
-		}
+		v.Built = forTheMerge(ctx, run, spec, v, workspace, buildingTree, judged, forge, func(step, message string) {
+			if log != nil {
+				log(step, redact.redact(message))
+			}
+		}, say)
 	}
 	return named(v), nil
 }
 
+// forTheMerge builds what the merge moves once the check has judged it, from merged — the copy of the merge
+// checkoutForTheBuilds made outside the workspace — **in the builder's own workspace** (novox/hq issue 713). A build's
+// containers are run by the host's container engine, which mounts a path as the host names it: the build seat's
+// holder runs in a container that shares only its workspace with the host, at the same path, so a working copy in
+// its own temporary directory reaches the toolchain as an empty directory ("go.mod file not found"), every check
+// build failed, and every walk built again. The copy stays outside the workspace, so nothing the tests ran reaches it;
+// the working copy cloned from it is made only now, after every container of the check has ended.
+func forTheMerge(ctx context.Context, run Runner, spec CheckSpec, verdict CheckVerdict, workspace, merged string,
+	judged CheckVerdict, forge GitCredential, log Log, say func(step, format string, args ...any)) []CheckBuilt {
+	if why := skipsTheBuilds(verdict); why != "" {
+		say("check", "%s", why)
+		return nil
+	}
+	began := time.Now()
+	built := buildForTheMerge(ctx, run, spec, *spec.BuildWith, workspace, merged, judged.Merge, judged, forge, log, say)
+	say("check", "built for the merge: %d of %d module(s) (%s)", builtCount(built), len(built),
+		time.Since(began).Round(time.Second))
+	return built
+}
+
 // checkoutForTheBuilds is a clone of the commit a check builds for its merge, made before anything of the check runs,
 // outside the workspace its containers mount (novox/hq issue 703): the directory to remove, and the clone in it.
 // Cloned over git's own transport, never by linking the objects: what the check's containers later write into the

internal/builder/check_builds.go

@@ -70,7 +70,9 @@ func skipsTheBuilds(v CheckVerdict) string {
 
 // buildForTheMerge builds each module the plan moves, tier by tier, into the scratch namespace, and keeps what it
 // built under its keys. A module standing after one not built is not built either: what it stands on in the walk
-// is not what it would stand on here. tree is the check's clone and merge the merge commit it made there.
+// is not what it would stand on here. tree is the check's clone and merge the merge commit it made there; workspace
+// is where each build's working copy is made, a directory the host's container engine sees at the same path — the
+// builder's own workspace, never a temporary directory of its container (novox/hq issue 713).
 func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with CheckBuilding, workspace, tree, merge string,
 	judged CheckVerdict, forge GitCredential, log Log, say func(step, format string, args ...any)) []CheckBuilt {
 	if len(spec.Builds) == 0 || with.Publish == nil || with.Scratch == nil {
@@ -80,6 +82,8 @@ func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Chec
 	if ref == "" {
 		ref = spec.Ref
 	}
+	// What an earlier check left, ended before it could remove it, is not built on.
+	_ = os.RemoveAll(filepath.Join(workspace, "for-the-merge"))
 	held := map[string]string{}
 	for k, v := range with.Held {
 		held[k] = v

internal/builder/replay713_test.go

@@ -0,0 +1,125 @@
+package builder
+
+import (
+	"context"
+	"errors"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// novox/hq issue 713: after ADR 0350 the walk of novox/mesh-controller#331 built mesh-controller, build-agent and
+// route-proxy again, each "built: no check of novox/mesh-controller#331 built it". The check had built none: its
+// build log said "built for the merge: 0 of 3 module(s)", mesh-controller's go bundle failing with "go.mod file not
+// found". The build seat's holder runs in a container sharing only its workspace with the host, and a build's
+// toolchain container is run by the host's container engine, which mounts a path as the host names it. The check
+// built in a temporary directory of the holder's own (issue 703's copy), which the engine mounted as an empty one.
+
+// aHostsEngine runs as the host's container engine does for a holder in a container: a mount of a path outside the
+// workspace the holder shares with the host is an empty directory, and a Go toolchain finds no go.mod in it.
+type aHostsEngine struct {
+	onTrunk
+	shared string
+}
+
+func (h aHostsEngine) run(ctx context.Context, dir, name string, args ...string) (string, error) {
+	if name == "docker" && len(args) > 0 && args[0] == "run" {
+		for i, a := range args {
+			if a != "--volume" || i+1 >= len(args) {
+				continue
+			}
+			from, _, _ := strings.Cut(args[i+1], ":")
+			if !withinDir(h.shared, from) {
+				return "", errors.New("go: go.mod file not found in current directory or any parent directory")
+			}
+		}
+	}
+	return h.onTrunk.run(ctx, dir, name, args...)
+}
+
+// aGoModule is a module of one Go bundle built from a repository's root.
+func aGoModule(module string) map[string]string {
+	files := aSharedRepository("one", "1")
+	files[ManifestName] = `{"module":"` + module + `","version":"1","build":{"artifacts":[
+	  {"name":"server","kind":"bundle","language":"go","system":"arch","from":"cmd/ctl","binary":"ctl"}]},
+	  "resources":[{"id":"server","type":"process","name":"` + module + `","artifact":"server","run":["./ctl"]}]}`
+	return files
+}
+
+// A controller change whose walk moves mesh-controller, build-agent and route-proxy (#331's) has its check build all
+// three, in the workspace the host's engine sees, and the walk of its merge takes each from the check by its key.
+func TestReplay713TheWalkOfAControllerChangeTakesAllThreeFromItsCheck(t *testing.T) {
+	withVersion(t, "builder-1")
+	pull := PullRef{Owner: "novox", Repo: "mesh-controller", Number: 331}
+	held := map[string]string{"mesh-tools-go/build": "registry.invalid/mesh-tools-go/build@sha256:" + strings.Repeat("b", 64)}
+	repositories := map[string]string{"build-agent": "https://forge.invalid/mesh-catalog-build-agent.git",
+		"route-proxy": "https://forge.invalid/mesh-catalog-route-proxy.git"}
+	checked := func(shared string) (*recorded, aHostsEngine) {
+		r := &recorded{tree: "tree-of-the-merge", contents: aGoModule("mesh-controller"),
+			secondary: map[string]map[string]string{}}
+		for module, repository := range repositories {
+			r.secondary[repository] = aGoModule(module)
+		}
+		return r, aHostsEngine{onTrunk: onTrunk{recorded: r}, shared: shared}
+	}
+	spec := CheckSpec{ID: "build-331", Owner: pull.Owner, Repo: pull.Repo, Number: pull.Number,
+		Builds: []CheckBuildSpec{{Module: "mesh-controller", Merged: true, Tier: 0},
+			{Module: "build-agent", Repository: repositories["build-agent"], Ref: "main", Tier: 1},
+			{Module: "route-proxy", Repository: repositories["route-proxy"], Ref: "main", Tier: 2}}}
+	judged := CheckVerdict{Head: "98e856874913", Trunk: "c975dbf7b053", Merge: "2d22279b1d07"}
+	passed := CheckVerdict{Gate: &Layer{Verdict: "pass"}, Repo: &Layer{Verdict: "pass"}}
+
+	// As it ran: the builds made in the holder's own temporary directory, which the host's engine does not see.
+	shared, outside := t.TempDir(), t.TempDir()
+	merged := filepath.Join(outside, "merged")
+	r, engine := checked(shared)
+	asItRan := buildForTheMerge(t.Context(), engine.run, spec, CheckBuilding{Publish: r, Scratch: newScratch(), Held: held},
+		outside, merged, judged.Merge, judged, GitCredential{}, nil, logging(nil))
+	if builtCount(asItRan) != 0 {
+		t.Fatalf("the engine of this test sees the holder's temporary directory: %+v", asItRan)
+	}
+
+	// The check as it builds now: every module its merge moves, kept under its key.
+	scratch := newScratch()
+	spec.BuildWith = &CheckBuilding{Publish: r, Scratch: scratch, Held: held}
+	built := forTheMerge(t.Context(), engine.run, spec, passed, shared, merged, judged, GitCredential{}, nil,
+		logging(nil))
+	if len(built) != 3 || builtCount(built) != 3 {
+		t.Fatalf("the check built %d of %d module(s) its merge moves: %+v", builtCount(built), len(built), built)
+	}
+	var names []NamedBuild
+	for _, b := range built {
+		n := NamedBuild{Pull: pull, Check: spec.ID, Module: b.Module, Key: b.Key, Digests: map[string]string{}}
+		for _, m := range b.Made {
+			d := m.Digest
+			if d == "" {
+				d = DigestOf(m.Reference)
+			}
+			n.Digests[m.Name] = d
+		}
+		names = append(names, n)
+	}
+
+	// The walk of the merge commit, tier by tier: each module taken from the check, none built.
+	walked := map[string]string{"mesh-controller": "https://forge.invalid/mesh-controller.git"}
+	for module, repository := range repositories {
+		walked[module] = repository
+	}
+	for _, module := range []string{"mesh-controller", "build-agent", "route-proxy"} {
+		w, walk := checked(shared)
+		w.secondary[walked["mesh-controller"]] = aGoModule("mesh-controller")
+		got, err := BuildReusing(t.Context(), walk.run, w, Reuse{Scratch: scratch, Pulls: []PullRef{pull}, Named: names},
+			walked[module], "", "main", t.TempDir(), held, Npmrc{}, GitCredential{}, nil)
+		if err != nil {
+			t.Fatalf("%s: %v", module, err)
+		}
+		if got.Reused == nil || got.Why != "" {
+			t.Fatalf("the walk built %s again: %s", module, got.Why)
+		}
+		for _, line := range w.ran {
+			if strings.HasPrefix(line, "docker run") && strings.Contains(line, "go build") {
+				t.Fatalf("the walk compiled %s: %s", module, line)
+			}
+		}
+	}
+}