Send the bus's machine before the grants, and only when its user list moved (hq issue 249)

Grants first could hold back the very declaration that lets the controller
issue them. The holder now goes first, then buckets and memberships, then
the rest; a membership that fails holds back only its own machine, and an
announcement whose send stopped at its grants is asked again. Whether the
holder must go first is read from a digest of the user list it was last
sent, not its whole declaration. Migration renumbered to 0058.

cmd/mesh-controller/delivery_order_test.go

@@ -6,6 +6,8 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+
+	"github.com/novox/mesh-controller/internal/link"
 )
 
 // recordingDelivery is a delivery that writes down what was done, in order, and fails where told.
@@ -39,10 +41,11 @@ func ready(names ...string) []readyNode {
 }
 
 // novox/hq issue 249: the grants that come with a module's new declarations are issued before any
-// machine is sent the code that uses them.
+// machine is sent the code that uses them — except the machine holding the bus, whose declaration
+// carries the controller's own right to issue them, and goes first.
 func TestGrantsAreIssuedBeforeTheDeclarations(t *testing.T) {
 	d := &recordingDelivery{}
-	digests, err := deliver(t.Context(), d, ready("anchor", "laptop"))
+	digests, err := deliver(t.Context(), d, "", ready("anchor", "laptop"))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -53,27 +56,71 @@ func TestGrantsAreIssuedBeforeTheDeclarations(t *testing.T) {
 	if digests["anchor"] != "digest-anchor" || digests["laptop"] != "digest-laptop" {
 		t.Fatalf("the digests sent were not answered: %v", digests)
 	}
+
+	held := &recordingDelivery{}
+	if _, err := deliver(t.Context(), held, "broker", ready("broker", "anchor")); err != nil {
+		t.Fatal(err)
+	}
+	want = []string{"declare broker", "grant broker", "grant anchor", "declare anchor"}
+	if !reflect.DeepEqual(held.did, want) {
+		t.Fatalf("with the bus's machine in the send: %v, wanted %v", held.did, want)
+	}
 }
 
-// A grant that cannot be issued sends nothing and is an error — never "until the next push".
-func TestAGrantThatFailsSendsNothingAndSaysSo(t *testing.T) {
-	d := &recordingDelivery{grantErr: errors.New("the bus refused the membership")}
-	_, err := deliver(t.Context(), d, ready("anchor", "laptop"))
-	if err == nil || !strings.Contains(err.Error(), "the bus refused the membership") {
+// A grant that cannot be issued holds back the machines it concerns and is an error the caller
+// retries on — never "until the next push" — and the bus's own machine is sent regardless, so the
+// grant that would let the controller issue memberships is never held behind them.
+func TestAGrantThatFailsHoldsBackWhatItConcerns(t *testing.T) {
+	// A failure naming no machine (the buckets): everything but the bus's machine.
+	d := &recordingDelivery{grantErr: errors.New("the bus refused the bucket")}
+	_, err := deliver(t.Context(), d, "broker", ready("broker", "anchor", "laptop"))
+	if err == nil || !errors.Is(err, errGrants) || !strings.Contains(err.Error(), "the bus refused the bucket") ||
+		!strings.Contains(err.Error(), "anchor, laptop not sent") {
 		t.Fatalf("a failed grant was not said as the send's failure: %v", err)
 	}
-	for _, did := range d.did {
-		if strings.HasPrefix(did, "declare") {
-			t.Fatalf("code was sent after its grant failed: %v", d.did)
-		}
+	if want := []string{"declare broker", "grant broker", "grant anchor", "grant laptop"}; !reflect.DeepEqual(d.did, want) {
+		t.Fatalf("delivered %v, wanted the bus's machine alone", d.did)
 	}
+
+	// A membership that failed for one machine: that machine alone.
+	one := &recordingDelivery{grantErr: &grantsRefused{nodes: map[string]error{"laptop": errors.New("no")}}}
+	_, err = deliver(t.Context(), one, "", ready("anchor", "laptop"))
+	if !errors.Is(err, errGrants) || !strings.Contains(err.Error(), "laptop not sent") {
+		t.Fatalf("one machine's refused membership was not said: %v", err)
+	}
+	if want := []string{"grant anchor", "grant laptop", "declare anchor"}; !reflect.DeepEqual(one.did, want) {
+		t.Fatalf("delivered %v, wanted anchor sent and laptop held back", one.did)
+	}
+
+	// The announced upgrade that hit it is asked again.
+	if !errors.Is(askAgainOnGrants(err), link.ErrTryAgain) {
+		t.Fatal("an announcement whose send stopped at its grants is not asked again")
+	}
+	if other := errors.New("laptop could not be resolved"); errors.Is(askAgainOnGrants(other), link.ErrTryAgain) {
+		t.Fatal("any failure is asked again, not only a grant's")
+	}
+
 	// Nothing to send is nothing granted either.
 	none := &recordingDelivery{grantErr: errors.New("never asked")}
-	if _, err := deliver(t.Context(), none, nil); err != nil || len(none.did) != 0 {
+	if _, err := deliver(t.Context(), none, "", nil); err != nil || len(none.did) != 0 {
 		t.Fatalf("an empty send granted or failed: %v %v", none.did, err)
 	}
 }
 
+// Whether the bus's machine goes first is read from the user list alone, by its digest.
+func TestTheBusMachineIsBehindByItsUserListAlone(t *testing.T) {
+	list := "users: [a, b]"
+	if userListBehind(list, digestOf([]byte(list))) {
+		t.Fatal("the list it was sent reads as behind")
+	}
+	if !userListBehind(list, digestOf([]byte("users: [a]"))) || !userListBehind(list, "") {
+		t.Fatal("a changed or never-sent list reads as current")
+	}
+	if userListBehind("", "") {
+		t.Fatal("a machine sent no list reads as behind")
+	}
+}
+
 // The machine holding the bus goes first: its declaration carries the user list the new grants are
 // checked against. Among the machines it is moved to the front; not among them it is added only
 // when it is behind.