F-catalog-B: distribution's windows step by mesh-sdk's delivery.StepWindow and say why a node holds one

A read naming no node (an empty node_haves) closed both windows for good (security review F3, second of
a kind): the window embeds delivery.WindowHeld and steps by the shared rule, named being every node the
read named, and keeps its own guard that a window once closed stays closed. A node-engine whose
manifest-reads did not answer (a timeout included) counts as stating nothing as before, and the window's
record now says so per node (Why), so registry_old_name tells a timeout from an old node-engine. A read
that fails otherwise is an error the wake judges nothing on, no longer taken as not served.

Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig

modules/distribution/cmd/distribution/windows.go

@@ -91,9 +91,12 @@ func (d busDelivery) Reads(node string) (bool, []string, error) {
 	raw, err := d.c.CallOn(node, readsAsk, map[string]any{})
 	if err != nil {
 		if notServed(err) {
-			return false, nil, nil // a node-engine not serving manifest-reads states nothing (design 51)
+			// A node-engine not serving manifest-reads states nothing (design 51); what the call answered (an away node,
+			// an older node-engine, a timeout) goes with it, so the window's tool says why the node holds it.
+			return false, nil, err
 		}
-		return false, nil, err
+		// Any other failure is a read that did not happen: served true with its error, so the wake judges nothing.
+		return true, nil, err
 	}
 	var a struct {
 		Reads []string `json:"manifest-reads"`

modules/distribution/internal/holder/driver.go

@@ -3,6 +3,7 @@ package holder
 import (
 	"errors"
 	"fmt"
+	"maps"
 	"slices"
 	"sort"
 	"strings"
@@ -44,7 +45,8 @@ type NodeHave struct {
 
 // Delivery is what the windows' driver asks of the delivery parts and the node-engine seat, through their tools and
 // verbs on the bus (ADR 0596): every node's haves, away ones included, and one node-engine's stated reader
-// capabilities. Reads answers served false when the node-engine does not serve `manifest-reads` (it states nothing).
+// capabilities. Reads answers served false when the node-engine does not serve `manifest-reads` (it states nothing),
+// with the call's error, if any, saying why (an away node, an older node-engine, a timeout).
 type Delivery interface {
 	Haves() ([]NodeHave, error)
 	Reads(node string) (served bool, stated []string, err error)
@@ -118,6 +120,9 @@ func (d *Driver) Read() ([]NodeRead, error) {
 			return nil, fmt.Errorf("%s's manifest-reads cannot be read, so no window moves: %w", node, err)
 		}
 		n.Serves, n.Stated = served, stated
+		if !served && err != nil {
+			n.NotServed = err.Error()
+		}
 		out = append(out, *n)
 	}
 	return out, nil
@@ -131,13 +136,20 @@ func (d *Driver) Wake(why string) ([]string, error) {
 		return nil, err
 	}
 	now := d.now()
+	named := make([]string, 0, len(nodes))
+	byNode := map[string]NodeRead{}
+	for _, n := range nodes {
+		named = append(named, n.Node)
+		byNode[n.Node] = n
+	}
 	var said, errs []string
 	for _, c := range []struct {
 		name, event, seat string
 		holding           func([]NodeRead, []DroppedNode) []string
+		why               func(NodeRead) string
 	}{
-		{OldNameWindow, OldNameClosedEvent, OldSeat, HoldingOldName},
-		{DirectWritesWindow, DirectWritesClosedEvent, "", HoldingDirectWrites},
+		{OldNameWindow, OldNameClosedEvent, OldSeat, HoldingOldName, WhyOldName},
+		{DirectWritesWindow, DirectWritesClosedEvent, "", HoldingDirectWrites, WhyDirectWrites},
 	} {
 		cur, err := d.State.Window(c.name)
 		if err != nil {
@@ -145,9 +157,16 @@ func (d *Driver) Wake(why string) ([]string, error) {
 			continue
 		}
 		if cur.Name == "" {
-			cur = Window{Name: c.name, Open: true}
+			cur = Window{Name: c.name}
+			cur.Open = true
+		}
+		next, closed, overdue := cur.Step(named, c.holding(nodes, cur.Dropped), now)
+		if next.Open && len(named) > 0 {
+			next.Why = map[string]string{}
+			for _, n := range next.Holding {
+				next.Why[n] = c.why(byNode[n])
+			}
 		}
-		next, closed, overdue := cur.Step(c.holding(nodes, cur.Dropped), now)
 		if !windowsEqual(cur, next) {
 			if err := d.State.PutWindow(next); err != nil {
 				errs = append(errs, fmt.Sprintf("the %s window is not kept: %v", c.name, err))
@@ -172,6 +191,8 @@ func (d *Driver) Wake(why string) ([]string, error) {
 				}
 			}
 			said = append(said, fmt.Sprintf("the %s window is closed (%s); it was held last by %v", c.name, why, closed.LastHolding))
+		} else if len(named) == 0 {
+			said = append(said, fmt.Sprintf("the %s window is not judged (%s): the read named no node", c.name, why))
 		} else if next.Open {
 			said = append(said, fmt.Sprintf("the %s window stays open (%s): held by %v", c.name, why, next.Holding))
 		}
@@ -191,5 +212,5 @@ func (d *Driver) now() time.Time {
 
 func windowsEqual(a, b Window) bool {
 	return a.Open == b.Open && slices.Equal(a.Holding, b.Holding) && a.Closed.Equal(b.Closed) &&
-		len(a.Since) == len(b.Since) && len(a.Dropped) == len(b.Dropped)
+		len(a.Since) == len(b.Since) && len(a.Dropped) == len(b.Dropped) && maps.Equal(a.Why, b.Why)
 }

modules/distribution/internal/holder/holder_test.go

@@ -630,20 +630,26 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) {
 			t.Errorf("direct writes held by %v, want g14", got)
 		}
 		t0 := time.Unix(0, 0)
-		w, closed, _ := Window{Name: OldNameWindow}.Step([]string{"g14"}, t0)
+		all := []string{"ace", "g14", "novox"}
+		w, closed, _ := Window{Name: OldNameWindow}.Step(all, []string{"g14"}, t0)
 		if closed != nil || !w.Open {
 			t.Fatalf("Step = %+v, %+v", w, closed)
 		}
-		if _, _, overdue := w.Step([]string{"g14"}, t0.Add(WindowFindingAfter)); !reflect.DeepEqual(overdue, []string{"g14"}) {
+		if _, _, overdue := w.Step(all, []string{"g14"}, t0.Add(WindowFindingAfter)); !reflect.DeepEqual(overdue, []string{"g14"}) {
 			t.Errorf("seven days: overdue %v", overdue)
 		}
+		// Security review F3: a read naming no node (an empty node_haves) judges nothing; the window stays open.
+		if same, shut, _ := w.Step(nil, nil, t0.Add(time.Minute)); shut != nil || !same.Open || !reflect.DeepEqual(same.Holding, []string{"g14"}) {
+			t.Errorf("a read naming no node: %+v, closed %+v; want the window open as it was", same, shut)
+		}
 		w, _ = w.Drop("g14", "person.jochen", "retired", t0)
-		w, closed, _ = w.Step(HoldingOldName(nodes, w.Dropped), t0.Add(time.Hour))
+		w, closed, _ = w.Step(all, HoldingOldName(nodes, w.Dropped), t0.Add(time.Hour))
 		if closed == nil || closed.Window != OldNameWindow || !reflect.DeepEqual(closed.LastHolding, []string{"g14"}) || len(closed.Dropped) != 1 {
 			t.Errorf("closing = %+v", closed)
 		}
-		if _, again, _ := w.Step([]string{"g14"}, t0.Add(2*time.Hour)); again != nil {
-			t.Error("a closed window closed again")
+		// Once closed, it stays closed: a node holding it later opens nothing.
+		if again, shut, _ := w.Step(all, []string{"g14"}, t0.Add(2*time.Hour)); shut != nil || again.Open {
+			t.Errorf("a closed window stepped on a holder: %+v, closed %+v; want it closed, said once", again, shut)
 		}
 	})
 	t.Run("the windows' driver: every node read on each wake, closed once, a paused build agent's same build counted", func(t *testing.T) {
@@ -665,6 +671,17 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) {
 		if !reflect.DeepEqual(st.w[OldNameWindow].Holding, []string{"g14"}) || !reflect.DeepEqual(st.w[DirectWritesWindow].Holding, []string{"g14"}) {
 			t.Errorf("windows = %+v, want g14 holding both (away: states nothing; runs the old build)", st.w)
 		}
+		// The window's record says why g14 holds it: its manifest-reads timed out, counted as stating nothing.
+		if why := st.w[OldNameWindow].Why["g14"]; !strings.Contains(why, "did not answer") || !strings.Contains(why, "timeout") {
+			t.Errorf("why g14 holds the old name = %q, want the timeout said", why)
+		}
+		// Security review F3: a wake whose node_haves names no node closes neither window.
+		haves := del.haves
+		del.haves = nil
+		if _, err := d.Wake("delivered"); err != nil || !st.w[OldNameWindow].Open || !st.w[DirectWritesWindow].Open || len(closed) != 0 {
+			t.Fatalf("a wake on an empty node_haves: %v, windows %+v, closed %v; want both open, none said", err, st.w, closed)
+		}
+		del.haves = haves
 		now = now.Add(WindowFindingAfter)
 		_, _ = d.Wake("delivered")
 		if !found[FindingWindowHeld+" "+OldNameWindow+".g14"] {

modules/distribution/internal/holder/mirror_fakes_test.go

@@ -98,7 +98,10 @@ type fakeDelivery struct {
 func (f *fakeDelivery) Haves() ([]NodeHave, error) { return f.haves, nil }
 func (f *fakeDelivery) Reads(node string) (bool, []string, error) {
 	s, ok := f.stated[node]
-	return ok, s, nil
+	if !ok {
+		return false, nil, errors.New("nats: timeout")
+	}
+	return true, s, nil
 }
 
 type memWindows struct {

modules/distribution/internal/holder/window.go

@@ -2,6 +2,8 @@ package holder
 
 import (
 	"fmt"
+
+	"git.novox.be/novox/mesh-sdk/go/delivery"
 	"slices"
 	"sort"
 	"time"
@@ -27,16 +29,16 @@ const (
 )
 
 // Window is one window's record, kept in distribution's state windows: what registry_old_name and
-// registry_direct_writes answer.
+// registry_direct_writes answer. Its open, holding, since, closed and last holding are mesh-sdk's delivery.WindowHeld,
+// the part every window keeper shares and steps by one rule (delivery.StepWindow).
 type Window struct {
-	Name    string               `json:"name"`
-	Open    bool                 `json:"open"`
-	Holding []string             `json:"holding,omitempty"`
-	Since   map[string]time.Time `json:"since,omitempty"`
-	Dropped []DroppedNode        `json:"dropped,omitempty"`
-	Closed  time.Time            `json:"closed,omitzero"`
-	// LastHolding are the nodes that held it last, said when it closes.
-	LastHolding []string `json:"last_holding,omitempty"`
+	Name string `json:"name"`
+	delivery.WindowHeld
+	Dropped []DroppedNode `json:"dropped,omitempty"`
+	// Why says, of each node holding the window, why it holds it: what its read answered, a node-engine whose
+	// manifest-reads did not answer (an away node, an older node-engine, a timeout) included, so a timeout taken as
+	// stating nothing is told apart from a node-engine that states the old names.
+	Why map[string]string `json:"why,omitempty"`
 }
 
 // DroppedNode is one retired node dropped from a window by hand: a hand act.
@@ -57,16 +59,42 @@ type Closed struct {
 }
 
 // NodeRead is what distribution read of one node: the reader capabilities its node-engine states through
-// `manifest-reads` (Serves false while it does not serve the verb: it states nothing), whether its declaration names an
-// artifact, and its build modules' haves: whether each runs a build that publishes through the seats.
+// `manifest-reads` (Serves false while it does not serve the verb: it states nothing; NotServed then says what the
+// call answered), whether its declaration names an artifact, and its build modules' haves: whether each runs a build
+// that publishes through the seats.
 type NodeRead struct {
 	Node          string          `json:"node"`
 	Serves        bool            `json:"serves"`
+	NotServed     string          `json:"not_served,omitempty"`
 	Stated        []string        `json:"stated,omitempty"`
 	NamesArtifact bool            `json:"names_artifact"`
 	BuildModules  map[string]bool `json:"build_modules,omitempty"`
 }
 
+// WhyOldName says why a node holds the old name open.
+func WhyOldName(n NodeRead) string {
+	if !n.Serves {
+		if n.NotServed != "" {
+			return "its node-engine's manifest-reads did not answer, so it counts as stating nothing: " + n.NotServed
+		}
+		return "its node-engine does not serve manifest-reads, so it states nothing"
+	}
+	return "its node-engine does not state " + NewNamesCapability
+}
+
+// WhyDirectWrites says why a node holds the direct writes open: its build modules running a build not heard
+// publishing through the seats.
+func WhyDirectWrites(n NodeRead) string {
+	var old []string
+	for m, publishes := range n.BuildModules {
+		if !publishes {
+			old = append(old, m)
+		}
+	}
+	sort.Strings(old)
+	return fmt.Sprintf("its %v run a build not heard publishing through the seats", old)
+}
+
 // HoldingOldName are the nodes holding the old name open: a node whose declaration names an artifact and whose
 // node-engine does not state the new names (one not serving manifest-reads states nothing).
 func HoldingOldName(nodes []NodeRead, dropped []DroppedNode) []string {
@@ -106,33 +134,25 @@ func isDropped(dropped []DroppedNode, node string) bool {
 	return slices.ContainsFunc(dropped, func(d DroppedNode) bool { return d.Node == node })
 }
 
-// Step moves the window to the nodes holding it at now. It answers the next record, the closing event when it closed in
-// this step (said once), and the nodes holding it past WindowFindingAfter (the finding; cleared when it closes). A
-// window once closed stays closed: a node that holds it later is no reason to serve the old name again.
-func (w Window) Step(holding []string, now time.Time) (next Window, closed *Closed, overdue []string) {
-	next = Window{Name: w.Name, Dropped: w.Dropped, Closed: w.Closed, LastHolding: w.LastHolding}
+// Step moves the window by mesh-sdk's one rule (delivery.StepWindow): named is every node the read named, dropped ones
+// included, holding those holding the window at now. A read naming no node judges nothing. It answers the next record,
+// the closing event when it closed in this step (said once), and the nodes holding it past WindowFindingAfter (the
+// finding; cleared when it closes). A window once closed stays closed: a node that holds it later is no reason to serve
+// the old name again or to take direct writes again.
+func (w Window) Step(named, holding []string, now time.Time) (next Window, closed *Closed, overdue []string) {
+	next = w
 	if !w.Open && !w.Closed.IsZero() {
 		return next, nil, nil
 	}
-	if len(holding) == 0 {
-		next.Closed, next.LastHolding = now, w.Holding
-		if next.LastHolding == nil {
-			next.LastHolding = []string{}
-		}
-		return next, &Closed{Window: w.Name, At: now, LastHolding: next.LastHolding, Dropped: w.Dropped}, nil
+	held, shut, overdue := delivery.StepWindow(w.WindowHeld, named, holding, now, WindowFindingAfter)
+	next.WindowHeld = held
+	if !held.Open {
+		next.Why = nil
 	}
-	next.Open, next.Holding, next.Since = true, holding, map[string]time.Time{}
-	for _, n := range holding {
-		since, had := w.Since[n]
-		if !had {
-			since = now
-		}
-		next.Since[n] = since
-		if now.Sub(since) >= WindowFindingAfter {
-			overdue = append(overdue, n)
-		}
+	if !shut {
+		return next, nil, overdue
 	}
-	return next, nil, overdue
+	return next, &Closed{Window: w.Name, At: held.Closed, LastHolding: held.LastHolding, Dropped: w.Dropped}, nil
 }
 
 // Drop drops a retired node from the window by hand, a hand act with who and why.