A settings gate keeps the module's build, and a walk that cannot keep a build fails (issue 641 review)

Without a build on the gate, a merge walk sending the machine a new build of
the module was read as the settings send's own report. A refusal for a last
send not known, or a kept build gone, never clears, so retrying it every
tick held the walk of waiting builds; it now fails the walk with why. Every
failure names the machines passed over as away (ADR 0344 §6).

cmd/mesh-controller/recorded_kept.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 
@@ -93,6 +94,11 @@ func keepingBuildsForSettings(ctx context.Context, node string) context.Context
 	return context.WithValue(ctx, keepForSettingsKey{}, node)
 }
 
+// errSettingsCannotKeep is a settings walk's send refused because a build of its machine cannot be kept: what the
+// machine was last sent is not known, or a build it runs is gone from the records. It does not clear by itself, so
+// the walk fails on it rather than trying again (ADR 0344).
+var errSettingsCannotKeep = errors.New("the settings cannot be sent with every build kept")
+
 // settingsKeptOn is whether this context is a settings walk's send to this machine.
 func settingsKeptOn(ctx context.Context, node string) bool {
 	on, _ := ctx.Value(keepForSettingsKey{}).(string)
@@ -123,8 +129,7 @@ func recordedKept(ctx context.Context, open *stores, node string) (map[string]st
 	}
 	if !known {
 		if settingsKeptOn(ctx, node) {
-			return nil, fmt.Errorf("what %s was last sent is not known, so its settings cannot be sent there with "+
-				"every build kept (novox/hq ADR 0344)", node)
+			return nil, fmt.Errorf("%w: what %s was last sent is not known (novox/hq ADR 0344)", errSettingsCannotKeep, node)
 		}
 		if every {
 			return nil, fmt.Errorf("what %s was last sent is not known, so the bus's user list cannot be sent "+
@@ -187,8 +192,8 @@ func keepRecorded(ctx context.Context, open *stores, node string, shelf map[stri
 			return nil, err
 		}
 		if !found && settingsKeptOn(ctx, node) {
-			return nil, fmt.Errorf("%s runs %s's build %s, which the build records no longer hold: its settings cannot "+
-				"be sent there with it kept (novox/hq ADR 0344)", node, m, short(kept[m]))
+			return nil, fmt.Errorf("%w: %s runs %s's build %s, which the build records no longer hold (novox/hq ADR 0344)",
+				errSettingsCannotKeep, node, m, short(kept[m]))
 		}
 		if !found && everyBuildKept(ctx, node) {
 			return nil, fmt.Errorf("%s runs %s's build %s, which the build records no longer hold: the bus's user "+

cmd/mesh-controller/settings_walk.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"slices"
 	"sort"
@@ -179,26 +180,37 @@ func advanceSettingsWalk(ctx context.Context, open *stores, p *inventory.Plan) (
 			}
 			// **A running module's data that would move is a person's word** (ADR 0217 §3): the walk stops before
 			// that machine, failed and said, and `push --move` sends it.
-			if held, err := settingsDataMoves(ctx, open, node); err != nil {
+			held, err := settingsDataMoves(ctx, open, node)
+			if errors.Is(err, errSettingsCannotKeep) {
+				return failBefore(p, node, err.Error())
+			}
+			if err != nil {
 				return false, fmt.Errorf("whether %s's data would move could not be read: %w", node, err)
-			} else if len(held) > 0 {
+			}
+			if len(held) > 0 {
 				var said strings.Builder
 				sayDataHeld(&said, node, held)
-				p.State = inventory.PlanFailed
-				p.Note = fmt.Sprintf("%s not sent to %s: a running module's data would move there — %s; the layer "+
-					"stays as the operator approved it", settingsWalkFor(*s), node, strings.Join(strings.Fields(said.String()), " "))
-				if rest := r.Order[r.Next+1:]; len(rest) > 0 {
-					p.Note += "; not sent: " + strings.Join(rest, ", ")
-				}
-				fmt.Printf("%s: %s\n", p.ID, p.Note)
-				return true, nil
+				return failBefore(p, node, "a running module's data would move there — "+
+					strings.Join(strings.Fields(said.String()), " "))
 			}
 			sent, grants, err := sendJudged(keepingBuildsForSettings(ctx, node), open, node)
+			if errors.Is(err, errSettingsCannotKeep) {
+				// It does not clear by itself: tried again on every tick, the walk would stay open and hold the walk
+				// of waiting builds until a person came.
+				return failBefore(p, node, err.Error())
+			}
 			if err != nil {
 				return false, fmt.Errorf("sending %s %s: %w", node, settingsWalkFor(*s), err)
 			}
+			// **The module's build is the one the gate judges** (issue 641 review): kept by this send, it is what a
+			// crossing send — a merge's walk sending the machine a new build of the module — supersedes, so this gate
+			// never reads that send's report as its own.
+			kept := ""
+			if builds, known, err := open.inventory.SentBuilds(ctx, node); err == nil && known {
+				kept = builds[s.Module]
+			}
 			r.Gate = &inventory.PlanGate{Component: coreComponent(s.Module), Machines: sent, Since: &now,
-				Sent: sentNow(ctx, open.inventory, sent), Grants: grants}
+				From: kept, To: kept, Sent: sentNow(ctx, open.inventory, sent), Grants: grants}
 			p.Note = fmt.Sprintf("sent %s %s (ask %s), every build there kept; judging %s there", node,
 				settingsWalkFor(*s), s.Ask, s.Module)
 			if said := grantsSaid(grants); said != "" {
@@ -243,10 +255,30 @@ func advanceSettingsWalk(ctx context.Context, open *stores, p *inventory.Plan) (
 	}
 	p.State = inventory.PlanFailed
 	p.Note = fmt.Sprintf("%s failed its gate on %s: %s — the layer stays as the operator approved it, and the one it "+
-		"replaced is in `settings %s` history", settingsWalkFor(*s), node, g.Why, s.Module)
-	if rest := r.Order[r.Next+1:]; len(rest) > 0 {
-		p.Note += "; not sent: " + strings.Join(rest, ", ")
-	}
+		"replaced is in `settings %s` history", settingsWalkFor(*s), node, g.Why, s.Module) + unreached(r)
 	fmt.Printf("%s: %s\n", p.ID, p.Note)
 	return true, nil
 }
+
+// failBefore ends a settings walk failed before it sent a machine, saying why, and that the layer stays.
+func failBefore(p *inventory.Plan, node, why string) (bool, error) {
+	r, s := p.Release, p.Release.Settings
+	p.State = inventory.PlanFailed
+	p.Note = fmt.Sprintf("%s not sent to %s: %s; the layer stays as the operator approved it", settingsWalkFor(*s),
+		node, why) + unreached(r)
+	fmt.Printf("%s: %s\n", p.ID, p.Note)
+	return true, nil
+}
+
+// unreached is what a stopped settings walk left without the layer, in words: the machines after the one it stopped
+// at, and those passed over as away (ADR 0344 §6). Empty when there are none.
+func unreached(r *inventory.PlanRelease) string {
+	var out string
+	if r.Next+1 < len(r.Order) {
+		out += "; not sent: " + strings.Join(r.Order[r.Next+1:], ", ")
+	}
+	if len(r.Skipped) > 0 {
+		out += "; not heard from, left as they were: " + strings.Join(r.Skipped, ", ")
+	}
+	return out
+}

cmd/mesh-controller/settings_walk_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -474,3 +475,91 @@ func TestASettingsWalkStopsBeforeADataMoveAndPassesOverAMachineAway(t *testing.T
 		t.Fatalf("a walk that reached nobody is %s: %s", p.State, p.Note)
 	}
 }
+
+// The gate keeps the module's build its send kept, so a crossing send — a merge's walk sending the machine a new
+// build of the module while the settings gate judges there — supersedes it, and its report is never read as the
+// settings send's (issue 641 review, finding 1).
+func TestACrossingSendSupersedesASettingsGate(t *testing.T) {
+	judgeLeavesAsTheCore(t)
+	w := aSettledMesh(t)
+	ctx := t.Context()
+	inv := w.open.inventory
+	if _, err := openSettingsWalk(ctx, w.open, inventory.PlanSettings{Module: "app", Ask: "s7"}); err != nil {
+		t.Fatal(err)
+	}
+	advancePlans(ctx, w.open)
+	p := w.walk(t)
+	if g := p.Release.Gate; g == nil || g.To != "c1" || g.From != "c1" {
+		t.Fatalf("the settings gate judges no build: %+v", p.Release.Gate)
+	}
+	// Another walk sends anchor app's c2 while the settings gate judges there.
+	now := time.Now().Add(-time.Minute)
+	manifest, _ := json.Marshal(catalogue.Manifest{Module: "app", Version: "1"})
+	if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-app-c2", Module: "app", Commit: "c2",
+		Repository: "novox/mesh-catalog", Path: "modules/app", Manifest: manifest, Asked: now, At: now,
+		Made: []inventory.Artifact{{Name: "x", Kind: "bundle", Reference: "sha256:app-2"}}}); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.RecordSent(ctx, nodeID(t, w.open, "anchor"), "d-merge", map[string]string{"app": "c2"}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inv.RecordDoing(ctx, nodeID(t, w.open, "anchor"), inventory.Doing{Node: "anchor",
+		Outcome: inventory.OutcomeApplied, Declared: "d-merge", Applied: 1, At: time.Now()}); err != nil {
+		t.Fatal(err)
+	}
+	gateEvery = 0
+	advancePlans(ctx, w.open)
+	p = w.walk(t)
+	if p.State != inventory.PlanSuperseded || p.Release.Gate == nil || p.Release.Gate.Verdict != inventory.GateSuperseded ||
+		!strings.Contains(p.Note, "superseded on anchor") || len(p.Release.Done) != 0 {
+		t.Fatalf("the settings walk after a crossing send is %s: %s %+v", p.State, p.Note, p.Release)
+	}
+	if !reflect.DeepEqual(w.sent, [][]string{{"anchor"}}) {
+		t.Fatalf("sent %v: laptop was sent after a superseded judging", w.sent)
+	}
+}
+
+// A send refusal that cannot clear by itself — what the machine was last sent is not known, or a build it runs is
+// gone — fails the walk with why, rather than keeping it open and holding the walk of waiting builds; and a
+// failure names the machines passed over as away (issue 641 review, findings 2 and 3).
+func TestASettingsWalkFailsOnARefusalThatCannotClear(t *testing.T) {
+	w := aSettledMesh(t)
+	ctx := t.Context()
+	inv := w.open.inventory
+	// What a send to a machine never sent can keep: nothing, and said as the refusal the walk fails on.
+	if _, err := inv.AddNode(ctx, "fresh"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := recordedKept(keepingBuildsForSettings(ctx, "fresh"), w.open, "fresh"); !errors.Is(err, errSettingsCannotKeep) {
+		t.Fatalf("a settings send to a machine never sent: %v", err)
+	}
+	w.heard = map[string]bool{"laptop": true}
+	settingsDataMoves = func(_ context.Context, _ *stores, node string) ([]dataMove, error) {
+		return nil, fmt.Errorf("%w: what %s was last sent is not known", errSettingsCannotKeep, node)
+	}
+	if _, err := openSettingsWalk(ctx, w.open, inventory.PlanSettings{Module: "app", Ask: "s8"}); err != nil {
+		t.Fatal(err)
+	}
+	advancePlans(ctx, w.open)
+	p := w.walk(t)
+	if p.State != inventory.PlanFailed || len(w.sent) != 0 ||
+		!strings.Contains(p.Note, "not sent to laptop: the settings cannot be sent with every build kept: what laptop was last sent is not known") ||
+		!strings.Contains(p.Note, "not heard from, left as they were: anchor") {
+		t.Fatalf("the walk is %s: %s", p.State, p.Note)
+	}
+	// It is not open, so a build waiting for a gate is walked.
+	now := time.Now().Add(-time.Minute)
+	manifest, _ := json.Marshal(catalogue.Manifest{Module: "app", Version: "1"})
+	if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-app-c2", Module: "app", Commit: "c2",
+		Repository: "novox/mesh-catalog", Path: "modules/app", Manifest: manifest, Asked: now, At: now,
+		Made: []inventory.Artifact{{Name: "x", Kind: "bundle", Reference: "sha256:app-2"}}}); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "app", Version: "1"}, inventory.Source{
+		Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/app", BuiltFrom: "c2", Head: "c2", Asked: now}); err != nil {
+		t.Fatal(err)
+	}
+	if r, err := releaseBacklog(ctx, w.open, ""); err != nil || r == nil {
+		t.Fatalf("the waiting build is held by the failed settings walk: %+v %v", r, err)
+	}
+}