Keep a check build whose key is unchanged instead of building it again

A fresh check asked when only the trunk moved rebuilt every image, though
the reuse key said nothing it is built from changed: a controller check
took about 12 minutes instead of 5.5. The check now keeps a build kept
under its key when a verdict of the build seat names that key and every
digest, names it as the build's own check so a walk takes it, and the
cleanup keeps it while any check names it (novox/hq issue 703).

cmd/mesh-builder/main.go

@@ -319,7 +319,7 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri
 		// at build time has no reuse key anyway, so nothing is lost by refusing it here.
 		if isRegistry && len(spec.Builds) > 0 {
 			spec.BuildWith = &builder.CheckBuilding{Publish: publisher, Scratch: registryHere, Held: request.Held,
-				Seats: request.Seats}
+				Seats: request.Seats, Named: namedOf(request.Check.Named)}
 		}
 		var v builder.CheckVerdict
 		v, err = builder.Check(building, builder.Command, spec, workspace, registry, forgeFrom(), say)
@@ -352,15 +352,7 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri
 				reuse.Pulls = append(reuse.Pulls, builder.PullRef{Owner: p.Owner, Repo: p.Repo, Number: p.Number})
 			}
 			// Only what a verdict names is taken (ADR 0350 §2).
-			for _, b := range request.Reuse.Builds {
-				named := builder.NamedBuild{Pull: builder.PullRef{Owner: b.Pull.Owner, Repo: b.Pull.Repo,
-					Number: b.Pull.Number}, Check: b.Check, Module: b.Built.Module, Key: b.Built.Key,
-					Digests: map[string]string{}}
-				for _, m := range b.Built.Made {
-					named.Digests[m.Name] = builder.DigestOf(m.Reference)
-				}
-				reuse.Named = append(reuse.Named, named)
-			}
+			reuse.Named = namedOf(request.Reuse.Builds)
 			built, err = builder.BuildReusing(building, builder.Labelled(builder.Command, request.ID), publisher, reuse,
 				request.Repository, request.Path, request.Ref, workspace, request.Held, npmrc,
 				forgeFrom(), say, request.Seats)
@@ -496,11 +488,28 @@ func triedOf(v builder.CheckVerdict) link.Tried {
 	return t
 }
 
+// namedOf are the builds the seat's verdicts name, as the builder believes a scratch record by them (novox/hq ADR
+// 0350 §2, issue 703): each module's key with every artifact's digest.
+func namedOf(builds []link.CheckedBuild) []builder.NamedBuild {
+	var out []builder.NamedBuild
+	for _, b := range builds {
+		named := builder.NamedBuild{Pull: builder.PullRef{Owner: b.Pull.Owner, Repo: b.Pull.Repo,
+			Number: b.Pull.Number}, Check: b.Check, Module: b.Built.Module, Key: b.Built.Key,
+			Digests: map[string]string{}}
+		for _, m := range b.Built.Made {
+			named.Digests[m.Name] = builder.DigestOf(m.Reference)
+		}
+		out = append(out, named)
+	}
+	return out
+}
+
 // checkBuiltOf is a check's builds for its merge as the outcome carries them (novox/hq ADR 0350 §1).
 func checkBuiltOf(built []builder.CheckBuilt) []link.CheckBuilt {
 	var out []link.CheckBuilt
 	for _, b := range built {
-		c := link.CheckBuilt{Module: b.Module, Key: b.Key, NotBuilt: b.NotBuilt, Took: b.Took.Round(time.Second).String()}
+		c := link.CheckBuilt{Module: b.Module, Key: b.Key, NotBuilt: b.NotBuilt, Took: b.Took.Round(time.Second).String(),
+			Kept: b.Kept}
 		for _, m := range b.Made {
 			c.Made = append(c.Made, link.MadeArtifact{Name: m.Name, Kind: m.Kind, Reference: m.Reference})
 		}

cmd/mesh-controller/check_builds_test.go

@@ -214,3 +214,28 @@ func TestAWalksLeaveNamesTheBuildsItsPullRequestsVerdictsMade(t *testing.T) {
 		t.Fatalf("verdicts that could not be read named %+v", got.Builds)
 	}
 }
+
+// A check is handed the builds the seat's verdicts name for the modules it builds, read once, and none when they
+// cannot be read (novox/hq issue 703).
+func TestACheckIsHandedTheBuildsTheVerdictsNameForItsModules(t *testing.T) {
+	was := checkedBuildsOfModules
+	t.Cleanup(func() { checkedBuildsOfModules = was })
+	var asked [][]string
+	named := []link.CheckedBuild{{Pull: link.PullRef{Owner: "novox", Repo: "shop", Number: 7}, Check: "build-1",
+		Built: link.CheckBuilt{Module: "app", Key: "r1:1"}}}
+	checkedBuildsOfModules = func(_ context.Context, modules []string) ([]link.CheckedBuild, error) {
+		asked = append(asked, modules)
+		return named, nil
+	}
+	builds := []link.CheckBuild{{Module: "app"}, {Module: "web", Skip: "the merge deletes it"}}
+	if got := namedForTheCheck(t.Context(), builds); !reflect.DeepEqual(got, named) ||
+		!reflect.DeepEqual(asked, [][]string{{"app"}}) {
+		t.Fatalf("the check is handed %+v, read %v", got, asked)
+	}
+	checkedBuildsOfModules = func(context.Context, []string) ([]link.CheckedBuild, error) {
+		return nil, errors.New("no bus")
+	}
+	if got := namedForTheCheck(t.Context(), builds); got != nil {
+		t.Fatalf("verdicts that could not be read named %+v", got)
+	}
+}

cmd/mesh-controller/checks.go

@@ -273,6 +273,7 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop
 			}
 		}
 	}
+	builds := checkBuildsOf(p, scope, entries, clone)
 	return link.BuildRequest{
 		ID:         link.NewBuildID(time.Now()),
 		Repository: repository,
@@ -282,10 +283,33 @@ 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: checkBuildsOf(p, scope, entries, clone)},
+			Manifests: scope.Manifests, Judge: scope.Judge, Builds: builds, Named: namedForTheCheck(ctx, builds)},
 	}, nil
 }
 
+// namedForTheCheck are the builds the seat's verdicts name for the modules a check builds, read once for the check
+// (novox/hq issue 703): what it may keep instead of building again. None when they cannot be read, and the check
+// builds, as before.
+func namedForTheCheck(ctx context.Context, builds []link.CheckBuild) []link.CheckedBuild {
+	var modules []string
+	for _, b := range builds {
+		if b.Skip == "" {
+			modules = append(modules, b.Module)
+		}
+	}
+	if len(modules) == 0 {
+		return nil
+	}
+	named, err := checkedBuildsOfModules(ctx, modules)
+	if err != nil {
+		fmt.Printf("  the checks' builds of %d module(s) could not be read, so the check builds them: %v\n",
+			len(modules), err)
+		// empty-on-error: said above; a check handed no named build builds every module, as before issue 703
+		return nil
+	}
+	return named
+}
+
 // checkBuildsOf are the modules a merge of the change would build, in the walk's order, as the check builds them
 // for the walk to take (novox/hq ADR 0350 §1): a module built from the repository checked, at the branch it merges
 // into, from the merge the check makes; any other — a module standing on one — from its own repository at the

cmd/mesh-controller/release_plan.go

@@ -386,6 +386,21 @@ var checkedBuildsOf = func(ctx context.Context, pulls []link.PullRef) ([]link.Ch
 	return link.ReadCheckBuilds(ctx, js, buildSeatHeld(ctx), pulls, time.Now().Add(-builder.CheckBuildsKept))
 }
 
+// checkedBuildsOfModules are the builds the build seat's checks of any pull request said they made, or kept, of
+// those modules within the week check builds are kept (novox/hq issue 703): what a check may keep instead of
+// building. A variable so a test needs no bus.
+var checkedBuildsOfModules = func(ctx context.Context, modules []string) ([]link.CheckedBuild, error) {
+	if len(modules) == 0 {
+		return nil, nil
+	}
+	js, err := aBus()
+	if err != nil {
+		return nil, err
+	}
+	defer js.Close()
+	return link.ReadCheckBuildsOf(ctx, js, buildSeatHeld(ctx), modules, time.Now().Add(-builder.CheckBuildsKept))
+}
+
 // reuseWithVerdicts is a walk's leave with the builds its pull requests' verdicts name: the only builds the builder
 // may take (ADR 0350 §2). When they cannot be read the leave names none, and the walk builds, saying why.
 func reuseWithVerdicts(ctx context.Context, r *link.ReuseAsk) *link.ReuseAsk {