diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 1627f89f..360746eb 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -1327,6 +1327,25 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( h.keepGroup(g) }) } + // A superseded member of a closed group: whether a later delivered walk on its trunk carried every module it + // moves, ADR 0270's covered test, kept on it as CoveredBy. Its own state stays superseded — a final state never + // moves — and its group counts it as reached (novox/hq issue 442). + if g.Closed { + for _, d := range members { + if d.State != Superseded || d.CoveredBy != "" { + continue + } + var ask []string + if d.CoveredBy, ask = h.coverOf(d); d.CoveredBy != "" { + h.keep(d) + } + for _, in := range ask { + if a := h.askContains(d, in, now); a != nil { + asks = append(asks, a) + } + } + } + } // Its state, said when it changes. if state, why := GroupState(g, members); state != g.Said { g.Said = state @@ -1398,6 +1417,10 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { } switch { case d.State == Failed || d.State == Stopped || d.State == Superseded: + // A superseded member is an end here, and it stops the members after it even when a later delivered + // walk carried it. GroupState counts such a member as reached (novox/hq issue 442), so for a carried + // member the two do not agree: the members after it are stopped all the same, and the group then reads + // stopped for them. That gap is novox/hq issue 445, not closed here. for _, later := range ordered[i+1:] { if a := h.stopInGroup(byID[later.ID], fmt.Sprintf("its group %s stopped: %s %s", g.ID, d.ID, d.State)); a != nil { asks = append(asks, a) @@ -1467,11 +1490,32 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() { } } -// GroupState is a group's state, derived from its members and its composed check — never set apart. +// GroupState is a group's state, derived from its members and its composed check — never set apart (novox/hq +// ADR 0239 decision 4, which lists checking, rejected, ready, delivering, delivered, failed and stopped). +// +// It extends that list for a superseded member, which ADR 0239 does not name (novox/hq issue 442: five closed +// groups read "delivering, 1 of 2 delivered" for days with nothing in progress). A superseded member is an end. +// It counts as reached when a later delivered walk on its trunk carried every module it moves (CoveredBy, ADR +// 0270's covered test); a closed group whose members all ended is then delivered, and otherwise stopped, naming +// the member. A group with nothing in progress never reads delivering. +// +// groupTurns does not yet agree for a carried member: it stops the members after any superseded one, carried or +// not, and a stopped member makes the group read stopped here. That gap is novox/hq issue 445. func GroupState(g *Group, members []*Delivery) (string, string) { count := map[State]int{} + reached, ended := 0, 0 + var uncovered []string for _, d := range members { count[d.State]++ + if d.State.Final() { + ended++ + } + switch { + case d.State.Reached(), d.State == Superseded && d.CoveredBy != "": + reached++ + case d.State == Superseded: + uncovered = append(uncovered, d.ID) + } } n := len(members) switch { @@ -1479,10 +1523,12 @@ func GroupState(g *Group, members []*Delivery) (string, string) { return "failed", "a member failed" case count[Stopped] > 0 && g.Closed: return "stopped", "a member was stopped" - case n > 0 && count[Delivered]+count[Retired]+count[Covered] == n: + case n > 0 && reached == n: return "delivered", "every member is delivered" + case g.Closed && n > 0 && ended == n: + return "stopped", supersededWhy + strings.Join(uncovered, ", ") case g.Closed: - return "delivering", fmt.Sprintf("%d of %d delivered", count[Delivered]+count[Covered], n) + return "delivering", fmt.Sprintf("%d of %d delivered", reached, n) case len(g.Cycle) > 0: return "rejected", cycleSaid(g) case count[Rejected] > 0: @@ -1496,6 +1542,10 @@ func GroupState(g *Group, members []*Delivery) (string, string) { return "checking", fmt.Sprintf("%d of %d member(s) ready", count[Ready], n) } +// supersededWhy begins why a group whose members all ended is stopped. It is kept short: the forge's status line, +// "group stopped: ", is clipped at 140 characters, and the member's id must fit in it whole. +const supersededWhy = "superseded, not carried by a later delivery: " + func groupStatusState(s string) string { switch s { case "ready", "delivered": diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue442_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue442_test.go new file mode 100644 index 00000000..2863aca4 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue442_test.go @@ -0,0 +1,201 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// novox/hq issue 442: five closed groups read "delivering, 1 of 2 delivered" for days, each beside a member +// whose walk a later walk of the same trunk superseded. Nothing in them was still in progress. A superseded +// member counts as reached when a later delivered walk on the same trunk carried every module it moves (ADR +// 0270's covered test); a closed group whose members all ended reads delivered when every one reached the +// machines, and stopped otherwise — never delivering. +// +// The shapes are the live groups' as mesh-delivery showed them on 2026-10-11: feat/delivery-checks-verb (the +// superseded member last in the order) and fix/assign-says-why-a-module-is-not-there (the superseded member +// first). +func TestAGroupWhoseMembersEndedEnds(t *testing.T) { + const cat, ctl, lab = "novox/mesh-catalog", "novox/mesh-controller", "novox/mesh-lab" + at := time.Date(2026, 10, 7, 19, 4, 10, 0, time.UTC) + + // superseded is a member merged and walked, its walk superseded by a later walk of the same trunk. + superseded := func(repo, sha, merged, group string, modules ...string) *Delivery { + return &Delivery{ID: IDOf(repo, sha), Repository: repo, Commit: sha, Number: 109, Base: "main", + Branch: group, Group: group, State: Superseded, Since: at.Add(time.Minute), Created: at.Add(-time.Hour), + MergedAs: merged, MergedAt: at, Plan: planMoving(repo, modules...), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-" + merged[:6], Commit: merged, State: walkSuperseded, + Note: "superseded at tier 0 by plan-later (" + repo + " at 68aeee2c); planned there again"}, + Transitions: []Transition{{At: at.Add(time.Minute), From: Delivering, To: Superseded, Event: EvSuperseded, + Why: "a newer delivery to the same trunk took over its walk"}}} + } + // delivered is the other member, delivered by its own walk. + delivered := func(repo, sha, merged, group string) *Delivery { + return &Delivery{ID: IDOf(repo, sha), Repository: repo, Commit: sha, Number: 117, Base: "main", Branch: group, + Group: group, State: Delivered, Since: at.Add(-10 * time.Minute), Created: at.Add(-2 * time.Hour), + MergedAs: merged, MergedAt: at.Add(-time.Hour), Plan: planMoving(repo, "mesh-controller"), + Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-" + merged[:6], Commit: merged, State: walkDone}} + } + // later is the later merge of the same repository whose walk took the member's over and finished, sending + // each module given to every machine running it. + later := func(repo, merged string, modules ...string) *Delivery { + d := &Delivery{ID: IDOf(repo, "c0c0c0c0c0c0c0c0"), Repository: repo, Commit: "c0c0c0c0c0c0c0c0", Number: 110, + Base: "main", State: Delivered, Since: at.Add(30 * time.Minute), Created: at.Add(-time.Hour), + MergedAs: merged, MergedAt: at.Add(time.Minute), Plan: planMoving(repo, modules...), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-later", Commit: merged, State: walkDone}} + for _, m := range modules { + d.Steps = append(d.Steps, Step{Machine: "novox", Module: m, State: StepPassed}, + Step{Machine: "the rest running it", Module: m, State: StepSent}) + } + return d + } + group := func(id string, order ...string) *Group { + return &Group{ID: id, Members: append([]string(nil), order...), Closed: true, Order: order, Said: "delivering", + OrderOf: map[string]string{}} + } + + const checks, assign = "feat/delivery-checks-verb", "fix/assign-says-why-a-module-is-not-there" + catID, ctlID := IDOf(cat, "13894744a701fe59"), IDOf(ctl, "1fdff00794e95470") + asCtlID, asLabID := IDOf(ctl, "8adb7f1a051941e9"), IDOf(lab, "33e63ee5b47a7552") + for _, c := range []struct { + name string + kept []*Delivery + group *Group + contains bool // what the forge says of the later merge containing the superseded member's + want string + whySays string + }{ + {"superseded last, a later delivered walk carried every module it moves", + []*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"), + delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks), + later(cat, "68aeee2c00000000", "gitea", "mesh-delivery")}, + group(checks, ctlID, catID), true, "delivered", "every member"}, + {"superseded last, the later walk carried one of its two modules", + []*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"), + delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks), + later(cat, "68aeee2c00000000", "mesh-delivery")}, + group(checks, ctlID, catID), true, "stopped", "not carried by a later delivery: "}, + {"superseded last, the forge says the later merge does not contain it", + []*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"), + delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks), + later(cat, "68aeee2c00000000", "gitea", "mesh-delivery")}, + group(checks, ctlID, catID), false, "stopped", "not carried by a later delivery: "}, + {"superseded first, a later delivered walk carried every module it moves", + []*Delivery{superseded(ctl, "8adb7f1a051941e9", "a5f53ef9ebdf3650", assign, "build-agent", "mesh-controller", "route-proxy"), + delivered(lab, "33e63ee5b47a7552", "77777777aaaaaaaa", assign), + later(ctl, "3f68a49500000000", "build-agent", "mesh-controller", "route-proxy")}, + group(assign, asCtlID, asLabID), true, "delivered", "every member"}, + {"superseded first, nothing later delivered", + []*Delivery{superseded(ctl, "8adb7f1a051941e9", "a5f53ef9ebdf3650", assign, "build-agent", "mesh-controller", "route-proxy"), + delivered(lab, "33e63ee5b47a7552", "77777777aaaaaaaa", assign)}, + group(assign, asCtlID, asLabID), true, "stopped", "not carried by a later delivery: "}, + {"still in progress: a member delivering beside a superseded one", + func() []*Delivery { + d := delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks) + d.State, d.Walk.State = Delivering, walkRolling + return []*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"), d} + }(), + group(checks, ctlID, catID), true, "delivering", "0 of 2"}, + } { + t.Run(c.name, func(t *testing.T) { + store := newMemStore() + for _, d := range c.kept { + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + if err := store.PutGroup(c.group); err != nil { + t.Fatal(err) + } + forge := newFakeForge() + forge.contains[c.kept[0].MergedAs+" in "+"68aeee2c00000000"] = c.contains + forge.contains[c.kept[0].MergedAs+" in "+"3f68a49500000000"] = c.contains + w := newWorldOver(t, store, newFakeController(), forge, at.Add(4*24*time.Hour)) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + g := w.h.groups[c.group.ID] + state, why := GroupState(g, w.h.membersOf(g)) + if state != c.want || !strings.Contains(why, c.whySays) { + t.Fatalf("the group reads %s (%s), want %s saying %q", state, why, c.want, c.whySays) + } + // The forge is asked once per pair, however many ticks follow. + for range 3 { + w.later(2 * time.Minute) + w.settleAll() + } + w.forge.mu.Lock() + asked := map[string]int{} + for _, a := range w.forge.containsAsks { + asked[a]++ + } + w.forge.mu.Unlock() + for pair, n := range asked { + if n != 1 { + t.Fatalf("the forge was asked %d times whether %s", n, pair) + } + } + // What was learned survives a read-back, and the group reads the same with no new ask. + sup := w.delivery(c.kept[0].ID) + again := newWorldOver(t, store, newFakeController(), forge, w.now.Add(time.Hour)) + if err := again.h.Reconcile(); err != nil { + t.Fatal(err) + } + again.settleAll() + back := again.delivery(c.kept[0].ID) + if back.CoveredBy != sup.CoveredBy || len(back.ContainedIn) != len(sup.ContainedIn) { + t.Fatalf("read back, covered by %q and contained in %v; was %q and %v", back.CoveredBy, back.ContainedIn, + sup.CoveredBy, sup.ContainedIn) + } + for k, v := range sup.ContainedIn { + if back.ContainedIn[k] != v { + t.Fatalf("read back, contained in %s is %v, was %v", k, back.ContainedIn[k], v) + } + } + if s2, _ := GroupState(again.h.groups[c.group.ID], again.h.membersOf(again.h.groups[c.group.ID])); s2 != state { + t.Fatalf("read back, the group reads %s, was %s", s2, state) + } + forge.mu.Lock() + n := len(forge.containsAsks) + forge.mu.Unlock() + if want := len(asked); c.want != "delivering" && n != want { + t.Fatalf("read back, the forge was asked again: %d asks, was %d", n, want) + } + // Each member keeps its own end: the group's label changes, no delivery is moved by hand. + for _, d := range c.kept { + if got := w.state(d.ID); got != d.State { + t.Fatalf("%s moved from %s to %s", d.ID, d.State, got) + } + } + // What the group said reached every member's head, once it ended. + if c.want != "delivering" { + w.forge.mu.Lock() + s := w.forge.statuses[c.kept[0].Repository+"@"+c.kept[0].Commit+" mesh/delivery-group"] + w.forge.mu.Unlock() + if !strings.Contains(s, c.want) { + t.Fatalf("mesh/delivery-group on the superseded member's head says %q", s) + } + } + }) + } +} + +// The forge's mesh/delivery-group line, "group stopped: ", is clipped at 140 characters: for each of +// the five live groups of novox/hq issue 442, the superseded member's id fits in it whole. +func TestAStoppedGroupsLineNamesItsSupersededMemberWhole(t *testing.T) { + for group, member := range map[string]string{ + "feat/delivery-checks-verb": "novox/mesh-catalog@13894744a701", + "feat/machine-network-health": "novox/mesh-controller@b8bbf9c79fd8", + "fix/373-one-message-per-condition": "novox/mesh-catalog@8dfc19c9f7c8", + "fix/assign-says-why-a-module-is-not-there": "novox/mesh-controller@8adb7f1a0519", + "fix/needs-you-from-the-console": "novox/mesh-controller@2e6a9b1efc05", + } { + g := &Group{ID: group, Members: []string{member, "novox/hq@39585e3bb2ea"}, Closed: true} + state, why := GroupState(g, []*Delivery{{ID: member, State: Superseded}, {ID: "novox/hq@39585e3bb2ea", State: Delivered}}) + line := clip("group "+group+" "+state+": "+why, 140) + if state != "stopped" || !strings.HasSuffix(line, member) { + t.Fatalf("group %s: the line %q (%d characters) does not end in %s", group, line, len(line), member) + } + } +}