Write a removal after a restart from the declaration the apply replaced (issue 1369)

The set of modules this process wrote lives in memory, so a node-engine restarted before the apply that drops a module never wrote that module's removal. The report now names the modules the kept declaration held before, and they are removal candidates.

cmd/mesh-host/main.go

@@ -2314,7 +2314,17 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto
 	}
 	// Kept whichever way it went, so a node that is disconnected next minute still knows what it
 	// was told. Saved after applying rather than before: what is kept is what this node acted on.
+	var before []string
 	if signed != nil {
+		// The modules the declaration kept until now held, so a have key written for one the new declaration no longer
+		// holds is written removed even by a node-engine that started since (issue 1369).
+		if kept, keptErr := store.ReadDeclared(store.DeclaredPath(opts.state)); keptErr == nil {
+			if old, parseErr := declaration.ParseTrusted(kept.Declaration); parseErr == nil {
+				for _, f := range moduleFacts(old, apply.Report{}, nil) {
+					before = append(before, f.Module)
+				}
+			}
+		}
 		if err := store.SaveDeclared(store.DeclaredPath(opts.state), *signed); err != nil {
 			report.Failed = map[string]string{"keeping the declaration": err.Error()}
 		}
@@ -2326,6 +2336,7 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto
 	// And how each module of the declaration came out, for the have keys written beside this report (novox/hq ADR 0429
 	// §4, issue 1132): not on the wire.
 	report.Modules = moduleFacts(declared, outcome, applyErr)
+	report.Before = before
 	// Which resources did not apply, for whether a reconcile's account is news (issue 233): not said on the wire.
 	report.Unapplied = unapplied(declared, outcome, applyErr)
 	return report

internal/link/have.go

@@ -108,7 +108,23 @@ func (q *Queue) noteApply(r Report) bool {
 	for _, m := range r.Modules {
 		next[m.Module] = m
 	}
+	// Candidates: what this process wrote, what the declaration kept before this one held (a node-engine that started
+	// since wrote nothing yet), and what the last apply here held. writeRemoved creates no missing key.
+	gone := map[string]string{}
 	for module, build := range s.written {
+		gone[module] = build
+	}
+	for _, module := range r.Before {
+		if _, known := gone[module]; !known {
+			gone[module] = ""
+		}
+	}
+	for module, f := range s.modules {
+		if _, known := gone[module]; !known {
+			gone[module] = f.Build
+		}
+	}
+	for module, build := range gone {
 		if _, held := next[module]; !held {
 			if s.removed == nil {
 				s.removed = map[string]string{}

internal/link/have_test.go

@@ -427,3 +427,24 @@ func TestARefusedRemovalIsWrittenByTheNextPass(t *testing.T) {
 		t.Errorf("b: %+v", b)
 	}
 }
+
+// 1369/F1: a node-engine that started since wrote nothing, and the report names the modules the kept declaration held
+// before: one of them the new declaration lacks is written removed, one that is not in the bucket is not created.
+func TestARestartedNodeEngineWritesTheRemovalFromTheReportsBefore(t *testing.T) {
+	l := newHaveLink()
+	l.put(t, "n1.a", delivery.Have{V: delivery.Version, Build: "b-1", Phase: delivery.PhaseRunning, Health: delivery.HealthOK})
+	l.put(t, "n1.b", delivery.Have{V: delivery.Version, Build: "b-2", Phase: delivery.PhaseRunning, Health: delivery.HealthOK})
+	q, _ := aHaveQueue(l) // fresh: nothing written by this process
+	r := onlyA()
+	r.Before = []string{"a", "b", "c"}
+	q.tell(context.Background(), r, applied)
+	if b := l.have(t, "n1.b"); b.Phase != delivery.PhaseRemoved {
+		t.Errorf("b: %+v", b)
+	}
+	if _, there := l.kv["n1.c"]; there {
+		t.Error("c, which has no key, was created")
+	}
+	if l.kv["n1.a"].rev != 1 {
+		t.Errorf("a was written, revision %d", l.kv["n1.a"].rev)
+	}
+}

internal/link/messages.go

@@ -188,6 +188,11 @@ type Report struct {
 	// have. Nil on a report that is not an apply's.
 	Modules []ModuleFact `json:"-"`
 
+	// Before is the names of the modules the declaration kept before this apply held, taken before it was replaced
+	// (issue 1369): a module of it that Modules lacks has left the node, whether or not this process wrote its have key.
+	// **Not on the wire.**
+	Before []string `json:"-"`
+
 	// Host is the version of the host that produced this report (novox/hq ADR 0141).
 	//
 	// Without it nothing can say a machine is behind, so "every machine current with its source"