buildstep: keep a check ask's cache layer out of every step's reach, move through an os.Root, and layer the build cache alone

Review of 62a50eb: the layer lay in the workspace, which every step binds read-write,
so a change's command could swap a kept directory for a symlink and the move, run
outside the sandbox, would write into the shared cache (ADR 0541 §2). The layer now
lies beside the workspace (Workspaces.CacheLayer, removed by Release and Sweep),
Command refuses a layer within reach of the workspace, a writable directory or a
cache, and every move goes through an os.Root on the layer and keeps no symlink. Only
LayerCaches (the build cache) are layered; a module cache keeps its throwaway layer
(issue 1921).

go/buildstep/askrun.go

@@ -170,8 +170,8 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error)
 	}
 	defer s.release(f.Workspace)
 	answer := delivery.CheckAnswer{Fetch: r}
-	if len(f.Conflicts) > 0 {
-		return answer, nil // the asker reads the conflict from the fetch's answer: a verdict, not an error
+	if len(f.Conflicts) > 0 || job.MergeOnly {
+		return answer, nil // the asker reads the merge or the conflict from the fetch's answer: a verdict, not an error
 	}
 	if s.Test == nil {
 		return nil, Refuse("this build module tests nothing")

go/buildstep/askrun_test.go

@@ -235,7 +235,7 @@ test {"level":"unit","runner":"workspace","workspace":"/w"}`},
 }
 
 // A Node check tests each module directory, the one whose tests changed first; a conflict ends the ask with the fetch
-// alone, a verdict for the asker.
+// alone, a verdict for the asker; so does a merge-only ask of a head that merges.
 func TestRunAskChecksNodeModulesAndConflicts(t *testing.T) {
 	r := &recorder{fetch: map[string]any{"workspace": "/w", "modules": []string{"a", "b"}, "changed": []string{"b/test/x.test.ts"}},
 		tests: map[string]bool{"a": true, "b": true}}
@@ -252,6 +252,12 @@ func TestRunAskChecksNodeModulesAndConflicts(t *testing.T) {
 	if err != nil || len(r.calls) != 1 || len(out.(delivery.CheckAnswer).Levels) != 0 {
 		t.Fatalf("a conflict: %v %v", err, r.calls)
 	}
+	// A merge-only ask answers the fetch of a head that merges and runs no level.
+	r = &recorder{fetch: map[string]any{"workspace": "/w", "merge": "m1"}, tests: map[string]bool{"a": true}}
+	out, err = RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit}, MergeOnly: true, Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}}))
+	if err != nil || len(r.calls) != 1 || len(out.(delivery.CheckAnswer).Levels) != 0 || !strings.Contains(string(out.(delivery.CheckAnswer).Fetch), `"m1"`) {
+		t.Fatalf("merge only: %v %v %v", err, r.calls, out)
+	}
 }
 
 // Issue 1921: a failure in the packages whose tests changed does not stop the level: the other packages still run, so two

go/buildstep/isolate.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -33,20 +34,26 @@ const Bubblewrap = "bwrap"
 //     downloaded) read-only;
 //   - the shared warm Caches read-only under a throwaway writable layer (S18), or read-write for a trunk build alone
 //     (WriteCaches), so a candidate's code never changes what a later build reads;
-//   - with a CacheLayer, what one command writes into the caches kept for the later commands of the same ask (issue 1921,
-//     the try speed-up): the vet, the unit tests and the integration compile of one ask compile each package once.
+//   - with a CacheLayer, what one command writes into the LayerCaches kept for the later commands of the same ask (issue
+//     1921, the try speed-up): the vet, the unit tests and the integration compile of one ask compile each package once.
 type Isolation struct {
 	Workspace   string
 	Writable    []string
 	ReadOnly    []string
 	Caches      []string
 	WriteCaches bool
-	// CacheLayer is a directory of the ask's own, in its workspace and gone with it: each command writes the caches into
-	// a layer of its own beneath it (overlayfs refuses an upper layer two mounts share, and an ask's commands run side by
-	// side), which Isolated moves into the ask's kept layer when the command ends; every later command reads the kept
-	// layer above the shared cache. The shared cache stays unchanged. Empty: what a command writes is thrown away.
+	// CacheLayer is a directory of the ask's own, gone with its workspace (Workspaces.CacheLayer): each command writes
+	// the LayerCaches into a layer of its own beneath it (overlayfs refuses an upper layer two mounts share, and an
+	// ask's commands run side by side), which Isolated moves into the ask's kept layer when the command ends; every
+	// later command reads the kept layer above the shared cache. The shared cache stays unchanged. It must lie outside
+	// the Workspace and every Writable directory, so no command reaches it (a command that could would make the move,
+	// which runs outside the sandbox, write where it points). Empty: what a command writes is thrown away.
 	CacheLayer string
-	Env        []string
+	// LayerCaches are the Caches kept in the CacheLayer: a build cache, whose entries are whole files written by
+	// rename. Every other cache (a module cache, with read-only directories and half-written modules) keeps a
+	// throwaway layer.
+	LayerCaches []string
+	Env         []string
 	// Hosts is the file bound as /etc/hosts; Isolated writes one naming only the loopback.
 	Hosts string
 	// run is the directory of this command's own writable layers under CacheLayer; Isolated makes one per command.
@@ -110,6 +117,9 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin
 			}
 		}
 	}
+	if err := iso.checkLayer(); err != nil {
+		return nil, err
+	}
 	words := append(append([]string{}, iso.Env...), env...)
 	home := false
 	for _, w := range words {
@@ -146,16 +156,16 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin
 
 // cacheArgs binds the shared warm caches (S18): for a trunk build read-write, so the next build is warm; for any other
 // read-only beneath an overlay whose writes go to a tmpfs of the step's own, so the step writes as a build does and
-// nothing it wrote outlives it (bubblewrap's --tmp-overlay, an overlay mount in the step's user namespace). With a
-// CacheLayer the ask's kept layer lies above the shared cache and the command's writes go to its own layer on disk
-// (--overlay), which Isolated keeps for the ask when the command ends.
+// nothing it wrote outlives it (bubblewrap's --tmp-overlay, an overlay mount in the step's user namespace). A cache of
+// LayerCaches, with a CacheLayer, has the ask's kept layer above the shared cache and the command's writes go to its own
+// layer on disk (--overlay), which Isolated keeps for the ask when the command ends.
 func (iso Isolation) cacheArgs() []string {
 	var argv []string
 	for i, c := range iso.Caches {
 		switch {
 		case iso.WriteCaches:
 			argv = append(argv, "--bind", c, c)
-		case iso.CacheLayer != "":
+		case iso.layered(c):
 			upper, work := iso.runLayer(i)
 			argv = append(argv, "--overlay-src", c, "--overlay-src", keptLayer(iso.CacheLayer, i), "--overlay", upper, work, c)
 		default:
@@ -165,6 +175,33 @@ func (iso Isolation) cacheArgs() []string {
 	return argv
 }
 
+// layered says whether the cache c is kept in the ask's cache layer.
+func (iso Isolation) layered(c string) bool {
+	return iso.CacheLayer != "" && !iso.WriteCaches && slices.Contains(iso.LayerCaches, c)
+}
+
+// checkLayer refuses a cache layer a command could reach (in or above the workspace or a writable directory, or a
+// cache), and a layered cache that is not one of the caches.
+func (iso Isolation) checkLayer() error {
+	if iso.CacheLayer == "" {
+		return nil
+	}
+	for _, d := range append(append([]string{iso.Workspace}, iso.Writable...), iso.Caches...) {
+		if within(iso.CacheLayer, d) || within(d, iso.CacheLayer) {
+			return fmt.Errorf("buildstep: the cache layer %s is within reach of the step's %s", iso.CacheLayer, d)
+		}
+	}
+	for _, c := range iso.LayerCaches {
+		if !slices.Contains(iso.Caches, c) {
+			return fmt.Errorf("buildstep: the layered cache %s is not one of the caches", c)
+		}
+	}
+	return nil
+}
+
+// within says whether p is dir or lies beneath it.
+func within(p, dir string) bool { return p == dir || strings.HasPrefix(p, dir+"/") }
+
 func layerDirs(layer string) []string {
 	if layer == "" {
 		return nil
@@ -172,8 +209,9 @@ func layerDirs(layer string) []string {
 	return []string{layer}
 }
 
-// keptLayer is where the ask keeps what its commands wrote into the cache i.
-func keptLayer(layer string, i int) string { return filepath.Join(layer, "kept", strconv.Itoa(i)) }
+// keptRel is where, in the cache layer, the ask keeps what its commands wrote into the cache i; keptLayer is its path.
+func keptRel(i int) string                 { return filepath.Join("kept", strconv.Itoa(i)) }
+func keptLayer(layer string, i int) string { return filepath.Join(layer, keptRel(i)) }
 
 // runLayer is the upper and work directory of this command's writes into the cache i. Command, which makes no
 // directory, names the layer "run"; Isolated gives each command one of its own.
@@ -190,9 +228,12 @@ var layerLocks sync.Map
 
 // prepareLayer makes the kept layers and a run directory of the command's own under the cache layer.
 func (iso *Isolation) prepareLayer() error {
-	if iso.CacheLayer == "" || iso.WriteCaches || len(iso.Caches) == 0 {
+	if iso.CacheLayer == "" || iso.WriteCaches || len(iso.LayerCaches) == 0 {
 		return nil
 	}
+	if err := iso.checkLayer(); err != nil {
+		return err
+	}
 	if err := os.MkdirAll(filepath.Join(iso.CacheLayer, "runs"), 0o700); err != nil {
 		return err
 	}
@@ -201,7 +242,10 @@ func (iso *Isolation) prepareLayer() error {
 		return err
 	}
 	iso.run = run
-	for i := range iso.Caches {
+	for i, c := range iso.Caches {
+		if !iso.layered(c) {
+			continue
+		}
 		upper, work := iso.runLayer(i)
 		for _, d := range []string{keptLayer(iso.CacheLayer, i), upper, work} {
 			if err := os.MkdirAll(d, 0o700); err != nil {
@@ -212,10 +256,11 @@ func (iso *Isolation) prepareLayer() error {
 	return nil
 }
 
-// keepLayer moves what the command wrote into the caches into the ask's kept layers, and removes its run directory.
-// A file the command wrote replaces the kept one (a cache's newest write wins); a whiteout (a file the command
-// deleted) is not kept, since a cache entry deleted by one command is no fault of a later one. A write that cannot be
-// kept only means a later command builds it again.
+// keepLayer moves what the command wrote into the layered caches into the ask's kept layers, and removes its run
+// directory. A file the command wrote replaces the kept one (a cache's newest write wins); a whiteout (a file the
+// command deleted), a symlink or any other entry that is not a directory or a regular file is not kept. Every move goes
+// through an os.Root on the cache layer, so no symlink leads it outside the layer. A write that cannot be kept only
+// means a later command builds it again.
 func (iso Isolation) keepLayer() {
 	if iso.run == "" {
 		return
@@ -224,23 +269,37 @@ func (iso Isolation) keepLayer() {
 	mu, _ := layerLocks.LoadOrStore(iso.CacheLayer, &sync.Mutex{})
 	mu.(*sync.Mutex).Lock()
 	defer mu.(*sync.Mutex).Unlock()
-	for i := range iso.Caches {
-		upper, _ := iso.runLayer(i)
-		kept := keptLayer(iso.CacheLayer, i)
-		filepath.WalkDir(upper, func(p string, d fs.DirEntry, err error) error {
+	root, err := os.OpenRoot(iso.CacheLayer)
+	if err != nil {
+		return
+	}
+	defer root.Close()
+	runRel, err := filepath.Rel(iso.CacheLayer, iso.run)
+	if err != nil {
+		return
+	}
+	for i, c := range iso.Caches {
+		if !iso.layered(c) {
+			continue
+		}
+		upper := filepath.Join(runRel, strconv.Itoa(i), "upper")
+		fs.WalkDir(root.FS(), upper, func(p string, d fs.DirEntry, err error) error {
 			if err != nil || p == upper {
 				return nil
 			}
 			rel, _ := filepath.Rel(upper, p)
-			to := filepath.Join(kept, rel)
+			to := filepath.Join(keptRel(i), rel)
 			switch {
 			case d.IsDir():
-				if fi, err := os.Lstat(to); err == nil && !fi.IsDir() {
-					os.Remove(to)
+				if fi, err := root.Lstat(to); err == nil && !fi.IsDir() {
+					root.Remove(to)
 				}
-				os.MkdirAll(to, 0o700)
+				root.MkdirAll(to, 0o700)
 			case d.Type().IsRegular():
-				os.Rename(p, to)
+				if fi, err := root.Lstat(to); err == nil && !fi.Mode().IsRegular() {
+					return nil // never replace what is not a file the layer kept
+				}
+				root.Rename(p, to)
 			}
 			return nil
 		})

go/buildstep/isolate_caches_test.go

@@ -32,24 +32,35 @@ func TestACandidatesStepWritesTheCachesOnlyIntoAThrowawayLayer(t *testing.T) {
 	}
 	// 1921/S2: with a cache layer, a command reads the ask's kept layer above the shared cache and writes into a layer
 	// on disk of the ask's own, never into the shared cache and never into a throwaway tmpfs.
-	iso = Isolation{Workspace: "/w", Caches: []string{"/cache/gomod", "/cache/gobuild"}, CacheLayer: "/w/cachelayer"}
+	// Only the layered caches (the build cache) are; the module cache keeps its throwaway layer.
+	iso = Isolation{Workspace: "/w/ws1", Caches: []string{"/cache/gomod", "/cache/gobuild"}, CacheLayer: "/w/ws1.cachelayer",
+		LayerCaches: []string{"/cache/gobuild"}}
 	argv, err = iso.Command("go", "vet")
 	if err != nil {
 		t.Fatal(err)
 	}
-	for i, c := range iso.Caches {
-		if !holds(argv, "--overlay-src", c, "--overlay-src", keptLayer(iso.CacheLayer, i), "--overlay") {
-			t.Errorf("the cache %s is not read beneath the ask's kept layer: %v", c, argv)
-		}
-		if up := overlayOf(argv, c); up == "" || !strings.HasPrefix(up, "/w/cachelayer/") {
-			t.Errorf("the cache %s is written into %q, not a layer of the ask's own: %v", c, up, argv)
-		}
-		if holds(argv, "--tmp-overlay", c) || holds(argv, "--bind", c, c) {
-			t.Errorf("the cache %s is thrown away or written: %v", c, argv)
-		}
+	if !holds(argv, "--overlay-src", "/cache/gobuild", "--overlay-src", keptLayer(iso.CacheLayer, 1), "--overlay") {
+		t.Errorf("the build cache is not read beneath the ask's kept layer: %v", argv)
 	}
-	if _, err := (Isolation{Workspace: "/w", CacheLayer: "cachelayer"}).Command("go"); err == nil {
-		t.Error("a relative cache layer was taken")
+	if up := overlayOf(argv, "/cache/gobuild"); up == "" || !strings.HasPrefix(up, "/w/ws1.cachelayer/") {
+		t.Errorf("the build cache is written into %q, not a layer of the ask's own: %v", up, argv)
+	}
+	if !holds(argv, "--overlay-src", "/cache/gomod", "--tmp-overlay", "/cache/gomod") || overlayOf(argv, "/cache/gomod") != "" {
+		t.Errorf("the module cache is not under a throwaway layer: %v", argv)
+	}
+	// A layer a command could reach is refused: in the workspace (bound read-write), in a writable directory, in a
+	// cache, or above one of them; so is a relative one, and a layered cache that is no cache.
+	for _, bad := range []Isolation{
+		{Workspace: "/w/ws1", CacheLayer: "/w/ws1/cachelayer"},
+		{Workspace: "/w/ws1", CacheLayer: "/w"},
+		{Workspace: "/w/ws1", Writable: []string{"/out"}, CacheLayer: "/out/layer"},
+		{Workspace: "/w/ws1", Caches: []string{"/cache/gobuild"}, CacheLayer: "/cache/gobuild/layer"},
+		{Workspace: "/w/ws1", CacheLayer: "cachelayer"},
+		{Workspace: "/w/ws1", Caches: []string{"/cache/gobuild"}, CacheLayer: "/w/ws1.cachelayer", LayerCaches: []string{"/cache/other"}},
+	} {
+		if _, err := bad.Command("go"); err == nil {
+			t.Errorf("the cache layer %s of %+v was taken", bad.CacheLayer, bad)
+		}
 	}
 }
 

go/buildstep/isolate_integration_test.go

@@ -66,8 +66,9 @@ func TestARunStepReachesNothingAndLeavesTheCachesUnchanged(t *testing.T) {
 
 	// 1921/S2: with the ask's cache layer, what one command writes into the cache the next command of the ask reads,
 	// above what the shared cache holds, and the shared cache stays unchanged.
+	layer := filepath.Join(filepath.Dir(workspace), "ws1.cachelayer")
 	asked := Isolated(forgekey.ExecRunnerFor(0), Isolation{Workspace: workspace, Caches: []string{cacheDir},
-		CacheLayer: filepath.Join(workspace, "cachelayer")})
+		CacheLayer: layer, LayerCaches: []string{cacheDir}})
 	r = asked(context.Background(), []string{"PATH=/usr/bin:/bin"}, "/bin/sh", "-c", "cat "+filepath.Join(cacheDir, "warm")+" && mkdir -p "+filepath.Join(cacheDir, "ab")+" && echo compiled > "+filepath.Join(cacheDir, "ab", "one-d"))
 	if r.Status != 0 || r.Err != "" || strings.TrimSpace(r.Stdout) != "warm" {
 		t.Fatalf("the first command of the ask did not read the shared cache and write its own (status %d, %s): %s%s", r.Status, r.Err, r.Stdout, r.Stderr)
@@ -76,10 +77,23 @@ func TestARunStepReachesNothingAndLeavesTheCachesUnchanged(t *testing.T) {
 	if r.Status != 0 || r.Err != "" || r.Stdout != "compiled\nwarm\n" {
 		t.Fatalf("the next command of the ask did not find what the first wrote (status %d, %s): %q %s", r.Status, r.Err, r.Stdout, r.Stderr)
 	}
-	if _, err := os.Stat(filepath.Join(cacheDir, "ab")); !os.IsNotExist(err) {
-		t.Fatalf("the ask's commands wrote the shared cache: %v", err)
+	// A command cannot reach the layer (it lies outside every directory the step binds), and a symlink it writes into
+	// the cache is not kept, so nothing it does makes the keeping write outside the layer.
+	r = asked(context.Background(), []string{"PATH=/usr/bin:/bin"}, "/bin/sh", "-c",
+		"ls "+layer+" >/dev/null 2>&1 && echo reached; ln -s "+cacheDir+" "+filepath.Join(cacheDir, "esc")+" && mkdir "+filepath.Join(cacheDir, "cd")+" && echo x > "+filepath.Join(cacheDir, "cd", "f"))
+	if r.Status != 0 || r.Err != "" || strings.Contains(r.Stdout, "reached") {
+		t.Fatalf("the command reached the cache layer or could not write the cache (status %d, %s): %q %s", r.Status, r.Err, r.Stdout, r.Stderr)
 	}
-	if err := os.RemoveAll(filepath.Join(workspace, "cachelayer")); err != nil {
-		t.Fatalf("the ask's cache layer cannot be removed with its workspace: %v", err)
+	if fi, err := os.Lstat(filepath.Join(keptLayer(layer, 0), "esc")); err == nil {
+		t.Fatalf("a symlink the command wrote was kept: %v", fi.Mode())
+	}
+	if b, err := os.ReadFile(filepath.Join(keptLayer(layer, 0), "cd", "f")); err != nil || string(b) != "x\n" {
+		t.Fatalf("the command's file was not kept: %q %v", b, err)
+	}
+	if entries, _ := os.ReadDir(cacheDir); len(entries) != 1 || entries[0].Name() != "warm" {
+		t.Fatalf("the ask's commands wrote the shared cache: %v", entries)
+	}
+	if err := os.RemoveAll(layer); err != nil {
+		t.Fatalf("the ask's cache layer cannot be removed: %v", err)
 	}
 }

go/buildstep/isolate_test.go

@@ -124,7 +124,7 @@ func TestIsolatedRunsEachCommandThroughBubblewrap(t *testing.T) {
 	if err := os.MkdirAll(shared, 0o700); err != nil {
 		t.Fatal(err)
 	}
-	layer := filepath.Join(root, "ws", "cachelayer")
+	layer := filepath.Join(root, "ws1.cachelayer")
 	var mu sync.Mutex
 	uppers := map[string]bool{}
 	gate := make(chan struct{})
@@ -141,7 +141,15 @@ func TestIsolatedRunsEachCommandThroughBubblewrap(t *testing.T) {
 		os.WriteFile(filepath.Join(up, "ab", args[len(args)-1]+"-d"), []byte("compiled"), 0o600)
 		return forgekey.Ran{}
 	}
-	run := Isolated(writer, Isolation{Workspace: filepath.Join(root, "ws"), Caches: []string{shared}, CacheLayer: layer})
+	// A symlink in the kept layer (none is ever kept; planted here) leads no move outside the layer: the kept directory
+	// replaces it, and the shared cache it points to stays unchanged.
+	if err := os.MkdirAll(keptLayer(layer, 0), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(shared, filepath.Join(keptLayer(layer, 0), "ab")); err != nil {
+		t.Fatal(err)
+	}
+	run := Isolated(writer, Isolation{Workspace: filepath.Join(root, "ws1"), Caches: []string{shared}, CacheLayer: layer, LayerCaches: []string{shared}})
 	var wg sync.WaitGroup
 	for _, pkg := range []string{"one", "two"} {
 		wg.Add(1)

go/buildstep/workspaces.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"sort"
+	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -116,6 +117,14 @@ func (w *Workspaces) touch(id string) {
 	_ = os.Chtimes(filepath.Join(w.Dir, id), now, now) // a workspace not made yet has nothing to mark
 }
 
+// cacheLayerSuffix names a workspace's cache layer beside it (Isolation.CacheLayer): never inside it, which every step
+// of the workspace binds read-write.
+const cacheLayerSuffix = ".cachelayer"
+
+// CacheLayer is the directory of the workspace id's cache layer: beside the workspace, out of every step's reach, and
+// removed with it by Release and Sweep.
+func (w *Workspaces) CacheLayer(id string) string { return filepath.Join(w.Dir, id+cacheLayerSuffix) }
+
 // Release removes the workspace at once: its caller's chain of steps has ended. One already gone is no error; one a
 // running step holds is refused and kept; an id that is not a workspace's is refused.
 func (w *Workspaces) Release(id string) error {
@@ -130,6 +139,9 @@ func (w *Workspaces) Release(id string) error {
 	if err := os.RemoveAll(filepath.Join(w.Dir, id)); err != nil {
 		return fmt.Errorf("the workspace %s could not be removed: %w", id, err)
 	}
+	if err := os.RemoveAll(w.CacheLayer(id)); err != nil {
+		return fmt.Errorf("the cache layer of the workspace %s could not be removed: %w", id, err)
+	}
 	return nil
 }
 
@@ -144,6 +156,13 @@ func (w *Workspaces) Sweep(now time.Time, bound time.Duration) []string {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 	for _, e := range entries {
+		if id, ok := strings.CutSuffix(e.Name(), cacheLayerSuffix); ok && workspaceID.MatchString(id) && w.held[id] == 0 {
+			// a cache layer whose workspace is gone (its removal failed or was cut short) goes too
+			if _, err := os.Stat(filepath.Join(w.Dir, id)); os.IsNotExist(err) {
+				os.RemoveAll(filepath.Join(w.Dir, e.Name()))
+			}
+			continue
+		}
 		if !e.IsDir() || !workspaceID.MatchString(e.Name()) || w.held[e.Name()] > 0 {
 			continue
 		}
@@ -152,6 +171,7 @@ func (w *Workspaces) Sweep(now time.Time, bound time.Duration) []string {
 			continue
 		}
 		if os.RemoveAll(filepath.Join(w.Dir, e.Name())) == nil {
+			os.RemoveAll(w.CacheLayer(e.Name()))
 			removed = append(removed, e.Name())
 		}
 	}

go/buildstep/workspaces_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
@@ -68,8 +69,15 @@ func gone(p string) bool { _, err := os.Stat(p); return os.IsNotExist(err) }
 func TestReleaseRemovesAWorkspaceAtOnce(t *testing.T) {
 	w := &Workspaces{Dir: t.TempDir()}
 	p := makeWorkspace(t, w, "abc1", 0)
-	if err := w.Release("abc1"); err != nil || !gone(p) {
-		t.Fatalf("Release = %v, gone %v", err, gone(p))
+	layer := w.CacheLayer("abc1") // issue 1921/S2: the cache layer lies beside the workspace and goes with it
+	if filepath.Dir(layer) != w.Dir || strings.HasPrefix(layer, p+"/") {
+		t.Fatalf("the cache layer %s does not lie beside the workspace %s", layer, p)
+	}
+	if err := os.MkdirAll(filepath.Join(layer, "kept", "0"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Release("abc1"); err != nil || !gone(p) || !gone(layer) {
+		t.Fatalf("Release = %v, gone %v, its cache layer gone %v", err, gone(p), gone(layer))
 	}
 	if err := w.Release("abc1"); err != nil {
 		t.Errorf("a second release = %v, want nil", err)
@@ -98,11 +106,21 @@ func TestSweepRemovesUnheldWorkspacesPastTheBound(t *testing.T) {
 	if err := os.WriteFile(filepath.Join(w.Dir, "not-a-workspace.txt"), nil, 0o600); err != nil {
 		t.Fatal(err)
 	}
+	// issue 1921/S2: a swept workspace's cache layer goes with it, and so does a layer whose workspace is gone; a held
+	// workspace's stays
+	for _, id := range []string{"old1", "held1", "orphan1"} {
+		if err := os.MkdirAll(w.CacheLayer(id), 0o700); err != nil {
+			t.Fatal(err)
+		}
+	}
 	done := w.Hold("held1")
 	removed := w.Sweep(time.Now(), 2*time.Hour)
 	if !gone(old) || gone(heldOld) || gone(recent) {
 		t.Errorf("after the sweep: old gone %v, held kept %v, recent kept %v", gone(old), !gone(heldOld), !gone(recent))
 	}
+	if !gone(w.CacheLayer("old1")) || !gone(w.CacheLayer("orphan1")) || gone(w.CacheLayer("held1")) {
+		t.Errorf("after the sweep: old1's layer gone %v, orphan1's gone %v, held1's kept %v", gone(w.CacheLayer("old1")), gone(w.CacheLayer("orphan1")), !gone(w.CacheLayer("held1")))
+	}
 	if len(removed) != 1 || removed[0] != "old1" {
 		t.Errorf("Sweep answered %v, want [old1]", removed)
 	}

go/compose/composed.go

@@ -318,15 +318,6 @@ func Stub(m catalogue.Manifest) catalogue.Manifest {
 		m.OwnsPaths = catalogue.DeclaredPaths(m)
 	}
 	m.Build = nil
-	// Nor is a template its build kept for its own files handed: a stub carries none of them (issue 1126: novox's node
-	// refused "postgres: its build kept a template for consumers, which it does not declare" with postgres held). Its
-	// items stay, and the templates they render through: a held module's contributions to other modules' collections
-	// (seat contributions, shell blocks, seat data) still reach their collectors.
-	if m.Handed != nil {
-		h := *m.Handed
-		h.Templates = nil
-		m.Handed = &h
-	}
 	var dirs []map[string]any
 	for _, r := range m.Resources {
 		if r["type"] == "directory" {

go/compose/frombus/answer.go

@@ -209,59 +209,21 @@ func Answer(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]st
 	return a
 }
 
-// heldBehind is why a module is held with a held one it reads (composeEach's dependents).
-func heldBehind(m string) string {
-	return "it reads what " + m + " contributes or listens on, and " + m + " is held"
-}
-
-// behind is the held module a refusal's words say it is held behind, or "".
-func behind(why string) string {
-	rest, ok := strings.CutPrefix(why, "it reads what ")
-	if !ok {
-		return ""
-	}
-	m, _, ok := strings.Cut(rest, " contributes or listens on, and ")
-	if !ok || why != heldBehind(m) {
-		return ""
-	}
-	return m
-}
-
-// heldAsToday moves into Held each refusal of a module the node keeps today as it runs (Scope.HeldToday, with how:
-// held, failed, running a build of the old form), and of each module held only behind one of those: the candidate keeps
-// them as the node does, which is no change (issue 1126: a seed reproduces today, and today keeps them so). Each says
-// how it stands today (ModuleRefused.Today).
-func heldAsToday(a *delivery.CandidateAnswer, today map[string]string) {
+// heldAsToday moves each refusal of a module the node holds today (Scope.HeldToday) into Held: the candidate holds it as
+// the node does, which is no change (issue 1126: a seed reproduces today, and today holds it too).
+func heldAsToday(a *delivery.CandidateAnswer, today []string) {
 	if len(today) == 0 {
 		return
 	}
-	how := map[string]string{}
-	for m, w := range today {
-		how[m] = w
-	}
-	for moved := true; moved; {
-		moved = false
-		kept := a.Refusals[:0]
-		for _, r := range a.Refusals {
-			w, keep := how[r.Module]
-			if !keep {
-				if m := behind(r.Why); m != "" {
-					if _, behindKept := how[m]; behindKept {
-						w, keep = "held behind "+m, true
-					}
-				}
-			}
-			if keep {
-				r.Today = w
-				how[r.Module] = w
-				a.Held = append(a.Held, r)
-				moved = true
-			} else {
-				kept = append(kept, r)
-			}
+	kept := a.Refusals[:0]
+	for _, r := range a.Refusals {
+		if slices.Contains(today, r.Module) {
+			a.Held = append(a.Held, r)
+		} else {
+			kept = append(kept, r)
 		}
-		a.Refusals = kept
 	}
+	a.Refusals = kept
 	if len(a.Refusals) == 0 {
 		a.Refusals = nil
 	}
@@ -351,7 +313,7 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b
 		// What the others read of its manifest (what it contributes, what it listens on) is its new manifest's, not
 		// necessarily the build's that runs: what reads it is held with it (review of the apply, round 2).
 		for _, r := range dependents(in, node, src[m], failed) {
-			hold(r, heldBehind(m), false)
+			hold(r, "it reads what "+m+" contributes or listens on, and "+m+" is held", false)
 		}
 	}
 	for _, m := range sortedKeys(failed) {

go/compose/frombus/answer_rows_test.go

@@ -93,25 +93,17 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
 	}
 	// Issue 1126: a module the node holds today that the candidate holds too is held as today, no refusal: with every
 	// refused module held today the verdict is no refusal, and each is named under Held.
-	today := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: map[string]string{"api": "held", "spiegel": "held", "web": "held"}},
+	today := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: []string{"api", "spiegel", "web"}},
 		map[string]string{"lemurs": fp("e"), "web": fp("1"), "api": fp("2"), "spiegel": fp("3")}, nil, "p-h1", 3)
 	if today.Verdict == delivery.CandidateRefused || len(today.Refusals) != 0 || len(today.Held) != 3 || today.Held[0].Why == "" {
 		t.Errorf("held as today: %+v", today)
 	}
 	// A refused module the node does not hold today stays a refusal: holding only api today leaves spiegel and web refused.
-	part := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: map[string]string{"api": "held"}},
+	part := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: []string{"api"}},
 		map[string]string{"lemurs": fp("e"), "web": fp("1"), "api": fp("2"), "spiegel": fp("3")}, nil, "p-h1", 3)
 	if part.Verdict != delivery.CandidateRefused || len(part.Refusals) != 2 || len(part.Held) != 1 || part.Held[0].Module != "api" {
 		t.Errorf("held as today in part: %+v", part)
 	}
-	// A module held only behind one kept today is kept as today too, saying behind which (novox's mailu behind de-spiegel).
-	if a := (delivery.CandidateAnswer{Refusals: []delivery.ModuleRefused{{Module: "de-spiegel", Why: "a value is unset"},
-		{Module: "mailu", Why: heldBehind("de-spiegel")}, {Module: "proxy", Why: heldBehind("mailu")}, {Module: "web", Why: "no build"}}}); true {
-		heldAsToday(&a, map[string]string{"de-spiegel": "runs a build of the old form"})
-		if len(a.Refusals) != 1 || a.Refusals[0].Module != "web" || len(a.Held) != 3 || a.Held[2].Today != "held behind mailu" {
-			t.Errorf("held behind a module kept today: %+v", a)
-		}
-	}
 
 	// The fold: spiegel runs inside the node's own composition at a build of the old form. Its resources are carried as
 	// they run, each after the one it runs after: the node's own is the one it runs, and the node composes itself.
@@ -294,31 +286,6 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
 	if got, want := catalogue.DeclaredPaths(compose.Carried(f1, "f1-circuits", &f1runs)), catalogue.DeclaredPaths(f1); !reflect.DeepEqual(got, want) {
 		t.Errorf("f1-circuits carried owns %v, want what the controller composes from its manifest, %v", got, want)
 	}
-	// Issue 1126: a held module hands no template its build kept for a file it no longer carries (novox's node:
-	// "postgres: its build kept a template for consumers"), but keeps its items and their templates, so what it
-	// contributes to other modules' collections still reaches them.
-	for _, row := range []struct {
-		name   string
-		handed *catalogue.Handed
-		items  []catalogue.Item
-	}{
-		{"templates and items", &catalogue.Handed{Templates: map[string]string{"consumers": "files/consumers.json"},
-			Items: map[string]string{"routes": "files/routes.tmpl"}}, []catalogue.Item{{Collection: "routes", Content: "x"}}},
-		{"items alone", nil, []catalogue.Item{{Collection: "routes", Content: "x"}}},
-	} {
-		kept := f1
-		kept.Handed, kept.Items = row.handed, row.items
-		st := compose.Stub(kept)
-		if st.Handed != nil && st.Handed.Templates != nil {
-			t.Errorf("%s: a stub hands templates %v", row.name, st.Handed.Templates)
-		}
-		if !reflect.DeepEqual(st.Items, row.items) {
-			t.Errorf("%s: a stub collects %+v, want its items %+v", row.name, st.Items, row.items)
-		}
-		if row.handed != nil && (st.Handed == nil || !reflect.DeepEqual(st.Handed.Items, row.handed.Items) || row.handed.Templates == nil) {
-			t.Errorf("%s: a stub hands item templates %+v, want %v and its manifest untouched", row.name, st.Handed, row.handed.Items)
-		}
-	}
 	var k1, k2 any
 	_ = json.Unmarshal(keptComposed, &k1)
 	_ = json.Unmarshal(keptFile, &k2)

go/compose/frombus/frombus.go

@@ -134,10 +134,10 @@ type Scope struct {
 	Placements map[string]*delivery.Placement
 	InOwn      []string
 	Running    []delivery.Composed
-	// HeldToday is, in an answer, the modules the node keeps today as they run ("node" for its own composition), each with
-	// how ("held", "failed", "runs a build of the old form"): a candidate that cannot compose one, or holds one only
-	// behind one of them, keeps it as today, which is said under CandidateAnswer.Held and is no refusal (issue 1126).
-	HeldToday map[string]string
+	// HeldToday is, in an answer, the modules the node holds today ("node" for its own composition): their have's phase
+	// is held, or they are held inside its own composition. A candidate that holds one too holds it as today, which is
+	// said under CandidateAnswer.Held and is no refusal (issue 1126).
+	HeldToday []string
 	// SameValue, in an answer, says whether two sealed values open to the same value (the node opens them with its own
 	// key), so a differing resource shows a value sealed again as such (delivery.CompareComposedWith). Nil shows them
 	// as differing.

go/compose/judge/judge.go

@@ -258,13 +258,9 @@ func Brief(p delivery.PlacementProposal, v delivery.PlacementVerdict) string {
 		if held := v.Answers[n].Held; len(held) > 0 {
 			var names []string
 			for _, h := range held {
-				today := h.Today
-				if today == "" {
-					today = "held"
-				}
-				names = append(names, h.Module+" ("+today+")")
+				names = append(names, h.Module)
 			}
-			fmt.Fprintf(&b, "; stays as today: %s", strings.Join(names, ", "))
+			fmt.Fprintf(&b, "; stays held as today: %s", strings.Join(names, ", "))
 		}
 	}
 	return b.String()

go/compose/judge/judge_test.go

@@ -79,7 +79,7 @@ func TestJudge(t *testing.T) {
 				t.Errorf("the difference is named %q", got)
 			}
 		}
-		if strings.HasPrefix(row.name, "a module held as today") && !strings.Contains(Brief(q, v), "stays as today: spiegel (held)") {
+		if strings.HasPrefix(row.name, "a module held as today") && !strings.Contains(Brief(q, v), "stays held as today: spiegel") {
 			t.Errorf("%s: the brief does not show it: %s", row.name, Brief(q, v))
 		}
 		if row.marked != "" && (!slices.Contains(v.Unintended["anchor"], row.marked) || !strings.Contains(Brief(q, v), "not computed: "+row.marked)) {

go/delivery/placement_candidate.go

@@ -190,13 +190,10 @@ type ModuleAdded struct {
 	Unissued []string `json:"unissued,omitempty"`
 }
 
-// ModuleRefused is a value missing for a module the node runs, in the words the composition stood in for it with. Today,
-// in CandidateAnswer.Held, is how the node keeps it today (held, failed, runs a build of the old form, held behind
-// another), which the candidate keeps.
+// ModuleRefused is a value missing for a module the node runs, in the words the composition stood in for it with.
 type ModuleRefused struct {
 	Module string `json:"module"`
 	Why    string `json:"why"`
-	Today  string `json:"today,omitempty"`
 }
 
 // VerdictOf is the verdict of an answer's lists: refused on any refusal, differs on any difference, addition or

go/delivery/stepasks.go

@@ -345,6 +345,11 @@ type CheckJob struct {
 	// has to list them. The build step hands them to its test tool as the argument NodeNamesArg, and the build module
 	// gives them to the tests in NodeNamesEnv.
 	NodeNames []string `json:"node-names,omitempty"`
+	// MergeOnly says the build module answers the fetch alone, its merge or the files in conflict, and runs no level: the
+	// checker asks it of a delivery group's members after one member's check failed, so one verdict names every member
+	// that does not merge into its trunk instead of the first. Levels stay named, so a build module from before it (which
+	// ignores it) still takes the ask; the checker reads only the fetch of such an answer.
+	MergeOnly bool `json:"merge-only,omitempty"`
 }
 
 // NodeNamesArg is the test tool's argument that carries a check's node names, and NodeNamesEnv the environment