From 150fadfe8bed4765cc83beeb9ed83f4acfc5f62d Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 01:26:33 +0000 Subject: [PATCH] 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 | 11 +++++++++++ internal/link/have.go | 16 ++++++++++++++++ internal/link/have_test.go | 21 +++++++++++++++++++++ internal/link/messages.go | 5 +++++ 4 files changed, 53 insertions(+) diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 7a49a5d..f8691fa 100644 --- a/cmd/mesh-host/main.go +++ b/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 diff --git a/internal/link/have.go b/internal/link/have.go index 0d59e19..6428a36 100644 --- a/internal/link/have.go +++ b/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{} diff --git a/internal/link/have_test.go b/internal/link/have_test.go index 516dee3..152e8aa 100644 --- a/internal/link/have_test.go +++ b/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) + } +} diff --git a/internal/link/messages.go b/internal/link/messages.go index b8ee481..406c75d 100644 --- a/internal/link/messages.go +++ b/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"