From e92a3fe237f5b787b5d112628427ef173f977fc4 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 00:09:24 +0200 Subject: [PATCH] Record a push that only moves recorded builds as the person's word, not a repair (hq issue 301) A recorded build moves only by a person's push (ADR 0242), so that push is the word its upgrade policy asks for; S15 counted it as a repair and wanted a healer for split-dns, words and uplink-verbs. The push now reads what it carries before it is recorded and says so in its kind, and the ten pushes of 2026-10-07 are named so their three warnings clear on the next tick. --- cmd/mesh-controller/handacts.go | 38 +++++- cmd/mesh-controller/recorded_push.go | 153 ++++++++++++++++++++++ cmd/mesh-controller/recorded_push_test.go | 124 ++++++++++++++++++ cmd/mesh-controller/replays_test.go | 61 +++++++++ internal/link/handacts.go | 10 ++ 5 files changed, 383 insertions(+), 3 deletions(-) create mode 100644 cmd/mesh-controller/recorded_push.go create mode 100644 cmd/mesh-controller/recorded_push_test.go diff --git a/cmd/mesh-controller/handacts.go b/cmd/mesh-controller/handacts.go index 7ab4ad5..9901927 100644 --- a/cmd/mesh-controller/handacts.go +++ b/cmd/mesh-controller/handacts.go @@ -36,6 +36,9 @@ type handActVerb struct { Decision string // DecidedFor limits Decision to these causes; empty, it holds for every act of the verb. DecidedFor []string + // DecidedWhen limits Decision to the acts it answers true for: what the controller read the act to + // be from what it did (a push's kind), never a word the person gave. + DecidedWhen func(link.HandAct) bool } // causeLeakedInLogs is the cause a rotation after a value was printed into a log gives. @@ -52,8 +55,10 @@ const causeDrill = "drill" // listed without a decision, counts, so a new verb is a repair until its entry says otherwise. var handActVerbs = []handActVerb{ // Repairs: each repeated is a healer the mesh lacks. A push by hand is exactly what roll-out by - // default (ADR 0236) exists to end. - {Verb: "push"}, + // default (ADR 0236) exists to end — except a push that only moved builds a `record` policy held for + // a person's word (ADR 0242), which the push itself reads from what it carried (recorded_push.go). + {Verb: "push", Decision: "a recorded build moves only by a person's push: that push is the word its " + + "upgrade policy asks for (ADR 0242)", DecidedWhen: pushedRecorded}, {Verb: "plans stop"}, {Verb: "plans close"}, // A walk started by a person instead of its delivery's owner (novox/hq ADR 0239): the owner down, or @@ -93,7 +98,8 @@ func personsDecision(a link.HandAct) bool { if v.Verb != a.Verb { continue } - return v.Decision != "" && (len(v.DecidedFor) == 0 || slices.Contains(v.DecidedFor, a.Cause)) + return v.Decision != "" && (len(v.DecidedFor) == 0 || slices.Contains(v.DecidedFor, a.Cause)) && + (v.DecidedWhen == nil || v.DecidedWhen(a)) } return false } @@ -153,6 +159,19 @@ func (f handActFlags) record(ctx context.Context, verb string, args []string) { } act := link.HandAct{Verb: verb, Args: args, Why: strings.TrimSpace(*f.why), Cause: strings.TrimSpace(*f.cause), Condition: strings.TrimSpace(*f.condition)} + // A push naming one machine says whether it only moves recorded builds (novox/hq issue 301): + // read from what it carries, before it is sent. + if verb == "push" && len(args) == 1 && !strings.HasPrefix(args[0], "-") { + switch carried, why, err := recordedPushOf(ctx, args[0]); { + case err != nil: + fmt.Fprintf(os.Stderr, "whether this push only moves recorded builds could not be read, so it is "+ + "recorded as a push by hand: %v\n", err) + case len(carried) > 0: + act.Kind, act.Carried = link.KindRecordedBuilds, carried + default: + fmt.Printf("a push by hand, not of recorded builds only: %s\n", why) + } + } err := onTheBus(func(conn *nats.Conn) error { written, err := link.RecordHandAct(ctx, conn, act) act = written @@ -162,6 +181,12 @@ func (f handActFlags) record(ctx context.Context, verb string, args []string) { fmt.Fprintf(os.Stderr, "this act by hand could NOT be recorded in the hand-act log, and is done anyway: %v\n", err) return } + if act.Kind == link.KindRecordedBuilds { + fmt.Printf("recorded as %s in the hand-act log: a push of recorded builds (%s) by %s, because %q "+ + "— the person's word their upgrade policy asks for, which no healer is wanted for\n", act.ID, + strings.Join(act.Carried, "; "), act.By, act.Why) + return + } fmt.Printf("recorded as %s in the hand-act log: %s, because %q (cause: %s)\n", act.ID, act.By, act.Why, act.Cause) } @@ -244,6 +269,13 @@ func handActCommand(ctx context.Context, args []string) error { fmt.Printf(", condition %s", a.Condition) } fmt.Println(")") + if pushedRecorded(a) { + carried := strings.Join(a.Carried, "; ") + if carried == "" { + carried = recordedBefore[a.ID] + } + fmt.Printf(" a push of recorded builds, no repair: %s\n", carried) + } } if len(repeated) > 0 { causes := make([]string, 0, len(repeated)) diff --git a/cmd/mesh-controller/recorded_push.go b/cmd/mesh-controller/recorded_push.go new file mode 100644 index 0000000..676dbd1 --- /dev/null +++ b/cmd/mesh-controller/recorded_push.go @@ -0,0 +1,153 @@ +package main + +import ( + "context" + "fmt" + "sort" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// A push of recorded builds is no repair (novox/hq issue 301, ADR 0242, to-be 45 §7). +// +// **A recorded build moves only by a person's push** (ADR 0242): a module whose upgrade policy is +// `record` — the network path, the providers whose restart costs, mail — has its new build registered +// at its merge and sent by nothing but `push `. That push is the person's word the policy asks +// for: the mesh working as decided. The hand-act log counted it as a repair all the same, because a push +// by hand is the repair roll-out by default exists to end. On 2026-10-07 the operator approved, node by +// node, new builds of the resolver, the FortiClient adapter, the network managers and the packet filter; +// ten pushes later S15 wanted a healer for `split-dns`, `words` and `uplink-verbs`. +// +// **The push says what it was, from what it carried.** Never from a word the person gives — that is +// how a repair could pass for anything (issue 292). Before it is recorded, a push that names one machine +// reads what that machine was last sent against what the mesh holds. It is a **push of recorded builds** +// when: +// +// - what the machine was last sent is known; +// - its last report is no failure or refusal, so the push does not re-send to mend one; +// - it moves at least one module's build; and +// - every module it moves records rather than rolls out: a held new build of one the machine runs, or +// the first build of one newly assigned there (whose `assign` says "run `push ` to send it"). +// +// Anything else — a build that rolls out, a module taken off, nothing moved at all — is a push by hand +// as before, and counts. The act is written with `kind: recorded-builds` and what it carried; the table +// of verbs (handActVerbs) reads a push of that kind as a person's decision, and S15 never counts it. +// +// What is not compared: settings and grants. A push of a recorded build carries whatever else +// changed in the machine's declaration with it, as any push does. + +// recordedPush answers what a push to one machine carries, one "module from → to" each, when every +// build it moves is a recorded one held for a person's word; otherwise nil and why it is not. +// +// `modules` is the machine's set; `sent` and `known` what it was last sent (Inventory.SentBuilds); +// `current` what the mesh holds; `identical` whether two builds of a module put the same thing on a +// machine (moveFacts.identical); `failing` the machine's last report when it was a failure or a refusal. +func recordedPush(modules []string, sent map[string]string, known bool, current map[string]inventory.CurrentBuild, + identical func(module, a, b string) bool, failing string) ([]string, string) { + if !known { + return nil, "what the machine was last sent is not known" + } + if failing != "" { + return nil, "its last report was " + failing + ": the push sends again what failed" + } + in := map[string]bool{} + var carried []string + for _, m := range modules { + in[m] = true + // A module the mesh holds no build of, or holds without a source, has no build to move. + now, held := current[m] + if !held || now.Commit == "" { + continue + } + was, ran := sent[m] + if ran && identical(m, was, now.Commit) { + continue + } + if now.RollOut { + return nil, fmt.Sprintf("%s would move %sto %s, and it rolls out: its build is a plan's to send", m, + fromBuild(was, ran), buildName(now.Commit)) + } + carried = append(carried, fmt.Sprintf("%s %s→ %s", m, fromBuild(was, ran), buildName(now.Commit))) + } + for m := range sent { + if !in[m] { + return nil, m + " is taken off the machine" + } + } + if len(carried) == 0 { + return nil, "it moves no build: a send again" + } + sort.Strings(carried) + return carried, "" +} + +// recordedPushOf reads, for a push about to name one machine, whether it is a push of recorded builds: what +// it carries, +// or nil and why not. An error is that it could not be read; the push is then recorded as a push by hand, +// as every push was before, and says why. +func recordedPushOf(ctx context.Context, node string) ([]string, string, error) { + open, err := openStores(ctx) + if err != nil { + return nil, "", err + } + defer open.Close() + inv := open.inventory + plan, _, err := planFor(ctx, open, node) + if err != nil { + return nil, "", fmt.Errorf("its set cannot be worked out: %w", err) + } + modules := make([]string, 0, len(plan.Modules)) + for _, m := range plan.Modules { + modules = append(modules, m.Module) + } + sent, known, err := inv.SentBuilds(ctx, node) + if err != nil { + return nil, "", err + } + f, err := readMoveFacts(ctx, inv) + if err != nil { + return nil, "", err + } + doing, said, err := inv.DoingOf(ctx, node) + if err != nil { + return nil, "", err + } + failing := "" + if said && (doing.Outcome == inventory.OutcomeFailed || doing.Outcome == inventory.OutcomeRefused) { + failing = doing.Outcome + } + carried, why := recordedPush(modules, sent, known, f.current, f.identical, failing) + return carried, why, nil +} + +// recordedBefore are the pushes recorded before a push said its kind, each a push of recorded builds by +// what it carried (novox/hq issue 301): the operator's word, machine by machine, for new builds of modules +// whose upgrade policy records — systemd-resolved, forticlient, networkmanager, systemd-networkd, +// nftables, mailu. The log did not keep what a push carried then, so the record names them; no push +// recorded since needs it, because every push now says its kind. S15 reads these as it reads a push of +// that kind, and the three `healer-wanted` they opened clear on the next tick. +var recordedBefore = map[string]string{ + "act-1791404241010309198-1": "systemd-resolved, first build on its machine (ADR 0247)", + "act-1791404587689907257-1": "systemd-resolved, first build; nftables, held build (catalogue #111)", + "act-1791404809925218992-1": "nftables, held build (catalogue #111)", + "act-1791404844504887009-1": "mailu and nftables, held builds (catalogue #106, #111)", + "act-1791408072745552019-1": "forticlient, held build (catalogue #114)", + "act-1791408101286318350-1": "forticlient, held build (catalogue #114)", + "act-1791409209593713522-1": "networkmanager, held build (catalogue #107)", + "act-1791409280125022264-1": "networkmanager, held build (catalogue #107)", + "act-1791409336586475835-1": "networkmanager, held build (catalogue #107)", + "act-1791409393494198352-1": "systemd-networkd, held build (catalogue #107)", +} + +// pushedRecorded is whether an act in the log is a push of recorded builds. +func pushedRecorded(a link.HandAct) bool { + if a.Verb != "push" { + return false + } + if a.Kind == link.KindRecordedBuilds { + return true + } + _, before := recordedBefore[a.ID] + return before && a.Kind == "" +} diff --git a/cmd/mesh-controller/recorded_push_test.go b/cmd/mesh-controller/recorded_push_test.go new file mode 100644 index 0000000..599cf43 --- /dev/null +++ b/cmd/mesh-controller/recorded_push_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// **A push says whether it pushed recorded builds only, from what it carries** (novox/hq issue 301): +// every module it moves records rather than rolls out, the machine's last send is known and its last +// report no failure. Anything else is a push by hand, as before. +func TestAPushIsOfRecordedBuildsOnlyWhenEveryBuildItMovesWasHeldForAPersonsWord(t *testing.T) { + current := map[string]inventory.CurrentBuild{ + "resolver": {Commit: "r2222222"}, "nftables": {Commit: "n2222222"}, "mail": {Commit: "m2222222"}, + "web": {Commit: "w2222222", RollOut: true}, "proxy": {Commit: "p1111111", RollOut: true}, + } + same := func(_, a, b string) bool { return a == b } + for _, c := range []struct { + name string + modules []string + sent map[string]string + known bool + failing string + carried string // what it carried, joined; empty when it is not a push of recorded builds + why string + }{ + {name: "one held build", modules: []string{"resolver", "proxy"}, + sent: map[string]string{"resolver": "r1111111", "proxy": "p1111111"}, known: true, + carried: "resolver from r1111111 → r2222222"}, + {name: "two held builds", modules: []string{"mail", "nftables", "proxy"}, + sent: map[string]string{"mail": "m1111111", "nftables": "n1111111", "proxy": "p1111111"}, known: true, + carried: "mail from m1111111 → m2222222; nftables from n1111111 → n2222222"}, + {name: "a recorded module's first build, newly assigned", modules: []string{"resolver", "proxy"}, + sent: map[string]string{"proxy": "p1111111"}, known: true, + carried: "resolver (never sent it) → r2222222"}, + {name: "a build that rolls out moves too", modules: []string{"resolver", "web"}, + sent: map[string]string{"resolver": "r1111111", "web": "w1111111"}, known: true, why: "rolls out"}, + {name: "nothing moves", modules: []string{"resolver", "proxy"}, + sent: map[string]string{"resolver": "r2222222", "proxy": "p1111111"}, known: true, why: "a send again"}, + {name: "a module taken off", modules: []string{"resolver"}, + sent: map[string]string{"resolver": "r1111111", "proxy": "p1111111"}, known: true, why: "taken off"}, + {name: "what it was last sent is not known", modules: []string{"resolver"}, why: "not known"}, + {name: "its last report failed", modules: []string{"resolver"}, + sent: map[string]string{"resolver": "r1111111"}, known: true, failing: "failed", why: "failed"}, + } { + carried, why := recordedPush(c.modules, c.sent, c.known, current, same, c.failing) + if got := strings.Join(carried, "; "); got != c.carried { + t.Errorf("%s: carried %q, want %q (%s)", c.name, got, c.carried, why) + } + if c.why != "" && !strings.Contains(why, c.why) { + t.Errorf("%s: not of recorded builds because %q, want it to say %q", c.name, why, c.why) + } + } +} + +// **The live case, as the log holds it** (novox/hq issue 301). The ten pushes of 2026-10-07 recorded +// before a push said its kind pushed recorded builds only; the three `healer-wanted` they opened clear. +// The five pushes for the settings beside them, and any push that says no kind, still count; and a push +// recorded as one of recorded builds since, repeated, never does. +func TestThePushesOfRecordedBuildsOf20261007WantNoHealer(t *testing.T) { + now := time.Date(2026, 10, 8, 9, 0, 0, 0, time.UTC) + pushed := func(id, cause string, at time.Time) link.HandAct { + return link.HandAct{ID: id, At: at, By: "operator", Verb: "push", Args: []string{"laptop"}, + Why: "operator's yes", Cause: cause} + } + var acts []link.HandAct + at := now.Add(-12 * time.Hour) + causes := map[string]string{ + "act-1791404241010309198-1": "split-dns", "act-1791404587689907257-1": "split-dns", + "act-1791408072745552019-1": "split-dns", "act-1791408101286318350-1": "split-dns", + "act-1791404809925218992-1": "words", "act-1791404844504887009-1": "words", + "act-1791409209593713522-1": "uplink-verbs", "act-1791409280125022264-1": "uplink-verbs", + "act-1791409336586475835-1": "uplink-verbs", "act-1791409393494198352-1": "uplink-verbs", + } + if len(causes) != len(recordedBefore) { + t.Fatalf("the live case has %d acts and the record names %d", len(causes), len(recordedBefore)) + } + for id, cause := range causes { + if _, named := recordedBefore[id]; !named { + t.Fatalf("%s is not named a push of recorded builds", id) + } + acts = append(acts, pushed(id, cause, at)) + at = at.Add(time.Minute) + } + for i := range 2 { + acts = append(acts, pushed("act-settings-"+string(rune('a'+i)), "push", at)) + at = at.Add(time.Minute) + } + f := calm(now) + f.handActs = acts + got := watchHandActs(f) + if len(got) != 1 || got[0].ID != "hand-acts.push" { + t.Fatalf("S15 saw %+v; want only the settings pushes, counted as before", got) + } + + // A push that says it carried recorded builds, repeated, wants no healer; one recorded with the same word and no + // kind still counts, as does an act of another verb whose ID such a push once had. + ofRecorded := func(at time.Time) link.HandAct { + a := pushed("", "split-dns", at) + a.Kind, a.Carried = link.KindRecordedBuilds, []string{"resolver from r1111111 → r2222222"} + return a + } + f = calm(now) + f.handActs = []link.HandAct{ofRecorded(now.Add(-26 * time.Hour)), ofRecorded(now.Add(-time.Hour)), + ofRecorded(now.Add(-time.Minute))} + if got := watchHandActs(f); len(got) != 0 { + t.Errorf("pushes of recorded builds repeated asked for a healer: %+v", got) + } + f = calm(now) + f.handActs = []link.HandAct{pushed("a", "split-dns", now.Add(-time.Hour)), pushed("b", "split-dns", now.Add(-time.Minute))} + if got := watchHandActs(f); len(got) != 1 { + t.Errorf("pushes that said no kind were not counted: %+v", got) + } + other := actOf(now.Add(-time.Hour), "plans close", "split-dns") + other.ID = "act-1791404241010309198-1" + f = calm(now) + f.handActs = []link.HandAct{other, actOf(now.Add(-time.Minute), "plans close", "split-dns")} + if got := watchHandActs(f); len(got) != 1 { + t.Errorf("another verb under such a push's ID passed for one: %+v", got) + } +} diff --git a/cmd/mesh-controller/replays_test.go b/cmd/mesh-controller/replays_test.go index 2ba09fd..b1e5f49 100644 --- a/cmd/mesh-controller/replays_test.go +++ b/cmd/mesh-controller/replays_test.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "flag" "fmt" "os" "slices" @@ -522,3 +523,63 @@ func TestReplay300AMergeAddingAModuleAsksForItsBuild(t *testing.T) { t.Fatalf("a merge moving nothing the mesh holds opened a plan: %+v %v", plans, err) } } + +// **R301 — a person's push of a held recorded build is no repair.** On 2026-10-07 the operator approved, +// machine by machine, new builds of modules whose upgrade policy records rather than rolls out — the +// resolver, the FortiClient adapter, the network managers, the packet filter, mail. A recorded build +// moves only by a person's push (ADR 0242), so each was `push ` through the seat with the +// operator's yes as its why and a word for the change as its cause. The hand-act log counted every one +// as a repair, and S15 raised `mesh.hand-acts.split-dns.healer-wanted` ("repaired by hand 4 times in 14 +// days … a healer is wanted"), and the same for `words` and `uplink-verbs`. The outcome asserted: a push +// that carries only held recorded builds, recorded as the push records it, wants no healer however +// often the person gives that word — through S15 itself, reading the log as the watchdog reads it. +func TestReplay301APersonsPushOfAHeldRecordedBuildIsNoRepair(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + start := time.Now().Add(-time.Hour) + image := "registry.invalid:5000/resolver/server@sha256:" + strings.Repeat("e", 64) + if _, _, err := takeIn(ctx, inv, aContainerBuild(t, "resolver", "c1111111", catalogue.PolicyRecord, start, + map[string][2]string{"server": {image, ""}})); err != nil { + t.Fatal(err) + } + for _, node := range []string{"anchor", "laptop"} { + if _, err := inv.Assign(ctx, node, "resolver"); err != nil { + t.Fatal(err) + } + if err := inv.RecordSent(ctx, nodeID(t, open, node), "d-"+node, map[string]string{"resolver": "c1111111"}); err != nil { + t.Fatal(err) + } + } + // The merge: the resolver's new build, registered and held for a person's word. + if _, _, err := takeIn(ctx, inv, aContainerBuild(t, "resolver", "c2222222", catalogue.PolicyRecord, + start.Add(time.Minute), map[string][2]string{"server": {image, "runtime"}})); err != nil { + t.Fatal(err) + } + + conn := onATestBus(t) + // The operator's yes, machine by machine, as the seat's push records it before it sends. + for _, node := range []string{"anchor", "laptop"} { + set := flag.NewFlagSet("push", flag.ContinueOnError) + why := addHandActFlags(set) + if err := set.Parse([]string{"--why", "operator's yes: the resolver's new build on " + node, + "--cause", "split-dns"}); err != nil { + t.Fatal(err) + } + why.record(ctx, "push", []string{node}) + } + acts, err := link.HandActs(ctx, conn, start) + if err != nil { + t.Fatal(err) + } + if len(acts) != 2 { + t.Fatalf("the two pushes were recorded as %d act(s): %+v", len(acts), acts) + } + f := calm(time.Now()) + f.handActs = acts + for _, o := range watchHandActs(f) { + if strings.Contains(o.ID, "split-dns") { + t.Fatalf("a person's push of a held recorded build was counted as a repair: %s — %s", o.ID, o.Summary) + } + } +} diff --git a/internal/link/handacts.go b/internal/link/handacts.go index ed437e7..e4e916c 100644 --- a/internal/link/handacts.go +++ b/internal/link/handacts.go @@ -43,8 +43,18 @@ type HandAct struct { Cause string `json:"cause"` // Condition is the condition's key the act addresses, when it names one. Condition string `json:"condition,omitempty"` + // Kind is what the controller read the act to be from what it did, never a word a person gives: + // KindRecordedBuilds for a push that carried only builds a `record` policy held back for a person's + // word (novox/hq ADR 0242), empty for everything else. Absent from entries written before it existed. + Kind string `json:"kind,omitempty"` + // Carried is what such a push moved, one "module from → to" per module, so the log says it. + Carried []string `json:"carried,omitempty"` } +// KindRecordedBuilds is a push that only moved recorded builds: the person's word their `record` policy +// asks for, which is no repair (novox/hq ADR 0242, to-be 45 §7). +const KindRecordedBuilds = "recorded-builds" + // CallerVar carries a seat call's caller to the command the controller runs for it, so an act done // through the console says who asked rather than "the controller". const CallerVar = "MESH_CALLER"