From 2bee70a47858ba0617ebda46ef672b7f4c229c89 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 11:24:30 +0200 Subject: [PATCH] Give a waiting walk its word or say what holds it (issues 529, 362) A merge whose plan sends nothing is delivered at once, yet the controller can still open a walk for it; no live delivery then held the walk and it waited for a person (the nzbgeek case). Give such a walk its word. Name each held delivery in the status, and let stalled say, for every waiting walk, what holds it, so the controller's plans line points at the person's decision instead of a hand start. --- .../cmd/mesh-delivery/answers.go | 159 ++++++++++++++- .../mesh-delivery/cmd/mesh-delivery/holder.go | 6 + .../cmd/mesh-delivery/issue529_test.go | 182 ++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/main.go | 13 +- .../mesh-delivery/cmd/mesh-delivery/verbs.go | 26 +++ 5 files changed, 382 insertions(+), 4 deletions(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/answers.go b/modules/mesh-delivery/cmd/mesh-delivery/answers.go index 2dd8e594..f26c59a7 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/answers.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/answers.go @@ -366,6 +366,7 @@ func (h *Holder) words() []func() { } sort.Strings(walks) var asks []func() + h.waitsWhy = map[string]string{} for _, walk := range walks { want := h.turning[walk] var waiting []string @@ -377,14 +378,16 @@ func (h *Holder) words() []func() { } if len(waiting) > 0 { sort.Strings(waiting) - h.Logf("[mesh-delivery] the walk %s is not let go yet: %s carried by it not at its turn", walk, - strings.Join(waiting, ", ")) + h.waitsWhy[walk] = strings.Join(waiting, ", ") + " carried by it not at its turn" + h.Logf("[mesh-delivery] the walk %s is not let go yet: %s", walk, h.waitsWhy[walk]) continue } ids, why := append([]string(nil), want.ids...), strings.Join(want.whys, "; ") // **Never the word for a walk that carries a stopped change** (ADR 0299, review of #221): it is asked to // leave the merge out first (leaveStopped). if stopped, _ := h.carriesStopped(walk); len(stopped) > 0 { + h.waitsWhy[walk] = "it carries " + strings.Join(stopped, ", ") + ", whose delivery was stopped: it is " + + "asked to leave that merge out first" h.Logf("[mesh-delivery] the walk %s is not let go: it carries %s, whose delivery was stopped", walk, strings.Join(stopped, ", ")) continue @@ -419,3 +422,155 @@ func (h *Holder) words() []func() { h.turning = nil return asks } + +// --- a walk answering only merges that moved nothing --------------------------------------------------------- + +// endedMovingNothing is whether a delivery ended delivered with nothing for a walk to move: merged, its plan +// moving no module the mesh holds, and no walk followed. +func endedMovingNothing(d *Delivery) bool { + return d.State == Delivered && d.MergedAs != "" && d.Walk == nil && d.MovesNothing() +} + +// answeredMovingNothing are the deliveries a waiting walk answers when every merge it answers ended delivered +// with nothing for a walk to move; nil when any merge it answers is held by a live delivery, ended otherwise, or +// is not known to this owner. Under the lock. +// +// novox/hq issues 362 (the nzbgeek case, 2026-10-11) and 529: a merge whose plan adds a module and sends it +// nowhere is delivered at once, but the controller may still open a walk for it — when the module was placed on a +// machine after its plan was made. No live delivery then holds the walk, and nothing gave it its word: it waited +// 39 minutes for a person. Nothing is left for a person to decide about such a walk, so this owner gives it. +func (h *Holder) answeredMovingNothing(w Walk) []*Delivery { + if w.Release != nil || w.IsBatch() || !w.Open() || !w.Seen().Waits { + return nil + } + var out []*Delivery + for _, d := range h.deliveries { + _, answers := walkAnswers(w, d) + if !answers && (d.Walk == nil || d.Walk.ID != w.ID) { + continue + } + if !endedMovingNothing(d) { + return nil + } + out = append(out, d) + } + if len(out) == 0 { + return nil + } + // Every merge it names is one of them: a merge not heard of, or not yet ended, is not this owner's to answer. + if w.Delivery != nil { + for _, m := range w.Delivery.Merges { + found := false + for _, d := range out { + if strings.EqualFold(d.Repository, m.Repository) && sameCommit(d.MergedAs, m.Commit) { + found = true + break + } + } + if !found { + return nil + } + } + } + sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) + return out +} + +// nothingToMoveWords are the asks of the word for each waiting walk that answers only merges ended as nothing for +// a walk to move (answeredMovingNothing). Under the lock. +func (h *Holder) nothingToMoveWords() []func() { + ids := make([]string, 0, len(h.walks)) + for id := range h.walks { + ids = append(ids, id) + } + sort.Strings(ids) + var asks []func() + for _, walk := range ids { + if h.asking[walk] { + continue + } + ended := h.answeredMovingNothing(h.walks[walk].walk) + if len(ended) == 0 { + continue + } + var names []string + for _, d := range ended { + names = append(names, d.ID) + } + why := "it answers only " + strings.Join(names, ", ") + ", delivered with nothing for a walk to move: " + + "no person has anything to decide (novox/hq issue 529)" + h.asking[walk] = true + walk := walk + asks = append(asks, func() { + err := h.Controller.Deliver(walk, why) + h.mu.Lock() + defer h.mu.Unlock() + delete(h.asking, walk) + if err != nil { + h.Logf("[mesh-delivery] the walk %s could not be let go yet: %v", walk, err) + return + } + h.Logf("[mesh-delivery] the walk %s was let go: %s", walk, why) + // Asked once: the record kept says it went, until the controller's next record says more. + if s, known := h.walks[walk]; known && s.walk.Delivery != nil { + wd, now := *s.walk.Delivery, h.Now() + wd.Go, wd.By, wd.Why = &now, byOwner, why + s.walk.Delivery = &wd + h.walks[walk] = s + } + }) + } + return asks +} + +// WalksWaiting is `stalled` asked with `walks` (novox/hq issue 529): for each walk waiting for this owner's word, +// what holds it, in the shape of a stalled line naming the walk — each held delivery it carries (State held, its +// reason and the verb that releases it), else why its word was not given at the last tick (State published), else +// that no live delivery holds it (State unheld). A walk whose word is being given is not listed. The controller's +// `plans` says these instead of pointing at a hand start; the self-check never raises a condition for them. +func (h *Holder) WalksWaiting() []StalledLine { + h.mu.Lock() + defer h.mu.Unlock() + now := h.Now() + var out []StalledLine + for id, s := range h.walks { + w := s.walk + if w.Release != nil || w.IsBatch() || !w.Open() || !w.Seen().Waits || h.asking[id] { + continue + } + waited := now.Sub(w.Created).Round(time.Second).String() + var held []StalledLine + live := false + for _, d := range h.deliveries { + if d.State.Final() || d.Walk == nil || d.Walk.ID != id { + continue + } + live = true + if d.State == Held { + held = append(held, StalledLine{ID: d.ID, Number: d.Number, State: Held, Walk: id, For: waited, + Bound: "0s", H2: "none: held for a person — the operator's", Says: HeldLine(d)}) + } + } + switch { + case len(held) > 0: + out = append(out, held...) + case h.waitsWhy[id] != "": + out = append(out, StalledLine{ID: IDOf(w.Repository, w.Commit), State: Published, Walk: id, For: waited, + Bound: "0s", H2: "none: it goes at its turn", Says: "not let go yet: " + h.waitsWhy[id]}) + case len(h.answeredMovingNothing(w)) > 0: + // Its word is given at the next tick (nothingToMoveWords). + case !live: + out = append(out, StalledLine{ID: IDOf(w.Repository, w.Commit), State: Unheld, Walk: id, For: waited, + Bound: "0s", H2: "none: no delivery holds the walk — the operator's", + Says: "no delivery that has not ended follows it or is answered by it, so nothing gives it its word: " + + "`mesh-delivery.show` for the deliveries of " + w.Repository}) + } + } + sort.Slice(out, func(i, j int) bool { + if out[i].Walk != out[j].Walk { + return out[i].Walk < out[j].Walk + } + return out[i].ID < out[j].ID + }) + return out +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 23d1185d..dc0d1355 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -55,6 +55,9 @@ type Holder struct { walks map[string]seenWalk orphans map[string]time.Time turning map[string]*turnWant + // waitsWhy is, by walk, why the last tick did not give a waiting walk its word: a delivery it carries not at + // its turn, or a stopped change it carries (novox/hq issue 529). Said by WalksWaiting. + waitsWhy map[string]string // asking are the walks whose word is being asked right now, outside the lock: not asked twice meanwhile. asking map[string]bool // leaving are, by walk, the merges a stop gathered to leave out of it, asked once (novox/hq ADR 0299). @@ -103,6 +106,7 @@ func (h *Holder) init() { h.written = map[string][]byte{} h.dirtyGroup = map[string]bool{} h.unmatched = map[string]unmatched{} + h.waitsWhy = map[string]string{} h.askTried = map[string]time.Time{} h.walkAsked = map[string]time.Time{} h.unchecked = map[string]unchecked{} @@ -1028,6 +1032,8 @@ func (h *Holder) plan() []func() { } // The word, once per walk, when every live delivery it carries is at its turn. asks = append(asks, h.words()...) + // A waiting walk that answers only merges ended as nothing for a walk to move (novox/hq issues 362, 529). + asks = append(asks, h.nothingToMoveWords()...) // Waiting walks no live delivery holds: listed by `stalled` (issue 362). h.seeOrphans(now) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go new file mode 100644 index 00000000..60c8a111 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go @@ -0,0 +1,182 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// novox/hq issue 362's note of 2026-10-11 (the nzbgeek case) and issue 529: mesh-media-catalog #18 added a module +// whose delivery plan said "sent nowhere". This owner ended its delivery at once, "nothing for a walk to move", but +// the controller opened a walk for the merge that moved modules to a machine and waited for this owner's word. +// With its delivery ended, nothing gave the walk its word: it held 39 minutes and two walks queued behind it. + +const ( + media = "novox/mesh-media-catalog" + headNzb = "abababab00001818" + mergeNzb = "cdcdcdcd00001818" + nzbWalkID = "plan-1791748193836841649" +) + +var atNzb = time.Date(2026, 10, 11, 19, 43, 0, 0, time.UTC) + +// nzbgeekMerged is #18 checked on a plan that builds a new module and sends it nowhere, and merged: its delivery +// ends at once, nothing for a walk to move. +func nzbgeekMerged(t *testing.T) *world { + t.Helper() + w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), atNzb.Add(-time.Minute)) + p := pr(media, 18, headNzb, "feat/nzbgeek", "modules/nzbgeek/module.json") + w.h.PullUpdated(p) + v := verdict(media, 18, headNzb, "pass") + v.Plan = &DeliveryPlan{Repository: media, Base: "main", New: []string{"modules/nzbgeek"}, + Summary: "builds new: modules/nzbgeek, sent nowhere; no bus step"} + w.h.Checked(v) + w.later(time.Minute) + w.h.PullMerged(mergedAt(p, mergeNzb, atNzb)) + w.settleAll() + if d := w.delivery(IDOf(media, headNzb)); d.State != Delivered { + t.Fatalf("#18 is %s, not delivered as nothing for a walk to move", d.State) + } + return w +} + +// nzbWalk is the walk the controller cut for the merge's batch: it names the merge and moves nzbgeek and nzbget. +func nzbWalk(created time.Time, merges ...WalkMerge) Walk { + w := Walk{ID: nzbWalkID, Repository: media, Branch: "main", Commit: mergeNzb, Created: created, State: walkBuilding, + Tiers: [][]string{{"nzbgeek", "nzbget"}}, Modules: map[string]*WalkModule{"nzbgeek": {}, "nzbget": {}}, + Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: merges}} + return w +} + +// **A walk answering only merges that ended as nothing for a walk to move is given its word** (novox/hq issues 362 +// and 529): no live delivery holds it, no person has anything to decide, and nothing else would ever let it go. +func TestAWalkAnsweringOnlyNothingToMoveMergesIsGivenItsWord(t *testing.T) { + w := nzbgeekMerged(t) + w.later(90 * time.Second) + w.h.WalkMoved(w.ctl.put(nzbWalk(w.now, WalkMerge{Repository: media, Commit: mergeNzb}))) + w.settleAll() + if len(w.ctl.delivers) != 1 || w.ctl.delivers[0] != nzbWalkID { + t.Fatalf("the walk answering only #18 was let go %v; stalled says %+v", w.ctl.delivers, w.h.Stalled()) + } + w.h.WalkMoved(w.ctl.walk(nzbWalkID)) + w.later(3 * time.Minute) + w.settleAll() + for _, l := range w.h.Stalled() { + if l.Walk == nzbWalkID { + t.Fatalf("stalled still lists the walk: %+v", l) + } + } + if d := w.delivery(IDOf(media, headNzb)); d.State != Delivered || d.Walk != nil { + t.Fatalf("#18 is %s on %v: an ended delivery is never tied to a walk", d.State, d.Walk) + } +} + +// The same walk from a controller that names no merges: it answers the merge of its own commit. +func TestAWalkOfANothingToMoveMergesOwnCommitIsGivenItsWord(t *testing.T) { + w := nzbgeekMerged(t) + w.later(90 * time.Second) + w.h.WalkMoved(w.ctl.put(nzbWalk(w.now))) + w.settleAll() + if len(w.ctl.delivers) != 1 || w.ctl.delivers[0] != nzbWalkID { + t.Fatalf("the walk of #18's commit was let go %v", w.ctl.delivers) + } +} + +// Never for a walk that also answers a merge this owner holds no ended nothing-to-move delivery of: a merge not +// heard yet, or one that ended otherwise, is not a word this owner may give. +func TestAWalkAnsweringAnotherMergeIsNotGivenTheWordForANothingToMoveOne(t *testing.T) { + w := nzbgeekMerged(t) + w.later(90 * time.Second) + w.h.WalkMoved(w.ctl.put(nzbWalk(w.now, WalkMerge{Repository: media, Commit: mergeNzb}, + WalkMerge{Repository: media, Commit: "efefefef00002121"}))) + w.settleAll() + if len(w.ctl.delivers) != 0 { + t.Fatalf("a walk answering a merge not heard was let go: %v", w.ctl.delivers) + } +} + +// Its word is said: the log of the owner and the walk's why name the merges that ended as nothing to move. +func TestTheWordForANothingToMoveWalkSaysWhy(t *testing.T) { + w := nzbgeekMerged(t) + w.h.WalkMoved(w.ctl.put(nzbWalk(w.now, WalkMerge{Repository: media, Commit: mergeNzb}))) + w.settleAll() + walk := w.ctl.walk(nzbWalkID) + if walk.Delivery == nil || !strings.Contains(walk.Delivery.Why, "nothing for a walk to move") || + !strings.Contains(walk.Delivery.Why, IDOf(media, headNzb)) { + t.Fatalf("the word says %+v", walk.Delivery) + } +} + +// **The status names each held delivery, why, and the verb that releases it** (novox/hq issue 529): "held: 1" +// alone pointed a reader nowhere. +func TestTheStatusNamesAHeldDeliveryAndWhy(t *testing.T) { + w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), atNzb) + p := pr(media, 22, headNzb, "adr-0307-state-history", "modules/nzbgeek/module.json") + w.h.PullUpdated(p) + w.h.PullMerged(mergedAt(p, mergeNzb, atNzb)) // merged before any verdict: held for a person + w.settleAll() + id := IDOf(media, headNzb) + d := w.delivery(id) + if d.State != Held { + t.Fatalf("#22 is %s", d.State) + } + held, _ := w.h.Status("listening")["held"].([]string) + if len(held) != 1 || !strings.Contains(held[0], id) || !strings.Contains(held[0], d.HeldWhy) || + !strings.Contains(held[0], "mesh-delivery.release") { + t.Fatalf("the status says held %q", held) + } +} + +// **A walk waiting for this owner's word is told why** (novox/hq issue 529): asked with `walks`, `stalled` says of +// each walk waiting for its word what holds it — a held delivery, naming it, its reason and `mesh-delivery.release`, +// or the deliveries not at their turn — so the controller's `plans` points at the person's decision, not at a hand +// start. +func TestAWaitingWalkIsToldWhatHoldsIt(t *testing.T) { + w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), atDu.Add(5*time.Second)) + cc := pr(cat, 101, headCC, "feat/claude-code", "modules/claude-code/module.json") + du := pr(cat, 102, headDu, "feat/dunst", "modules/dunst/module.json") + w.h.PullUpdated(cc) + w.h.PullUpdated(du) + w.h.Checked(verdictMoving(cat, 101, headCC, "claude-code")) + w.h.PullMerged(mergedAt(cc, mergeCC, atCC)) + w.h.PullMerged(mergedAt(du, mergeDu, atDu)) // unchecked: held for a person + walk := catWalk("plan-9", mergeDu, atDu.Add(5*time.Second), "claude-code", "dunst") + walk.Delivery.Merges = []WalkMerge{{Repository: cat, Commit: mergeCC, Carried: mergeDu}, {Repository: cat, Commit: mergeDu}} + w.h.WalkMoved(w.ctl.put(walk)) + w.settleAll() + idDu := IDOf(cat, headDu) + lines := w.h.WalksWaiting() + if len(lines) != 1 || lines[0].Walk != "plan-9" || lines[0].State != Held || lines[0].ID != idDu || + !strings.Contains(lines[0].Says, w.delivery(idDu).HeldWhy) || !strings.Contains(lines[0].Says, "mesh-delivery.release") { + t.Fatalf("the waiting walk is said as %+v", lines) + } + // Not by default: `stalled` stays what the self-check raises conditions for. + for _, l := range w.h.Stalled() { + if l.Walk == "plan-9" { + t.Fatalf("stalled lists the waiting walk without being asked: %+v", l) + } + } + // Released, its word is asked: nothing holds it any more. + if _, err := w.h.Release(idDu, "checked by hand", "jochen"); err != nil { + t.Fatal(err) + } + w.h.WalkMoved(w.ctl.walk("plan-9")) + w.settleAll() + if lines := w.h.WalksWaiting(); len(lines) != 0 { + t.Fatalf("a walk let go is still said waiting: %+v", lines) + } +} + +// A walk waiting while a delivery it carries is not at its turn says so, naming the delivery — what was said only +// in this owner's journal (novox/hq issue 529, note 2). +func TestAWaitingWalkSaysWhichDeliveryIsNotAtItsTurn(t *testing.T) { + w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), atNzb) + w.h.mu.Lock() + w.h.waitsWhy = map[string]string{"plan-7": "novox/x@1 (published) carried by it not at its turn"} + w.h.walks["plan-7"] = seenWalk{walk: aWalk("plan-7", "novox/x", "1111", true), at: atNzb} + w.h.mu.Unlock() + lines := w.h.WalksWaiting() + if len(lines) != 1 || lines[0].Walk != "plan-7" || !strings.Contains(lines[0].Says, "not at its turn") { + t.Fatalf("the waiting walk is said as %+v", lines) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 984bf218..6a41bbb7 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -268,8 +268,17 @@ func tools(h *Holder, l *listening) []stdio.Tool { "and what healer H2 may do once it has passed; and the machine steps' table.", Run: func(map[string]any) (any, error) { return TableText(), nil }}, {Name: seat + "stalled", - Description: "Every delivery held past its state's bound, with the transition the table lets healer H2 take.", - Run: func(map[string]any) (any, error) { return h.Stalled(), nil }}, + Description: "Every delivery held past its state's bound, with the transition the table lets healer H2 take. " + + "With walks: true, instead each walk waiting for this owner's word and what holds it — a held delivery, " + + "its reason and the verb that releases it, or the deliveries not at their turn (novox/hq issue 529).", + Input: map[string]any{"type": "object", "properties": map[string]any{ + "walks": map[string]any{"type": "boolean", "description": "the waiting walks and what holds each, instead"}}}, + Run: func(a map[string]any) (any, error) { + if walks, _ := a["walks"].(bool); walks { + return h.WalksWaiting(), nil + } + return h.Stalled(), nil + }}, {Name: seat + "recheck", Description: "Check a rejected or ready delivery again: its merge check statuses put back to pending on the forge, " + "so nothing merges on the old verdict; proposed again, and its check asked. With why.", diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index 9771e281..4148b6e9 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -635,6 +635,18 @@ func (h *Holder) Status(listening string) map[string]any { out := map[string]any{"deliveries": byState, "groups": len(h.groups), "owed": owed, "unkept": len(h.dirty), "walks-unmatched": len(h.unmatched), "events": listening, "started": h.started.Format(time.RFC3339), "seat-on-record": h.held} + // Each held delivery by name, with why and the verb that releases it (novox/hq issue 529): a count alone + // pointed a reader at nothing. + var held []string + for _, d := range h.deliveries { + if d.State == Held { + held = append(held, HeldLine(d)) + } + } + if len(held) > 0 { + sort.Strings(held) + out["held"] = held + } if !h.reconciled.IsZero() { out["walks-read"] = h.reconciled.Format(time.RFC3339) } @@ -743,3 +755,17 @@ func withoutKinds(owed []Effect, kinds ...string) []Effect { } return out } + +// HeldLine says a held delivery in one line: which, since when, why, its walk when one waits, and the verb that +// releases it (novox/hq issue 529). +func HeldLine(d *Delivery) string { + why := d.HeldWhy + if why == "" { + why = lastWhyOf(d) + } + s := fmt.Sprintf("%s held since %s: %s", d.ID, d.Since.UTC().Format(time.RFC3339), why) + if d.Walk != nil && d.Walk.Waits { + s += "; its walk " + d.Walk.ID + " waits for it" + } + return s + " — a person decides: `mesh-delivery.release` (id " + d.ID + ", why) lets it go, `mesh-delivery.stop` ends it" +}