Clear a module's fingerprint-not-composed on every successful take-in, and stop serve's goroutines before an error return undoes anything (issue 1945)

A success cleared the condition only when the same follower had raised it, so
one raised before a restart or by another follower stayed open after the take-in
worked, and the operator read that a build needed a look. Clearing a condition
that is not open costs one read. On an error return, serve now ends its
goroutines before the keeper closes and before handActConn and the bus are put
back (the review of fix 12).

cmd/mesh-controller/composed_keys.go

@@ -166,10 +166,12 @@ func followDoneKey(ctx context.Context, open *stores, module string) {
 		err := takeOnceInSlot(ctx, open, module)
 		o := doneKeyObservation(module)
 		if err == nil {
-			if keeper := conditionsFrom; keeper != nil && failed {
+			// Cleared on every success, whoever raised it: a controller before a restart or another follower (issue
+			// 1945). A condition that is not open costs one read.
+			if keeper := conditionsFrom; keeper != nil {
 				_, _ = keeper.Clear(ctx, o.Key(), "composed and granted")
-				failed = false
 			}
+			failed = false
 			if _, again := doneKeysLookAgain.LoadAndDelete(module); again {
 				continue // a wake came while this one ran: once more, for the key written meanwhile
 			}

cmd/mesh-controller/composed_keys_cancel_test.go

@@ -59,3 +59,25 @@ func TestATakeInCutByACancellationRaisesNothing(t *testing.T) {
 		})
 	}
 }
+
+// A take-in that succeeds clears the module's fingerprint-not-composed condition whoever raised it: a controller before a
+// restart, or another follower (issue 1945: the operator read "a build needs a look" for take-ins that had since worked).
+func TestASucceedingTakeInClearsAConditionAnotherRaised(t *testing.T) {
+	was, wasRetry, wasSay, wasKeeper := doneKeyTakeOnce, doneKeyRetry, doneKeysSay, conditionsFrom
+	t.Cleanup(func() { doneKeyTakeOnce, doneKeyRetry, doneKeysSay, conditionsFrom = was, wasRetry, wasSay, wasKeeper })
+	store := conditions.NewInMemory()
+	k := conditions.NewKeeper(t.Context(), conditions.Options{Store: store, History: store, Teller: &conditions.Told{}})
+	t.Cleanup(func() { k.Close(context.Background()) })
+	conditionsFrom = k
+	doneKeysSay = func(string) {}
+	o := doneKeyObservation("raised-before")
+	o.Summary += "the store refused the connection"
+	if _, err := k.Observe(t.Context(), o); err != nil {
+		t.Fatal(err)
+	}
+	doneKeyTakeOnce = func(context.Context, *stores, string) error { return nil }
+	takeDoneKey(t.Context(), nil, "raised-before")
+	if c, found, err := conditions.ReadOne(t.Context(), store, o.Key()); err != nil || found {
+		t.Fatalf("a take-in that succeeded left the condition another raised: %+v (found %v, %v)", c, found, err)
+	}
+}

cmd/mesh-controller/push.go

@@ -179,6 +179,7 @@ func serve(ctx context.Context) (err error) {
 	keeper := openServingConditions(ctx, open, bus)
 	if keeper != nil {
 		defer closeConditions(keeper)
+		defer stopRunning() // on an error return, the goroutines end before the keeper closes
 	}
 
 	// The bus's own objects, asserted on every start. **Not created once at genesis**: a stream
@@ -247,10 +248,12 @@ func serve(ctx context.Context) (err error) {
 	// next test's `conditions` with "connection closed", issue 1945).
 	handActConn = bus.Conn
 	defer func() { handActConn = nil }()
+	defer stopRunning() // on an error return, the goroutines end before handActConn is put back
 	// And everything else this controller does on the bus for a moment (novox/hq issue 327).
 	servingBus.Store(server.JetStream())
 	// No longer serving: nothing is lent, and dead-letters says it is not read here (novox/hq issue 330).
 	defer servingBus.Store(nil)
+	defer stopRunning() // on an error return, the goroutines end before the bus is taken back
 	// And says when it replaced a value given by hand (novox/hq ADR 0228).
 	givenEvents = bus
 	// And a pull request's merge check, asked when the forge announces its head and said when judged