Leave an eligible index for a confirmed collect instead of letting it go alone

Let go of alone after a build, an index's platforms stayed for ever under a record that
said collected, so no later collect could reach them (re-review of #144).

cmd/mesh-controller/collect.go

@@ -59,6 +59,9 @@ type sweepResult struct {
 	LetGo []string
 	// Skipped is how many references the sweep will not address (novox/hq issue 226).
 	Skipped int
+	// Indexes is how many eligible indexes a sweep a person did not confirm left for one that is: an
+	// index goes only with its platform manifests (novox/hq ADR 0257 §4).
+	Indexes int
 	// Left is how many eligible references were not asked about this time.
 	Left int
 	// Bounded is whether it stopped at its bounds rather than at a refusal.
@@ -85,7 +88,7 @@ func sweep(ctx context.Context, inv *inventory.Inventory, store artifacts.Store,
 	// 0257), and a kept index keeps its own: which platform manifests the kept indexes name is read
 	// from the store for every repository the sweep will touch, before the first delete. A single
 	// read that fails stops the sweep before it deletes anything, as a kept archive that cannot be
-	// held does. The sweep after a build lets an index go alone, as it always has.
+	// held does. The sweep after a build leaves an eligible index for such a collect, below.
 	store.Spare = nil
 	if bounds.platforms {
 		if inv == nil {
@@ -124,6 +127,22 @@ func sweep(ctx context.Context, inv *inventory.Inventory, store artifacts.Store,
 			}
 			break
 		}
+		if !bounds.platforms {
+			// **An index waits for a confirmed collect** (novox/hq ADR 0257 §4). Let go of alone,
+			// its platform manifests would stay in the store and the record would say collected, so
+			// no later sweep would offer it again and they would stay for ever. Left eligible, the
+			// next collect a person confirms takes it with them.
+			index, err := store.IsIndex(within, reference)
+			if err != nil && !errors.Is(err, artifacts.Gone) && !errors.Is(err, artifacts.ErrNotOurs) {
+				r.Stopped = fmt.Sprintf("the artifact store could not say whether %s is an index, so nothing more was asked of it: %v", reference, err)
+				r.Left = len(references) - i
+				break
+			}
+			if index {
+				r.Indexes++
+				continue
+			}
+		}
 		err := store.LetGo(within, reference)
 		if err == nil || errors.Is(err, artifacts.Gone) {
 			// Gone is the outcome wanted, already true. Recorded so the next sweep does not ask
@@ -215,6 +234,9 @@ func collect(ctx context.Context, inv *inventory.Inventory) {
 	if r.Skipped > 0 {
 		fmt.Fprintf(os.Stderr, "%d artifact(s) the sweep will not address were skipped\n", r.Skipped)
 	}
+	if r.Indexes > 0 {
+		fmt.Fprintf(os.Stderr, "%d eligible index(es) wait for a collect a person confirms, which takes their platforms too\n", r.Indexes)
+	}
 }
 
 // holdKept holds every kept archive by its manifest, stopping at the first refusal by the store.

cmd/mesh-controller/mirrors_test.go

@@ -255,14 +255,32 @@ func TestAConfirmedCollectLetsAnIndexGoWithItsOwnPlatformsOnly(t *testing.T) {
 	}
 }
 
-// The sweep after a build lets an index go alone: its platforms wait for a person's collect.
-func TestTheSweepAfterABuildLetsAnIndexGoAlone(t *testing.T) {
+// The sweep after a build leaves an eligible index in place and eligible: let go of alone, its
+// platforms would stay for ever under a record that says collected. A later collect a person confirms
+// takes it with the platform only it names. An image the same sweep reaches is let go of as before.
+func TestTheSweepAfterABuildLeavesAnIndexForAConfirmedCollect(t *testing.T) {
 	inv := inventory.ForTest(t)
 	_, eligible, reg := twoCopies(t, inv)
+	reg.held[copyDigest(5)] = true
+	image := catalogue.ArtifactStoreScheme + "upstream/docker.io/library/golang@" + copyDigest(5)
 	store := reg.serve(t)
-	r := sweep(t.Context(), inv, store, []string{eligible}, nil, afterBuild)
-	if !slices.Equal(r.LetGo, []string{eligible}) || !slices.Equal(reg.deleted, []string{copyDigest(2)}) {
-		t.Fatalf("after a build: let go %v, deleted %v; want the index alone", r.LetGo, reg.deleted)
+	r := sweep(t.Context(), inv, store, []string{eligible, image}, nil, afterBuild)
+	if r.Indexes != 1 || !slices.Equal(r.LetGo, []string{image}) || !slices.Equal(reg.deleted, []string{copyDigest(5)}) {
+		t.Fatalf("after a build: %d indexes left, let go %v, deleted %v; want the index left and the image gone",
+			r.Indexes, r.LetGo, reg.deleted)
+	}
+	left, err := inv.ToCollect(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !slices.Equal(left, []string{eligible}) {
+		t.Fatalf("after the sweep the records offer %v; want the index still eligible", left)
+	}
+	a := runCollect(t.Context(), inv, store, left, nil, true,
+		sweepBounds{most: 10, budget: 5 * time.Second, platforms: true})
+	if !slices.Equal(a.LetGo, []string{eligible}) ||
+		!slices.Equal(reg.deleted, []string{copyDigest(5), copyDigest(13), copyDigest(2)}) {
+		t.Fatalf("a confirmed collect let go %v, deleted %v; want the index with its own platform", a.LetGo, reg.deleted)
 	}
 }
 

internal/artifacts/store.go

@@ -261,6 +261,21 @@ func split(path string) (repository, kind, digest string, err error) {
 	return "", "", "", fmt.Errorf("%w: %q names nothing the store holds by digest", ErrNotOurs, path)
 }
 
+// IsIndex is whether a recorded reference is an index the store holds: one that names manifests. A
+// blob, or a manifest that names none, is not. Gone when the store does not hold it.
+func (s Store) IsIndex(ctx context.Context, reference string) (bool, error) {
+	path, ours := catalogue.InArtifactStore(reference)
+	if !ours {
+		return false, fmt.Errorf("%w: %s", ErrNotOurs, reference)
+	}
+	repository, kind, digest, err := split(path)
+	if err != nil || kind != "manifests" {
+		return false, err
+	}
+	children, err := s.Platforms(ctx, repository, digest)
+	return len(children) > 0, err
+}
+
 // HoldsManifest is whether the store holds the manifest a recorded image reference names.
 func (s Store) HoldsManifest(ctx context.Context, reference string) (bool, error) {
 	path, ours := catalogue.InArtifactStore(reference)

internal/catalogue/verbs.go

@@ -451,7 +451,8 @@ var ControllerVerbs = []Verb{
 		"listed, nothing held or deleted. With confirm and why: every kept archive held first, then each eligible " +
 		"artifact let go of, oldest first, and recorded collected — a hand act, recorded with its why. A confirmed " +
 		"collect lets an index go with the platform manifests no kept index names (read first; any read that " +
-		"fails lets nothing go); the sweep after a build lets an index go alone (novox/hq ADR 0257). Bounded by " +
+		"fails lets nothing go); the sweep after a build leaves an eligible index for such a collect (novox/hq ADR " +
+		"0257). Bounded by " +
 		"most (500 by default, at most 5000) and 45 seconds; what is left is said. Bytes are reclaimed by the " +
 		"store's nightly collector. Never touches a digest the mesh did not record (novox/hq ADR 0189, ADR 0251).",
 		Input: schema(map[string]string{