Check the mirrors as ADR 0356 says, and prove a removal asks for the lease

Reviews 251 of mesh-controller#315 and ADR 0356 v4 (hq issue 652): a removal
without the lease removes nothing; a module no longer registered loses its
declared-config key with its event; a listed mirror is never reported as
undeclared state; only the audit and a mirror's own writer name a mirror's
bucket; and the mirrors' list and the writers table agree. The store test's
line moves where it merges with the integration branch's own new lines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/broker/controller_buckets_test.go

@@ -125,3 +125,56 @@ func TestWhatTheControllerAskedIsBounded(t *testing.T) {
 		t.Errorf("history %d, age %s, bytes %d", status.History(), status.TTL(), info.Config.MaxBytes)
 	}
 }
+
+// **A listed mirror is no undeclared bucket** (ADR 0356 §3, ADR 0201 §16): the controller's list of its mirrors
+// stands for their declaration, so the report of state nothing declares never names one.
+func TestAListedMirrorIsNotReportedAsUndeclared(t *testing.T) {
+	names := []string{"gone_state"}
+	for _, m := range ControllerMirrors() {
+		names = append(names, BucketName(ControllerSeat, m))
+	}
+	undeclared, err := RaiseBuckets(fakeBuckets{names: names}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !slices.Equal(undeclared, []string{"gone_state"}) {
+		t.Fatalf("the report of undeclared state names %v", undeclared)
+	}
+}
+
+// **The mirrors' list and the writers table agree** (ADR 0356 §3, to-be 45 §1): every listed mirror is a row of
+// the table whose one writer is the controller, on its bucket alone; and every row that writes a bucket of the
+// controller's named as a mirror is on the list.
+func TestTheMirrorsListAndTheWritersTableAgree(t *testing.T) {
+	controller := Principal{Kind: KindController}
+	module := Principal{Kind: KindModule, Module: ControllerSeat, Node: "one"}
+	rowed := map[string]bool{}
+	for _, row := range WritersTable {
+		if !strings.HasSuffix(row.State, " mirror") {
+			continue
+		}
+		for _, subject := range row.Subjects {
+			bucket := strings.TrimSuffix(strings.TrimPrefix(subject, "$KV."), ".>")
+			rowed[bucket] = true
+			if row.Writes == nil || !row.Writes(controller, subject) || row.Writes(module, subject) {
+				t.Errorf("%q is not written by the controller alone", row.State)
+			}
+		}
+	}
+	listed := map[string]bool{}
+	for _, m := range ControllerMirrors() {
+		bucket := BucketName(ControllerSeat, m)
+		listed[bucket] = true
+		if !rowed[bucket] {
+			t.Errorf("the mirror %s is listed and no row of the writers table writes it", bucket)
+		}
+		if !IsControllerBucket(bucket) || !slices.Contains(ControllerBuckets(), bucket) {
+			t.Errorf("the mirror %s is listed and not asserted as the controller's own", bucket)
+		}
+	}
+	for bucket := range rowed {
+		if !listed[bucket] {
+			t.Errorf("the writers table writes the mirror %s, which the list does not name", bucket)
+		}
+	}
+}

internal/recordmirror/mirror_store_test.go

@@ -155,4 +155,24 @@ func TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent(t *tes
 	if len(said) != 1 || said[0].Key != "anchor.ui" || said[0].Change != Removed {
 		t.Errorf("an unassignment raised %+v", said)
 	}
+
+	// **A module no longer registered** has its key removed, said once as `removed` (ADR 0356 §4).
+	if err := inv.ForgetModule(ctx, "notes"); err != nil {
+		t.Fatal(err)
+	}
+	declaredWrites = writes(t, api, DeclaredConfig.Bucket)
+	declaredSaid = len(heardDeclared.all())
+	if _, rd := pass(); rd != (Result{Removed: 1}) {
+		t.Errorf("a module forgotten did %+v", rd)
+	}
+	if got := writes(t, api, DeclaredConfig.Bucket) - declaredWrites; got != 1 {
+		t.Errorf("a module forgotten changed %d keys of the declared config", got)
+	}
+	if _, err := declared.KV.Get(ctx, "notes"); err == nil {
+		t.Error("a module forgotten keeps its key")
+	}
+	said = settle(heardDeclared, declaredSaid+1)[declaredSaid:]
+	if len(said) != 1 || said[0].Key != "notes" || said[0].Change != Removed {
+		t.Errorf("a module forgotten raised %+v", said)
+	}
 }

internal/recordmirror/mirror_test.go

@@ -312,6 +312,31 @@ func TestNatsAWriterWithoutTheLeaseWritesNothing(t *testing.T) {
 	}
 }
 
+// **A removal asks for the lease too** (ADR 0289 §9): a writer that held the lease when its pass began and lost
+// it before its first removal purges nothing, and says it does not hold the lease.
+func TestNatsARemovalWithoutTheLeaseRemovesNothing(t *testing.T) {
+	js, api := aMirrorBus(t)
+	ctx := t.Context()
+	held := aWriter(t, js, api, Assignments)
+	if _, err := held.Sync(ctx, map[string][]byte{"anchor.gitea": []byte(`{"a":1}`)}); err != nil {
+		t.Fatal(err)
+	}
+	before := writes(t, api, Assignments.Bucket)
+	asked := 0
+	w := aWriter(t, js, api, Assignments)
+	w.Holds = func() bool { asked++; return asked == 1 }
+	r, err := w.Sync(ctx, map[string][]byte{})
+	if !errors.Is(err, ErrNotHeld) || r != (Result{}) || asked != 2 {
+		t.Errorf("a removal without the lease did %+v, %v (the lease asked %d times)", r, err, asked)
+	}
+	if got := writes(t, api, Assignments.Bucket) - before; got != 0 {
+		t.Errorf("a removal without the lease wrote the bucket %d times", got)
+	}
+	if _, err := w.KV.Get(ctx, "anchor.gitea"); err != nil {
+		t.Errorf("the key was removed without the lease: %v", err)
+	}
+}
+
 // **A key the bus cannot hold is refused alone**, and the others are written.
 func TestAKeyTheBusCannotHoldIsRefusedAlone(t *testing.T) {
 	if validKey("anchor.gitea") != nil || validKey("bad key") == nil || validKey(".lead") == nil ||

internal/recordmirror/readers_test.go

@@ -0,0 +1,82 @@
+package recordmirror
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"io/fs"
+	"path/filepath"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-controller/internal/broker"
+)
+
+// mayNameAMirror are the files of the controller that may name a record mirror's bucket outside their tests:
+// where the buckets are defined, asserted and granted (internal/broker), the list a reader names in `reads`
+// (internal/catalogue/state.go), and each mirror's own writer (this package and the loop that runs it). The
+// audit of a mirror against its store (ADR 0289 §4, issue 419) joins this list when it is built.
+var mayNameAMirror = []string{
+	"cmd/mesh-controller/record_mirrors.go",
+	"internal/broker/controller_buckets.go",
+	"internal/broker/writers.go",
+	"internal/catalogue/state.go",
+}
+
+// **Nothing in the controller reads a mirror to decide anything** (ADR 0289 §19 as ADR 0356 §2 changes it): no
+// file of the controller's packages outside its tests names a mirror's bucket, by its Go name or its bus name,
+// but the audit and a mirror's own writer. A reader elsewhere fails here, naming the file and what it named.
+func TestOnlyTheAuditAndAMirrorsWriterReadAMirror(t *testing.T) {
+	names := []string{"AssignmentsBucket", "DeclaredConfigBucket", "MirrorAssignments", "MirrorDeclaredConfig",
+		"ControllerMirrors", "Assignments", "DeclaredConfig", "Mirrors"}
+	var buckets []string
+	for _, m := range broker.ControllerMirrors() {
+		buckets = append(buckets, broker.BucketName(broker.ControllerSeat, m))
+	}
+	root := filepath.Join("..", "..")
+	var found []string
+	for _, dir := range []string{"cmd", "internal"} {
+		err := filepath.WalkDir(filepath.Join(root, dir), func(path string, e fs.DirEntry, err error) error {
+			if err != nil || e.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
+				return err
+			}
+			rel, err := filepath.Rel(root, path)
+			if err != nil {
+				return err
+			}
+			rel = filepath.ToSlash(rel)
+			if strings.HasPrefix(rel, "internal/recordmirror/") || slices.Contains(mayNameAMirror, rel) {
+				return nil
+			}
+			f, err := parser.ParseFile(token.NewFileSet(), path, nil, 0)
+			if err != nil {
+				return err
+			}
+			ast.Inspect(f, func(n ast.Node) bool {
+				switch n := n.(type) {
+				case *ast.SelectorExpr:
+					pkg, ok := n.X.(*ast.Ident)
+					if ok && (pkg.Name == "broker" || pkg.Name == "recordmirror") && slices.Contains(names, n.Sel.Name) {
+						found = append(found, rel+": "+pkg.Name+"."+n.Sel.Name)
+					}
+				case *ast.BasicLit:
+					for _, b := range buckets {
+						if n.Kind == token.STRING && strings.Contains(n.Value, b) {
+							found = append(found, rel+": "+n.Value)
+						}
+					}
+				}
+				return true
+			})
+			return nil
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	if len(found) > 0 {
+		t.Fatalf("a package of the controller reads a record mirror, which only the audit and the mirror's "+
+			"writer may (ADR 0289 §19, ADR 0356 §2):\n  %s", strings.Join(found, "\n  "))
+	}
+}

testdata/store-tests

@@ -49,6 +49,7 @@
 ./internal/inventory TestACredentialIssuedOutsideIsNeverMade
 ./internal/inventory TestOneAcceptForAProviderIsSealedToEveryConsumer
 ./internal/inventory TestASharedOwnSecretIssuedOutsideIsNeverMade
+./internal/recordmirror TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent
 ./cmd/mesh-controller TestOneAcceptForAProviderIsPlacedForEveryConsumer
 ./cmd/mesh-controller TestReplay164
 ./cmd/mesh-controller TestAnApprovedLayerOpensTheWalkThatSendsIt
@@ -61,4 +62,3 @@
 ./cmd/mesh-controller TestAProviderElsewhereServesItsOwnComposedName
 ./cmd/mesh-controller TestAReusedCheckBuildIsRegisteredByItsDigestAndGatedAsABuiltOne
 ./internal/inventory TestAReportOfABusyContainerIsKeptAndReadBack
-./internal/recordmirror TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent