Review round 1 of the bus want: a removal keeps the bus server, the have says a server made anew runs the want's fingerprint, the old build is read before the first write, and an unanswered inspect fails the apply rather than replacing the server (ADR 0594 §2, §4)

internal/agent/agent.go

@@ -77,8 +77,9 @@ type Haves interface {
 	Read(ctx context.Context, module string) (Have, bool)
 	// Write writes a module's have where it differs from the bucket's.
 	Write(ctx context.Context, h Have)
-	// Removed writes the module's have with the phase removed, once, if a key of it exists.
-	Removed(ctx context.Context, module string)
+	// Removed writes the module's have with the phase removed, once, if a key of it exists; words, when given, are its
+	// detail.
+	Removed(ctx context.Context, module string, words ...string)
 }
 
 // Result is how one apply or removal came out.
@@ -88,6 +89,8 @@ type Result struct {
 	// JetStream API error while the bus reloads), not the module's: the apply is tried again without a new revision.
 	Transient bool
 	Detail    string
+	// Kept is the resources of the keep set the apply left as they ran, by id (apply.Report.Kept, novox/hq ADR 0594 §4).
+	Kept []string
 }
 
 // Backup is the answer of the backup asked before a module moves.
@@ -117,8 +120,9 @@ type Env struct {
 	// resources (catalogue.BusServerOf, novox/hq ADR 0594 §7), empty for any other module; known says whether the
 	// module's manifest could be read. Nil names no bus module.
 	BusServer func(ctx context.Context, module string) (ids []string, known bool)
-	// Remove removes the module's own resources and nothing of another's.
-	Remove func(ctx context.Context, module string, others []delivery.Composed) Result
+	// Remove removes the module's own resources and nothing of another's, and leaves those of keep as they run (the bus
+	// server's, ADR 0594 §2).
+	Remove func(ctx context.Context, module string, others []delivery.Composed, keep map[string]bool) Result
 	// Backup backs a module up before a new composition of it is applied; nil backs up nothing.
 	Backup func(ctx context.Context, module string) Backup
 	// Kept is the resources the node last applied, by module, from the declaration it kept before the switch (ADR 0539
@@ -612,6 +616,10 @@ func (a *Agent) writeHave(ctx context.Context, h Have) {
 
 // serverOf is what the module's have says the bus server runs (Have.Server): kept since this agent read or wrote it,
 // else read from the have once. Empty for every module but the bus.
+//
+// A have written before this node agent kept the server names none: the server then runs what that have says the module
+// runs, its fingerprint, else its old-form build (the take-over of a server the node's own composition made). Read so on
+// the first read, before this agent writes the have, so every have it writes carries it.
 func (a *Agent) serverOf(ctx context.Context, module string) string {
 	a.mu.Lock()
 	s, known := a.server[module]
@@ -619,8 +627,17 @@ func (a *Agent) serverOf(ctx context.Context, module string) string {
 	if known {
 		return s
 	}
+	if server, _ := a.busKeep(ctx, module); len(server) == 0 {
+		return "" // not the bus, or not known to be yet: nothing kept, so a later read asks again
+	}
 	if h, ok := a.env.Haves.Read(ctx, module); ok {
 		s = h.Server
+		if s == "" {
+			s = h.FP
+		}
+		if s == "" {
+			s = h.Build
+		}
 	}
 	a.mu.Lock()
 	if _, written := a.server[module]; !written {
@@ -1496,14 +1513,6 @@ func (a *Agent) apply(ctx context.Context, module string, w wantNow, comp compNo
 	ranServer, madeServer := "", ""
 	if len(keep) > 0 {
 		ranServer = a.serverOf(ctx, module)
-		if ranServer == "" {
-			if h, ok := a.env.Haves.Read(ctx, module); ok {
-				ranServer = h.FP
-				if ranServer == "" {
-					ranServer = h.Build
-				}
-			}
-		}
 		if ranServer == "" {
 			ranServer = fp // a bus module this node never ran: its server is made at this fingerprint
 		}
@@ -1606,6 +1615,10 @@ func (a *Agent) apply(ctx context.Context, module string, w wantNow, comp compNo
 	if a.env.Settled != nil {
 		a.env.Settled(module)
 	}
+	if len(keep) > 0 && len(res.Kept) == 0 {
+		// Nothing of the server was there to keep: the apply made it at this fingerprint (ADR 0594 §4).
+		madeServer = fp
+	}
 	a.writeHave(ctx, Have{Module: module, FP: fp, Composed: dig, Phase: string(delivery.PhaseRunning), Detail: res.Detail,
 		Server: madeServer})
 }
@@ -1624,7 +1637,16 @@ func (a *Agent) remove(ctx context.Context, module string, w wantNow) {
 	}
 	others := a.heldOut(module)
 	a.mu.Unlock()
-	res := a.env.Remove(ctx, module, others)
+	// A removed want of the bus module never removes its server (novox/hq ADR 0594 §2): a removal carries no warrant, and
+	// removing the server is not built. One whose manifest cannot be read keeps what would be the server's, said.
+	keep, known := a.busKeep(ctx, module)
+	if !known {
+		keep = map[string]bool{}
+		for _, id := range catalogue.BusServer {
+			keep[module+"."+id] = true
+		}
+	}
+	res := a.env.Remove(ctx, module, others, keep)
 	a.mu.Lock()
 	if res.Failed {
 		a.attempt[module] = attempt{FP: "removed", WantRev: w.Rev}
@@ -1644,6 +1666,16 @@ func (a *Agent) remove(ctx context.Context, module string, w wantNow) {
 	if a.env.Settled != nil {
 		a.env.Settled(module)
 	}
+	if len(res.Kept) > 0 {
+		words := fmt.Sprintf("removed, but for %s: the bus server stays as it runs until a want carrying the operator's "+
+			"warrant removes it (novox/hq ADR 0594 §2)", strings.Join(res.Kept, ", "))
+		if !known {
+			words += "; its manifest could not be read, so what would be its server was left"
+		}
+		a.env.Say(module + " is " + words)
+		a.env.Haves.Removed(ctx, module, words)
+		return
+	}
 	a.env.Haves.Removed(ctx, module)
 }
 

internal/agent/agent_test.go

@@ -65,10 +65,11 @@ func (f *feed) put(key string, value []byte) {
 }
 
 type haves struct {
-	mu      sync.Mutex
-	stored  map[string]Have
-	log     []Have
-	removed []string
+	mu           sync.Mutex
+	stored       map[string]Have
+	log          []Have
+	removed      []string
+	removedWords []string
 	// unreadable is each module whose have the bus cannot read now (ReadErr).
 	unreadable map[string]bool
 }
@@ -99,10 +100,11 @@ func (h *haves) Write(_ context.Context, x Have) {
 	h.stored[x.Module] = x
 	h.log = append(h.log, x)
 }
-func (h *haves) Removed(_ context.Context, m string) {
+func (h *haves) Removed(_ context.Context, m string, words ...string) {
 	h.mu.Lock()
 	defer h.mu.Unlock()
 	h.removed = append(h.removed, m)
+	h.removedWords = append(h.removedWords, strings.Join(words, ""))
 }
 func (h *haves) last(module string) Have {
 	h.mu.Lock()
@@ -127,7 +129,9 @@ type bed struct {
 	kept      map[string][]json.RawMessage
 	others    [][]delivery.Composed
 	keeps     []map[string]bool // what each apply was told to keep as it runs (ADR 0594 §2)
-	said      []string
+	// serverGone is a bus server not on the machine: an apply keeping it keeps nothing and makes it.
+	serverGone bool
+	said       []string
 	// settled is each module Settled was told, and settledRan whether what runs held it then.
 	settled    []string
 	settledRan []bool
@@ -143,6 +147,13 @@ func newBed(t *testing.T) *bed {
 			defer b.mu.Unlock()
 			b.others = append(b.others, others)
 			b.keeps = append(b.keeps, keep)
+			var kept []string // what of keep was there to keep: all of it, unless the test took the server away
+			for id := range keep {
+				if !b.serverGone {
+					kept = append(kept, id)
+				}
+			}
+			slices.Sort(kept)
 			name := c.Module
 			if name == "" {
 				name = NodeModule
@@ -151,13 +162,19 @@ func newBed(t *testing.T) *bed {
 			if b.failWith != "" && (b.failTimes == 0 || len(b.applied) <= b.failTimes) {
 				return Result{Failed: true, Transient: b.transient, Detail: b.failWith}
 			}
-			return Result{}
+			return Result{Kept: kept}
 		},
-		Remove: func(_ context.Context, m string, _ []delivery.Composed) Result {
+		Remove: func(_ context.Context, m string, _ []delivery.Composed, keep map[string]bool) Result {
 			b.mu.Lock()
 			defer b.mu.Unlock()
 			b.removed = append(b.removed, m)
-			return Result{}
+			b.keeps = append(b.keeps, keep)
+			var kept []string
+			for id := range keep {
+				kept = append(kept, id)
+			}
+			slices.Sort(kept)
+			return Result{Kept: kept}
 		},
 		Backup: func(_ context.Context, m string) Backup {
 			b.mu.Lock()
@@ -1582,8 +1599,22 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes(
 			return b.keeps[len(b.keeps)-1]
 		}
 		fpA, fpB := fpOf("a"), fpOf("b")
+		// A have written before the first apply (the node cannot compose yet) keeps the old build's name as what the
+		// server runs: the writer names no build, so it is read before the first write.
+		want("nats", fpA, "")
+		eventually(t, "the bus's want was not taken", func() bool {
+			b.ag.mu.Lock()
+			defer b.ag.mu.Unlock()
+			return b.ag.wants["nats"].Present
+		})
+		b.ag.NotComposed(context.Background(), "a stand-in for the bus's secret")
+		if h := b.haves.last("nats"); h.Build != "" || h.Server != took {
+			t.Fatalf("a have written before the first apply: %+v", h)
+		}
+		b.ag.NotComposed(context.Background(), "")
 		for i, row := range []struct {
 			name, fp, image, users, warrant string
+			gone                            bool // the server is not on the machine: made, at the want's fingerprint
 			applies                         bool
 			keep                            map[string]bool
 			server                          string
@@ -1595,7 +1626,12 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes(
 				applies: true, server: fpB},
 			{name: "the same warrant again", fp: fpB, image: "2", users: "u2", warrant: delivery.WarrantOperator, server: fpB},
 			{name: "a build after the warrant", fp: fpA, image: "3", users: "u2", applies: true, keep: server, server: fpB},
+			{name: "a build whose server is not there", fp: fpOf("e"), image: "4", users: "u2", gone: true, applies: true,
+				keep: server, server: fpOf("e")},
 		} {
+			b.mu.Lock()
+			b.serverGone = row.gone
+			b.mu.Unlock()
 			before := len(b.appliedList())
 			key, raw, dig := bus(row.fp, row.image, row.users)
 			b.comps.put(key, raw)
@@ -1645,9 +1681,28 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes(
 			}
 		}
 		time.Sleep(100 * time.Millisecond)
-		if got := b.appliedList(); len(got) != 7 {
+		if got := b.appliedList(); len(got) != 8 {
 			t.Errorf("applied %v, not once for each change", got)
 		}
+		// A removed want removes the rest of the bus and never its server: it is kept, said in the journal and in the
+		// removed have's words.
+		rkey, _ := delivery.WantKey(node, "nats")
+		rraw, _ := delivery.EncodeWant(delivery.Want{Removed: true})
+		b.wants.put(rkey, rraw)
+		eventually(t, "the bus's removal was not asked", func() bool {
+			b.haves.mu.Lock()
+			defer b.haves.mu.Unlock()
+			return slices.Contains(b.haves.removed, "nats")
+		})
+		if got := lastKeep(); !maps.Equal(got, server) {
+			t.Errorf("the bus's removal kept %v, not its server %v", got, server)
+		}
+		b.haves.mu.Lock()
+		words := b.haves.removedWords[len(b.haves.removedWords)-1]
+		b.haves.mu.Unlock()
+		if !strings.Contains(words, "the bus server stays") || !strings.Contains(words, "nats.server") {
+			t.Errorf("the removed have does not say the server stays: %q", words)
+		}
 	})
 }