Merge pull request 'Judge a seat's holder silent only on verbs it must serve (hq issue 299)' (#121) from fix/optional-verbs-are-not-silence into main

2026-10-07 21:01:32 +00:00
parent 7dbe80ad63 dd668ec887
commit c37d7742ba
2 changed files with 168 additions and 41 deletions
@@ -0,0 +1,94 @@
+package main
+
+import (
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+)
+
+// The live case of 2026-10-07 (novox/hq issue 299), replayed: the node-uplink seat gained its first
+// verbs, `resolvers` and `links`, both optional while its holders catch up, and the holders serve
+// neither. The working set is read from the store, whose rows carry no optional mark, so the seat comes
+// back through UseSeats as it does live. Its holder on every machine answers nothing for it on the bus,
+// and must not be judged silent: a seat whose verbs are all optional asks no holder to answer.
+func TestAHolderOfASeatWhoseVerbsAreAllOptionalIsNotSilent(t *testing.T) {
+	defer catalogue.UseSeats(catalogue.DefaultSeats())
+	var rows []catalogue.Seat
+	for _, s := range catalogue.DefaultSeats() {
+		stored := s
+		stored.Serves = nil
+		for _, v := range s.Serves {
+			v.Optional = false // the store never keeps the mark
+			stored.Serves = append(stored.Serves, v)
+		}
+		rows = append(rows, stored)
+	}
+	catalogue.UseSeats(rows)
+
+	uplink, ok := catalogue.SeatNamed("node-uplink")
+	if !ok || len(uplink.Serves) == 0 {
+		t.Fatalf("node-uplink must serve verbs for this replay: %+v", uplink)
+	}
+	for _, v := range uplink.Serves {
+		if !v.Optional {
+			t.Fatalf("node-uplink's %s is required; this replay is of a seat whose verbs are all optional", v.Name)
+		}
+	}
+
+	nodes := []string{"anchor", "home-server", "workstation", "laptop"}
+	heard := map[string]bool{}
+	var recorded []catalogue.Held
+	for _, n := range nodes {
+		heard[n] = true
+		recorded = append(recorded, catalogue.Held{Claim: "node-uplink", Scope: catalogue.ScopeNode, Node: n,
+			Module: "networkmanager"})
+	}
+	expected := holdersToHear(catalogue.SeatsWithAProtocol(), recorded, nil, heard, nil, time.Now())
+	if _, asked := expected["node-uplink"]; asked {
+		t.Fatalf("node-uplink's holders are expected to answer though every verb of the seat is optional: %v",
+			expected["node-uplink"])
+	}
+	// Nothing answers for the seat, as live: no silence is said about it.
+	for _, o := range silentHolders(expected, map[string]map[string]bool{}) {
+		if o.ID == "node-uplink."+o.Machine {
+			t.Errorf("a holder serving none of a seat's optional verbs was judged silent: %s", o.Summary)
+		}
+	}
+}
+
+// Silence is still judged on a required verb: a seat with one required and one optional verb, held on a
+// machine that is heard from, whose holder answers nothing — silent.
+func TestAHolderServingNoRequiredVerbIsStillSilent(t *testing.T) {
+	seat := catalogue.Seat{Name: "example-seat", Scope: catalogue.ScopeNode, Serves: []catalogue.Verb{
+		{Name: "state"}, {Name: "later", Optional: true}}}
+	recorded := []catalogue.Held{{Claim: "example-seat", Scope: catalogue.ScopeNode, Node: "laptop", Module: "m"}}
+	expected := holdersToHear([]catalogue.Seat{seat}, recorded, nil, map[string]bool{"laptop": true}, nil, time.Now())
+	if !expected["example-seat"]["laptop"] {
+		t.Fatalf("a holder of a seat with a required verb is not expected to answer: %v", expected)
+	}
+	out := silentHolders(expected, map[string]map[string]bool{})
+	if len(out) != 1 || out[0].ID != "example-seat.laptop" || out[0].Token != "silent" {
+		t.Fatalf("a holder serving no required verb is not said to be silent: %+v", out)
+	}
+	if got := silentHolders(expected, map[string]map[string]bool{"example-seat": {"laptop": true}}); len(got) != 0 {
+		t.Fatalf("a holder that answers is said to be silent: %+v", got)
+	}
+}
+
+func TestHoldingNeedsAnAnswer(t *testing.T) {
+	for _, c := range []struct {
+		name   string
+		serves []catalogue.Verb
+		want   bool
+	}{
+		{"no verb", nil, false},
+		{"only optional verbs", []catalogue.Verb{{Name: "a", Optional: true}, {Name: "b", Optional: true}}, false},
+		{"a required verb among optional ones", []catalogue.Verb{{Name: "a", Optional: true}, {Name: "b"}}, true},
+		{"only required verbs", []catalogue.Verb{{Name: "a"}}, true},
+	} {
+		if got := holdingNeedsAnAnswer(catalogue.Seat{Name: "s", Serves: c.serves}); got != c.want {
+			t.Errorf("%s: holdingNeedsAnAnswer = %v, want %v", c.name, got, c.want)
+		}
+	}
+}
@@ -419,8 +419,9 @@ func askResolver(ctx context.Context, at, name string, kind dnsmessage.Type) ([]
 	return addresses, answer.RCode, nil
 }

-// probeHolders is D3: every seat that serves verbs has, on every machine that holds it and is heard
-// from, a holder answering the bus's discovery for that seat. A machine past its heartbeat's bound is
+// probeHolders is D3: every seat with a verb its holder must serve has, on every machine that holds it
+// and is heard from, a holder answering the bus's discovery for that seat. A seat whose verbs are all
+// still optional is not asked about (holdingNeedsAnAnswer). A machine past its heartbeat's bound is
 // S1's, and is not asked about here.
 func probeHolders(ctx context.Context, d *doctor) ([]conditions.Observation, error) {
 	entries, err := d.open.inventory.Catalogued(ctx)
@@ -436,44 +437,7 @@ func probeHolders(ctx context.Context, d *doctor) ([]conditions.Observation, err
 	if err != nil {
 		return nil, err
 	}
-	now := time.Now()
-	expected := map[string]map[string]bool{} // seat → machine
-	add := func(seat, node string) {
-		if expected[seat] == nil {
-			expected[seat] = map[string]bool{}
-		}
-		expected[seat][node] = true
-	}
-	for _, s := range catalogue.SeatsWithAProtocol() {
-		if len(s.Serves) == 0 || s.Name == catalogue.ControllerSeatName {
-			continue // a seat with no verb has nothing to answer with; this controller is answering now
-		}
-		onRecord := false
-		for _, h := range recorded {
-			if h.Claim == s.Name && heard[h.Node] {
-				add(s.Name, h.Node)
-				onRecord = true
-			}
-		}
-		if onRecord && s.Scope == catalogue.ScopeMesh {
-			continue
-		}
-		for _, e := range entries {
-			for _, c := range e.Manifest.Claims {
-				if c.Name != s.Name {
-					continue
-				}
-				for _, node := range e.On {
-					// Assigned is not there yet: a holder is expected once its machine was sent it and
-					// had time to report, never between `assign` and `push` (novox/hq issue 275).
-					if heard[node] && (s.Scope == catalogue.ScopeNode || !onRecord) &&
-						delivered[node].settled(e.Manifest.Module, now) {
-						add(s.Name, node)
-					}
-				}
-			}
-		}
-	}
+	expected := holdersToHear(catalogue.SeatsWithAProtocol(), recorded, entries, heard, delivered, time.Now())
 	if len(expected) == 0 {
 		return nil, nil
 	}
@@ -509,6 +473,11 @@ func probeHolders(ctx context.Context, d *doctor) ([]conditions.Observation, err
 			}
 		}
 	}
+	return silentHolders(expected, answering), nil
+}
+
+// silentHolders is every expected holder the discovery did not hear, as D3's observations.
+func silentHolders(expected, answering map[string]map[string]bool) []conditions.Observation {
 	var out []conditions.Observation
 	for seat, nodes := range expected {
 		for node := range nodes {
@@ -521,7 +490,71 @@ func probeHolders(ctx context.Context, d *doctor) ([]conditions.Observation, err
 				Said:    fmt.Sprintf("no answer for %s from %s to the bus's discovery", seat, node)})
 		}
 	}
-	return sortedFound(out), nil
+	return sortedFound(out)
+}
+
+// holdersToHear is D3's expectation: seat → every machine whose holder of it must answer the bus's
+// discovery. A seat is listed only if holding it requires serving a verb (holdingNeedsAnAnswer); a holder
+// is expected on a machine that is heard from and that holds it on record or — for a node seat, or a mesh
+// seat nobody holds on record — was sent the module that claims it and had time to report.
+func holdersToHear(seats []catalogue.Seat, recorded []catalogue.Held, entries []inventory.Entry,
+	heard map[string]bool, delivered map[string]lastSend, now time.Time) map[string]map[string]bool {
+	expected := map[string]map[string]bool{} // seat → machine
+	add := func(seat, node string) {
+		if expected[seat] == nil {
+			expected[seat] = map[string]bool{}
+		}
+		expected[seat][node] = true
+	}
+	for _, s := range seats {
+		if s.Name == catalogue.ControllerSeatName {
+			continue // this controller is answering now
+		}
+		if !holdingNeedsAnAnswer(s) {
+			continue // nothing its holder must serve, so nothing its silence would say (novox/hq issue 299)
+		}
+		onRecord := false
+		for _, h := range recorded {
+			if h.Claim == s.Name && heard[h.Node] {
+				add(s.Name, h.Node)
+				onRecord = true
+			}
+		}
+		if onRecord && s.Scope == catalogue.ScopeMesh {
+			continue
+		}
+		for _, e := range entries {
+			for _, c := range e.Manifest.Claims {
+				if c.Name != s.Name {
+					continue
+				}
+				for _, node := range e.On {
+					// Assigned is not there yet: a holder is expected once its machine was sent it and
+					// had time to report, never between `assign` and `push` (novox/hq issue 275).
+					if heard[node] && (s.Scope == catalogue.ScopeNode || !onRecord) &&
+						delivered[node].settled(e.Manifest.Module, now) {
+						add(s.Name, node)
+					}
+				}
+			}
+		}
+	}
+	return expected
+}
+
+// holdingNeedsAnAnswer says whether a seat's holder is judged silent when the bus's discovery hears
+// nothing from it: only when the seat has a verb its holder must serve. **Silence is judged on required
+// verbs alone** (novox/hq issue 299). An optional verb is one the seat's holders are still catching up
+// to (Verb.Optional; design 33 §7, novox/hq ADR 0246): a holder that does not serve it yet holds the seat
+// rightly, and a holder serving nothing because every verb is still optional is exactly that holder, not
+// a silent one. A seat with no verb at all has nothing to answer with either.
+func holdingNeedsAnAnswer(s catalogue.Seat) bool {
+	for _, v := range s.Serves {
+		if !v.Optional {
+			return true
+		}
+	}
+	return false
 }

 // reportGrace is how long a machine is given, after it was sent a declaration, to apply it and report