Build for the merge from a copy its tests never touch, and keep or take only the same head's build

The gate and the repository's check mount the workspace read-write, so a
build from that tree could hold what the tests wrote under a key that
describes the merge. A check now clones the merge outside the workspace
before anything of the change runs, and builds from that. A check keeps,
and a walk takes, only a build an earlier check of the same pull request
at the same head made: another pull request's build, or an earlier
head's, is never registered. The walk reads the verdicts once
(novox/hq issue 703, review of #328).

internal/builder/builder.go

@@ -162,6 +162,10 @@ type Reuse struct {
 	// keyFirst is a check's build (§1): its key is taken before anything is built, and a module with none is not
 	// built at all — nothing could ever take it.
 	keyFirst bool
+	// pull and head are the check's own pull request and the head it judged (novox/hq issue 703): it keeps only a
+	// build an earlier check of the same pull request at the same head made.
+	pull PullRef
+	head string
 }
 
 // errNoKey is a check's build of a module whose source does not pin it: not built (ADR 0350 §1).
@@ -498,11 +502,22 @@ func takeACheckBuild(ctx context.Context, run Runner, publish Publisher, reuse R
 }
 
 // keptCheckBuild is the build a check keeps instead of building (novox/hq issue 703): the one kept under the
-// module's key, when a verdict of the build seat names its key and every digest — the record alone is never trusted
-// (ADR 0350 §2) — and it holds every artifact the module declares. Nil, and the check builds, otherwise.
+// module's key by an earlier check of the same pull request at the same head — only the trunk moved — when a verdict
+// of the build seat names its key and every digest — the record alone is never trusted (ADR 0350 §2) — and it holds
+// every artifact the module declares. Another pull request's build, or an earlier head's, is never kept. Nil, and
+// the check builds, otherwise.
 func keptCheckBuild(ctx context.Context, reuse Reuse, module, key string, artifacts int,
 	say func(step, format string, args ...any)) *Reused {
-	if reuse.Scratch == nil || len(reuse.Named) == 0 {
+	if reuse.Scratch == nil || reuse.head == "" {
+		return nil
+	}
+	var same []NamedBuild
+	for _, n := range reuse.Named {
+		if n.Pull == reuse.pull {
+			same = append(same, n)
+		}
+	}
+	if len(same) == 0 {
 		return nil
 	}
 	found, ok, err := reuse.Scratch.Find(ctx, module, key)
@@ -512,7 +527,11 @@ func keptCheckBuild(ctx context.Context, reuse Reuse, module, key string, artifa
 		return nil
 	case !ok:
 		return nil
-	case unnamed(reuse.Named, module, key, found) != "":
+	case found.Pull != reuse.pull || found.Head == "" || twelve(found.Head) != twelve(reuse.head):
+		say("reuse", "the build kept under %s is %s, not this pull request's at this head: built", twelve(key[3:]),
+			found.Said())
+		return nil
+	case unnamed(same, module, key, found) != "":
 		say("reuse", "a build is kept under %s and no verdict names it with its digests: built", twelve(key[3:]))
 		return nil
 	case len(found.Made) != artifacts:

internal/builder/check.go

@@ -354,6 +354,22 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 		v.Head, v.Trunk, v.Merge, v.Beside, v.Members = judged.Head, judged.Trunk, judged.Merge, judged.Beside, judged.Members
 		return v
 	}
+	// **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 := "", ""
+	if len(spec.Builds) > 0 && spec.BuildWith != nil && len(spec.Group) == 0 {
+		at := judged.Merge
+		if at == "" {
+			at = spec.Ref
+		}
+		building, buildingTree, err = checkoutForTheBuilds(ctx, run, tree, at, workspace)
+		if err != nil {
+			return CheckVerdict{}, err
+		}
+		defer os.RemoveAll(building)
+	}
 	// **The base branch's check judges the change** (novox/hq issue 310), never the one the change brings.
 	judging, err := TheCheckThatJudges(ctx, tree, spec.Base)
 	if err != nil {
@@ -610,7 +626,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 			say("check", "%s", why)
 		} else {
 			began := time.Now()
-			v.Built = buildForTheMerge(ctx, run, spec, *spec.BuildWith, workspace, tree, judged.Merge, judged, forge,
+			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))
@@ -623,6 +639,39 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 	return named(v), nil
 }
 
+// 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
+// workspace's objects cannot reach it.
+func checkoutForTheBuilds(ctx context.Context, run Runner, tree, at, workspace string) (string, string, error) {
+	dir, err := os.MkdirTemp("", "mesh-check-builds-")
+	if err != nil {
+		return "", "", err
+	}
+	if withinDir(workspace, dir) {
+		os.RemoveAll(dir)
+		return "", "", fmt.Errorf("the temporary directory %s is inside the workspace %s, which a check's containers "+
+			"mount: the merge is not built from where its tests could change it", dir, workspace)
+	}
+	commit, err := run(ctx, tree, "git", "rev-parse", "--verify", at+"^{commit}")
+	if err != nil {
+		os.RemoveAll(dir)
+		return "", "", fmt.Errorf("the commit to build for the merge, %s: %w", at, err)
+	}
+	commit = strings.TrimSpace(commit)
+	if _, err := run(ctx, tree, "git", "update-ref", "refs/heads/mesh-check-builds", commit); err != nil {
+		os.RemoveAll(dir)
+		return "", "", err
+	}
+	cloned := filepath.Join(dir, "merged")
+	if _, err := run(ctx, dir, "git", "clone", "--quiet", "--no-local", "--no-checkout", "--branch",
+		"mesh-check-builds", tree, cloned); err != nil {
+		os.RemoveAll(dir)
+		return "", "", fmt.Errorf("cannot copy the merge to build it: %w", err)
+	}
+	return dir, cloned, nil
+}
+
 // builtCount is how many of a check's builds for its merge were built.
 func builtCount(built []CheckBuilt) int {
 	n := 0

internal/builder/check_builds.go

@@ -37,9 +37,10 @@ type CheckBuilding struct {
 	Held    map[string]string
 	Npmrc   Npmrc
 	Seats   map[string]string
-	// Named are the builds the build seat's verdicts name for the modules this check builds, of any pull request,
-	// within the week check builds are kept (novox/hq issue 703): a build kept under a module's key is kept again,
-	// instead of built, only when one of them names its key and every digest. The record alone is never enough.
+	// Named are the builds the build seat's verdicts of earlier checks of this pull request at this head name
+	// (novox/hq issue 703): a build kept under a module's key is kept again, instead of built, only when one of them
+	// names its key and every digest, and the build is this pull request's at this head. The record alone is never
+	// enough.
 	Named []NamedBuild
 }
 
@@ -108,7 +109,8 @@ func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Chec
 		var mirrored []string
 		result, err := build(ctx, run, scratchPublisher{to: with.Publish}, repository, b.Path, at,
 			filepath.Join(workspace, "for-the-merge"), held, with.Npmrc, forge, log, &mirrored,
-			&Reuse{keyFirst: true, Scratch: with.Scratch, Named: with.Named}, with.Seats)
+			&Reuse{keyFirst: true, Scratch: with.Scratch, Named: with.Named,
+				pull: PullRef{Owner: spec.Owner, Repo: spec.Repo, Number: spec.Number}, head: judged.Head}, with.Seats)
 		built := CheckBuilt{Module: b.Module, Key: result.ReuseKey, Took: time.Since(began)}
 		switch {
 		case errors.Is(err, errNoKey):

internal/builder/check_merge_test.go

@@ -210,3 +210,53 @@ func TestACheckMergeRunsNoHook(t *testing.T) {
 		t.Fatalf("a hook ran when the check made its merge: %s", said)
 	}
 }
+
+// What a check's tests change in the workspace never reaches what it builds for the merge (novox/hq issue 703): the
+// builds are made from a copy of the merge, outside the workspace, cloned before anything of the change runs.
+func TestWhatTheTestsChangeInTheWorkspaceNeverReachesTheBuild(t *testing.T) {
+	repo, head, _ := aTrunkAndABranch(t, map[string]string{"main.go": "package main // the merge\n"},
+		map[string]string{"trunk-only": "t"}, map[string]string{"head-only": "h"})
+	workspace := t.TempDir()
+	tree := filepath.Join(workspace, "check", "checked")
+	if out, err := exec.Command("git", "clone", "--quiet", repo, tree).CombinedOutput(); err != nil {
+		t.Fatalf("%v\n%s", err, out)
+	}
+	dir, cloned, err := checkoutForTheBuilds(t.Context(), Command, tree, head, workspace)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+	if withinDir(workspace, cloned) {
+		t.Fatalf("the builds' checkout %s is inside the workspace %s the tests mount", cloned, workspace)
+	}
+
+	// The tests rewrite the tree, its branch and every object it holds.
+	if err := os.WriteFile(filepath.Join(tree, "main.go"), []byte("package main // planted\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := filepath.WalkDir(filepath.Join(tree, ".git", "objects"), func(path string, d os.DirEntry, err error) error {
+		if err != nil || d.IsDir() {
+			return err
+		}
+		_ = os.Chmod(path, 0o644)
+		return os.WriteFile(path, []byte("planted"), 0o644)
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	// The build clones the merge from its own checkout, as buildForTheMerge does, and reads what the merge holds.
+	build := filepath.Join(t.TempDir(), "source")
+	if out, err := exec.Command("git", "clone", "--quiet", cloned, build).CombinedOutput(); err != nil {
+		t.Fatalf("%v\n%s", err, out)
+	}
+	if out, err := exec.Command("git", "-C", build, "checkout", "--quiet", head).CombinedOutput(); err != nil {
+		t.Fatalf("%v\n%s", err, out)
+	}
+	body, err := os.ReadFile(filepath.Join(build, "main.go"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(body) != "package main // the merge\n" {
+		t.Fatalf("the build read %q: what the tests wrote reached it", body)
+	}
+}

internal/builder/reuse_test.go

@@ -630,13 +630,19 @@ func TestABuildTwoChecksShareGoesWithTheLastOfThem(t *testing.T) {
 // aCheckAgain is a later check (build-2) of aPull building the module from a tree, with the builds the build seat's
 // verdicts name for it (novox/hq issue 703).
 func aCheckAgain(t *testing.T, scratch *aScratch, tree string, leave []NamedBuild) (CheckBuilt, *recorded) {
+	t.Helper()
+	return aCheckOf(t, scratch, aPull, "abcdefabcdef", tree, leave)
+}
+
+// aCheckOf is a later check (build-2) of a pull request at a head, the trunk moved since build-1.
+func aCheckOf(t *testing.T, scratch *aScratch, pull PullRef, head, tree string, leave []NamedBuild) (CheckBuilt, *recorded) {
 	t.Helper()
 	r, run, workspace := aModuleAt(t, tree)
-	spec := CheckSpec{ID: "build-2", Owner: aPull.Owner, Repo: aPull.Repo, Number: aPull.Number,
+	spec := CheckSpec{ID: "build-2", Owner: pull.Owner, Repo: pull.Repo, Number: pull.Number,
 		Builds: []CheckBuildSpec{{Module: "meshboard", Repository: "https://forge.invalid/meshboard.git", Path: "mod",
 			Ref: "main"}}}
 	built := buildForTheMerge(t.Context(), run, spec, CheckBuilding{Publish: r, Scratch: scratch, Named: leave},
-		workspace, "", "", CheckVerdict{Head: "fedcbafedcba", Trunk: "654321654321"}, GitCredential{}, nil, logging(nil))
+		workspace, "", "", CheckVerdict{Head: head, Trunk: "654321654321"}, GitCredential{}, nil, logging(nil))
 	if len(built) != 1 || built[0].NotBuilt != "" || built[0].Key == "" {
 		t.Fatalf("the second check did not answer the module for its merge: %+v", built)
 	}
@@ -745,3 +751,28 @@ func TestABuildALaterCheckKeptStaysWhenTheCheckThatBuiltItGoes(t *testing.T) {
 		t.Fatalf("the build stayed after the last check naming it went: held %v, record %v", held(), found())
 	}
 }
+
+// Only an earlier check of the same pull request at the same head is kept from: another head of it, or another pull
+// request whose verdict names the same key and digests, builds (novox/hq issue 703).
+func TestACheckKeepsNoBuildOfAnotherHeadOrPullRequest(t *testing.T) {
+	withVersion(t, "builder-1")
+	other := PullRef{Owner: "novox", Repo: "meshboard", Number: 8}
+	for name, c := range map[string]struct {
+		pull PullRef
+		head string
+	}{
+		"another head":         {aPull, "999999999999"},
+		"another pull request": {other, "abcdefabcdef"},
+	} {
+		t.Run(name, func(t *testing.T) {
+			scratch := newScratch()
+			first := aCheckBuilt(t, scratch, "tree-one")
+			leave := named(first)
+			leave[0].Pull = c.pull // as if its own verdict named it
+			again, r := aCheckOf(t, scratch, c.pull, c.head, "tree-one", leave)
+			if !dockerBuilt(r) || again.Kept != "" {
+				t.Fatalf("a build of %s was kept: %+v", name, again)
+			}
+		})
+	}
+}

internal/inventory/plans.go

@@ -104,6 +104,9 @@ type PlanMerge struct {
 	Number int      `json:"number,omitempty"`
 	Title  string   `json:"title,omitempty"`
 	Moves  []string `json:"moves,omitempty"`
+	// Head is the pull request's head commit the forge merged (novox/hq issue 703): the walk takes only a build a
+	// check of that head made. Empty where the announcement said none, and then it takes none.
+	Head string `json:"head,omitempty"`
 	// Merged is when the forge made the merge and Heard when the controller heard it (novox/hq ADR 0282): where
 	// its delivery time starts. Zero for a merge named before they were kept.
 	Merged time.Time `json:"merged,omitzero"`

internal/link/build.go

@@ -114,11 +114,23 @@ type ReuseAsk struct {
 
 // CheckedBuild is one build a check's verdict names: the pull request it checked, its ask, and the build.
 type CheckedBuild struct {
-	Pull  PullRef    `json:"pull"`
-	Check string     `json:"check"`
+	Pull  PullRef `json:"pull"`
+	Check string  `json:"check"`
+	// Head is the pull request's head the check judged, as its verdict says it (twelve characters): a build is
+	// taken or kept only by a check of the same head (novox/hq issue 703).
+	Head  string     `json:"head,omitempty"`
 	Built CheckBuilt `json:"built"`
 }
 
+// SameHead says two spellings of a commit name the same one: both said, one the other's beginning, at least
+// twelve characters of it (a verdict says twelve).
+func SameHead(a, b string) bool {
+	if len(a) > len(b) {
+		a, b = b, a
+	}
+	return len(a) >= 12 && strings.HasPrefix(b, a)
+}
+
 // PullRef names a pull request.
 type PullRef struct {
 	Owner  string `json:"owner"`
@@ -164,9 +176,10 @@ type CheckRequest struct {
 	// each, from the merge it judged, into the scratch namespace once it has judged and not failed, for the walk to
 	// take. A build seat from before it builds nothing, and the walk builds as before.
 	Builds []CheckBuild `json:"builds,omitempty"`
-	// Named are the builds the seat's verdicts name for those modules, of any pull request, within the week check
-	// builds are kept (novox/hq issue 703): the check keeps a build kept under a module's unchanged key, instead of
-	// building it again, only when one of these names its key and every digest. A build seat from before it builds.
+	// Named are the builds the seat's verdicts of earlier checks of this pull request at this head name, within the
+	// week check builds are kept (novox/hq issue 703): asked again because only the trunk moved, the check keeps a
+	// build kept under a module's unchanged key, instead of building it again, only when one of these names its key
+	// and every digest. Never another pull request's, nor an earlier head's. A build seat from before it builds.
 	Named []CheckedBuild `json:"named,omitempty"`
 	// Try marks a branch tried before its pull request (novox/hq issue 686): the repository's own check for a
 	// pushed branch or commit, asked by a builder through the controller's `try` verb. Its outcome is read from

internal/link/check_builds_test.go

@@ -38,9 +38,9 @@ func TestNatsTheBuildsAWalkMayTakeAreTheChecksOutcomesOfItsPullRequests(t *testi
 	}
 }
 
-// A check reads the builds every pull request's checks made of the modules it builds, and a kept build is named as
-// the check's that built it (novox/hq issue 703).
-func TestNatsACheckReadsTheBuildsOfItsModulesAndAKeptBuildIsTheBuildersCheck(t *testing.T) {
+// A build is read with the head its check judged, and a kept build is named as the check's that built it (novox/hq
+// issue 703).
+func TestNatsAKeptBuildIsTheBuildersCheckAndEveryBuildSaysItsHead(t *testing.T) {
 	js := aBus(t)
 	since := time.Now().Add(-time.Minute)
 	say := func(r BuildResult) {
@@ -49,21 +49,39 @@ func TestNatsACheckReadsTheBuildsOfItsModulesAndAKeptBuildIsTheBuildersCheck(t *
 			t.Fatal(err)
 		}
 	}
+	mine := &CheckRequest{Owner: "novox", Repo: "shop", Number: 7}
 	app := CheckBuilt{Module: "app", Key: "r1:1", Made: []MadeArtifact{{Name: "server", Reference: "s/app/server@sha256:a"}}}
-	web := CheckBuilt{Module: "web", Key: "r1:2", Made: []MadeArtifact{{Name: "server", Reference: "s/web/server@sha256:b"}}}
-	say(BuildResult{ID: "build-1", Checked: &CheckRequest{Owner: "novox", Repo: "shop", Number: 7},
-		Check: &CheckOutcome{Verdict: "pass", Built: []CheckBuilt{app, web}}})
+	say(BuildResult{ID: "build-1", Checked: mine, Check: &CheckOutcome{Verdict: "pass", Head: "aaaaaaaaaaaa",
+		Built: []CheckBuilt{app}}})
 	kept := app
 	kept.Kept = "build-1"
-	say(BuildResult{ID: "build-2", Checked: &CheckRequest{Owner: "novox", Repo: "till", Number: 3},
-		Check: &CheckOutcome{Verdict: "pass", Built: []CheckBuilt{kept}}})
+	say(BuildResult{ID: "build-2", Checked: mine, Check: &CheckOutcome{Verdict: "pass", Head: "aaaaaaaaaaaa",
+		Built: []CheckBuilt{kept}}})
 
-	got, err := ReadCheckBuildsOf(t.Context(), js, TheBuildMachine, []string{"app"}, since)
+	got, err := ReadCheckBuilds(t.Context(), js, TheBuildMachine, []PullRef{{Owner: "novox", Repo: "shop", Number: 7}}, since)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(got) != 2 || got[0].Check != "build-1" || got[0].Pull.Repo != "till" || got[1].Check != "build-1" ||
-		got[1].Built.Module != "app" {
+	if len(got) != 2 || got[0].Check != "build-1" || got[0].Built.Kept != "build-1" || got[0].Head != "aaaaaaaaaaaa" ||
+		got[1].Check != "build-1" || got[1].Head != "aaaaaaaaaaaa" {
 		t.Fatalf("read %+v", got)
 	}
 }
+
+// Two spellings of a head are the same head only when both say at least twelve characters of it.
+func TestSameHeadNeedsTwelveCharactersOfBoth(t *testing.T) {
+	for _, c := range []struct {
+		a, b string
+		same bool
+	}{
+		{"aaaaaaaaaaaa", "aaaaaaaaaaaabbbb", true},
+		{"aaaaaaaaaaaabbbb", "aaaaaaaaaaaa", true},
+		{"aaaaaaaaaaaa", "aaaaaaaaaaab", false},
+		{"", "", false},
+		{"aaaa", "aaaaaaaaaaaa", false},
+	} {
+		if SameHead(c.a, c.b) != c.same {
+			t.Errorf("SameHead(%q, %q) is %v", c.a, c.b, !c.same)
+		}
+	}
+}

internal/link/events.go

@@ -172,10 +172,13 @@ type Announcement struct {
 // which branch, producing which commit. The mesh matches it against every module's recorded
 // source and builds what moved, bases first.
 type SourceMoved struct {
-	Owner    string `json:"owner"`
-	Repo     string `json:"repo"`
-	Base     string `json:"base"`
-	Head     string `json:"head"`
+	Owner string `json:"owner"`
+	Repo  string `json:"repo"`
+	Base  string `json:"base"`
+	Head  string `json:"head"`
+	// HeadSHA is the head commit the forge merged (novox/hq issue 703): a walk takes only a build a check of that
+	// head made. Empty from a forge module from before it, and then the walk takes no check's build.
+	HeadSHA  string `json:"head_sha,omitempty"`
 	Commit   string `json:"merge_commit_sha"`
 	CloneURL string `json:"clone_url"`
 	HTMLURL  string `json:"html_url"`

internal/link/queue.go

@@ -378,19 +378,6 @@ func ReadCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, pul
 	return readCheckBuilds(ctx, js, seat, since, func(p PullRef, _ CheckBuilt) bool { return wanted[p] })
 }
 
-// ReadCheckBuildsOf is every build the seat's checks of any pull request said they made, or kept, of those modules
-// since a moment (novox/hq issue 703): what a check may keep instead of building. Newest first.
-func ReadCheckBuildsOf(ctx context.Context, js *broker.JetStream, seat string, modules []string, since time.Time) ([]CheckedBuild, error) {
-	if len(modules) == 0 {
-		return nil, nil
-	}
-	wanted := map[string]bool{}
-	for _, m := range modules {
-		wanted[m] = true
-	}
-	return readCheckBuilds(ctx, js, seat, since, func(_ PullRef, b CheckBuilt) bool { return wanted[b.Module] })
-}
-
 // readCheckBuilds is every build the seat's checks said they made, or kept, that wanted takes. A kept build is named
 // as the check's that built it (novox/hq issue 703): the record under its key is that check's.
 func readCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, since time.Time,
@@ -428,7 +415,7 @@ func readCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, sin
 					if b.Kept != "" {
 						check = b.Kept
 					}
-					out = append(out, CheckedBuild{Pull: pull, Check: check, Built: b})
+					out = append(out, CheckedBuild{Pull: pull, Check: check, Head: r.Check.Head, Built: b})
 				}
 			}
 		}