From 3cf75580b5eca1a853d2afc56c8b5c320fe93fdd Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 14:36:35 +0200 Subject: [PATCH] =?UTF-8?q?Never=20hold=20a=20message=20past=20its=20first?= =?UTF-8?q?=20hold's=20limit,=20so=20a=20fault=20on=20a=20machine=20walk?= =?UTF-8?q?=20after=20walk=20touches=20still=20reaches=20the=20operator=20?= =?UTF-8?q?(ADR=200505=20=C2=A77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-gate/adr505_test.go | 47 ++++++++++ .../cmd/mesh-delivery-gate/issue1275_test.go | 6 +- .../messenger/cmd/messenger/adr505_test.go | 54 +++++++++++ modules/messenger/cmd/messenger/outbox.go | 5 ++ modules/messenger/cmd/messenger/walks.go | 89 ++++++++++++++++++- modules/messenger/cmd/messenger/walks_test.go | 14 +-- 6 files changed, 201 insertions(+), 14 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_test.go index 8c17bd12..76833d2f 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_test.go @@ -7,6 +7,8 @@ import ( "strings" "testing" "time" + + stdio "git.novox.be/novox/mesh-sdk/go" ) // The tests of novox/hq ADR 0505 "How it is checked" for the gate: a refusal the gate is already answering is no @@ -245,3 +247,48 @@ func TestANewHeadWithinTwoHoursKeepsTheRejectedHeadFromTheOperator(t *testing.T) t.Fatalf("the superseded head reached the operator: %+v", got) } } + +// ADR 0505 §2: a walk's end said again by the controller with its declared machines (`declared_read`, a touch marked +// declared) acts once: no transition, comment or ask of the controller the first saying did not make. +func TestAWalkSaidAgainWithItsDeclaredMachinesActsOnceInTheGate(t *testing.T) { + w, id, plan := aDeliveringWalk(t) + done := w.ctl.walk(plan) + sent := time.Date(2026, 10, 6, 12, 40, 0, 0, time.UTC) + done.Modules["app"] = &WalkModule{State: "built", First: []string{"anchor"}, FirstAt: &sent, SentAt: &sent, Build: "build-1", + Gate: &WalkGate{Machines: []string{"anchor"}, Since: &sent, Passes: 1, Verdict: "passed", JudgedAt: &sent}} + done.State = walkDone + done = w.ctl.put(done) + body := func(again bool) []byte { + raw, err := json.Marshal(done) + if err != nil { + t.Fatal(err) + } + var said map[string]any + _ = json.Unmarshal(raw, &said) + said["walk"], said["phase"], said["outcome"] = plan, "ended", "delivered" + said["touches"] = []map[string]any{{"machine": "anchor", "modules": []string{"app"}}} + if again { + said["touches"] = append(said["touches"].([]map[string]any), + map[string]any{"machine": "shanks", "modules": []string{}, "declared": true}) + said["declared_read"] = true + } + out, _ := json.Marshal(said) + return out + } + if err := take(w.h, stdio.Envelope{Key: ControllerSeat + ".plan-moved", Body: body(false)}); err != nil { + t.Fatal(err) + } + w.settleAll() + if s := w.state(id); s != Delivered { + t.Fatalf("the walk's end left it %s", s) + } + transitions, comments, delivers := len(w.delivery(id).Transitions), len(w.forge.comments), len(w.ctl.delivers) + if err := take(w.h, stdio.Envelope{Key: ControllerSeat + ".plan-moved", Body: body(true)}); err != nil { + t.Fatal(err) + } + w.settleAll() + if len(w.delivery(id).Transitions) != transitions || len(w.forge.comments) != comments || len(w.ctl.delivers) != delivers { + t.Fatalf("said again it acted again: transitions %d→%d, comments %d→%d, asks of the controller %d→%d", + transitions, len(w.delivery(id).Transitions), comments, len(w.forge.comments), delivers, len(w.ctl.delivers)) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go index 807c5270..e26f5b17 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go @@ -252,7 +252,7 @@ func TestAMemberBothRefusedAndHeldForAStatusIsOneStalledLine(t *testing.T) { // 1275/AC6, as novox/hq ADR 0505 §4 changed it: a member refused because its verdict is out of date (the trunk moved; its // fresh check runs) is no stalled line while the gate answers it, and urgent once ten minutes passed with the group not // merged; the pass stops at the refusal all the same. -func TestAnOutOfDateRefusalIsAWarningAtOnceAndUrgentAfterTenMinutes(t *testing.T) { +func TestAnOutOfDateRefusalIsNoLineWhileAnsweredAndUrgentAfterTenMinutes(t *testing.T) { w := approvedGroup(t) allGreen(w) w.forge.refuse = errAskedAgain @@ -303,7 +303,7 @@ func severities(w *world) []string { // 1275/AC6 (b), as ADR 0505 §4 changed it: the same out of date refusal on a LATER member, after the earlier one merged, // is no line at once and urgent after ten minutes: the half merged group does not say it at once. -func TestAnOutOfDateRefusalOnALaterMemberIsAWarningThenUrgent(t *testing.T) { +func TestAnOutOfDateRefusalOnALaterMemberIsNoLineThenUrgent(t *testing.T) { w := approvedGroup(t) allGreen(w) w.forge.refuseRepo = map[string]error{gCtl: errAskedAgain} @@ -375,7 +375,7 @@ var mergeToolRefusals = []struct { // 1275/AC6 (e), as ADR 0505 §4 changed it: only the merge tool's two out of date refusals (:121, :147) are answered by // the gate and said as no line at once; the one saying its check could not be asked (:144) and every other is urgent at // once. -func TestOnlyTheMergeToolsTwoOutOfDateRefusalsAreWarnings(t *testing.T) { +func TestOnlyTheMergeToolsTwoOutOfDateRefusalsWaitWhileAnswered(t *testing.T) { for _, c := range mergeToolRefusals { if got := outOfDateRefusal(c.words); got != c.outOfDated { t.Errorf("%s: outOfDateRefusal = %v, wanted %v", c.line, got, c.outOfDated) diff --git a/modules/messenger/cmd/messenger/adr505_test.go b/modules/messenger/cmd/messenger/adr505_test.go index 50ebb0e3..93396e5c 100644 --- a/modules/messenger/cmd/messenger/adr505_test.go +++ b/modules/messenger/cmd/messenger/adr505_test.go @@ -316,3 +316,57 @@ func TestTheDeliveryLineNamesAtMostSixModulesAndNothingTheRuleRefuses(t *testing t.Fatalf("a refused name in the line: %q", b) } } + +// ADR 0505 §7: a condition on a machine whose declaration three back-to-back walks change (a machine that stays +// behind is a declared touch of each) reaches the operator at the first walk's bound and settle, while the third +// still runs: never held walk after walk. +func TestADeclaredMachinesConditionAcrossThreeWalksIsSaidAtTheFirstWalksLimit(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + t0 := c.now() + walk(t, h, "plan-1", WalkStarted, "", c.now(), 600, novox("gitea"), shanksDeclared) // limit: t0 + 10 + 2 min + k := cond("machine.shanks.uncomposable", Urgent, "shanks cannot get new instructions") + k.Machine, k.Raised = "shanks", c.now() + h.Condition(EventRaised, k) + runFor(h, c, 5*time.Minute) + walk(t, h, "plan-1", WalkEnded, WalkDelivered, c.now(), 0, novox("gitea"), shanksDeclared) + runFor(h, c, time.Minute) + walk(t, h, "plan-2", WalkStarted, "", c.now(), 600, novox("mesh-issues"), shanksDeclared) + runFor(h, c, 3*time.Minute) + walk(t, h, "plan-2", WalkEnded, WalkDelivered, c.now(), 0, novox("mesh-issues"), shanksDeclared) + runFor(h, c, time.Minute) + walk(t, h, "plan-3", WalkStarted, "", c.now(), 1200, novox("gitea"), shanksDeclared) + for c.now().Before(t0.Add(12*time.Minute - 10*time.Second)) { + runFor(h, c, 5*time.Second) + } + if heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("said before the first walk's limit: %+v", dt.sends) + } + runFor(h, c, 30*time.Second) + if !heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("held past the first walk's bound and settle: %v", outcomes(h, k.Key)) + } +} + +// ADR 0505 §2: a walk's start or end said again by the controller with its declared machines (declared_read) acts +// once: one walk with both sayings' touches, and one end message. +func TestAWalkSaidAgainWithItsDeclaredMachinesActsOnce(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-r", WalkStarted, "", c.now(), 600, novox("gitea")) + walk(t, h, "plan-r", WalkStarted, "", c.now(), 600, novox("gitea"), shanksDeclared) + if len(h.walks) != 1 || len(h.walks["plan-r"].Touches) != 2 { + t.Fatalf("said again it is not one walk with both touches: %+v", h.walks) + } + runFor(h, c, time.Minute) + walk(t, h, "plan-r", WalkEnded, WalkFailed, c.now(), 0, novox("gitea")) + walk(t, h, "plan-r", WalkEnded, WalkFailed, c.now(), 0, novox("gitea"), shanksDeclared) + runFor(h, c, 3*time.Minute) + n := 0 + for _, m := range append(append([]Message(nil), dt.sends...), tg.sends...) { + if strings.HasPrefix(m.Title, "Delivery failed") { + n++ + } + } + if n != 1 { + t.Fatalf("its end was told %d times: %+v", n, dt.sends) + } +} diff --git a/modules/messenger/cmd/messenger/outbox.go b/modules/messenger/cmd/messenger/outbox.go index b8b2c880..496e5a7f 100644 --- a/modules/messenger/cmd/messenger/outbox.go +++ b/modules/messenger/cmd/messenger/outbox.go @@ -39,6 +39,11 @@ func (h *Holder) deliver(r *Record, what string) { // An open walk touches it: told when the walk ends or passes its bound (ADR 0308 §2). return } + h.route(r, what) +} + +// route sends a record's message where its severity routes it, held by no walk. +func (h *Holder) route(r *Record, what string) { var to []Channel if kinds := h.askKinds(r); len(kinds) > 0 { // An ask goes to every channel that carries one of its answers (novox/hq ADR 0259): the desk for an diff --git a/modules/messenger/cmd/messenger/walks.go b/modules/messenger/cmd/messenger/walks.go index b9f01c29..a6bd83fc 100644 --- a/modules/messenger/cmd/messenger/walks.go +++ b/modules/messenger/cmd/messenger/walks.go @@ -125,8 +125,15 @@ type WalkEvent struct { type HeldMessage struct { At time.Time `json:"at"` What string `json:"what"` + // Until is the first hold's limit: the start, bound and settle of the walk that first held it, or that walk's + // settle's end when it was held settling (ADR 0505 §7). Kept when it is handed on: no walk holds it past this, so a + // fault on a machine walk after walk touches reaches the operator all the same. Zero in a hold kept before it was. + Until time.Time `json:"until,omitzero"` } +// past says whether a held message's first hold's limit has passed. +func (hm HeldMessage) past(now time.Time) bool { return !hm.Until.IsZero() && !now.Before(hm.Until) } + // Walk is one open walk as this router keeps it. type Walk struct { ID string `json:"id"` @@ -164,6 +171,15 @@ func (w *Walk) settleFor() time.Duration { return time.Duration(w.SettleSeconds) // limit is the longest a walk holds anything: its start, its bound and its settle (ADR 0505 §7). func (w *Walk) limit() time.Time { return w.Started.Add(w.bound() + w.settleFor()) } +// holdEnd is the last moment the walk holds anything it holds now: its limit, or its settle's end when that is +// sooner. +func (w *Walk) holdEnd() time.Time { + if w.settling() && w.SettleUntil.Before(w.limit()) { + return w.SettleUntil + } + return w.limit() +} + // settling says whether the walk ended delivered and holds on until its settle has passed (ADR 0505 §3). func (w *Walk) settling() bool { return !w.SettleUntil.IsZero() } @@ -483,11 +499,12 @@ func (h *Holder) release(w *Walk, now time.Time) (open []*Record, whats map[stri if o.ID == w.ID || !o.holding(now) { continue } - if _, ok := o.Held[key]; ok { - elsewhere = true + if other, ok := o.Held[key]; ok { + elsewhere = elsewhere || !other.past(now) continue } - if live && o.touches(s) { + if live && o.touches(s) && !hm.past(now) { + // Handed on with its first hold's limit: never held past it (ADR 0505 §7). if o.Held == nil { o.Held = map[string]HeldMessage{} } @@ -561,6 +578,7 @@ func (h *Holder) endWalk(w *Walk, outcome string, ended, now time.Time) { // settledWalks tells the end of every delivered walk whose settle has passed (ADR 0505 §3), oldest first. func (h *Holder) settledWalks(now time.Time) { + h.overdueHolds(now) h.mu.Lock() var settled []*Walk for _, w := range h.walks { @@ -575,6 +593,49 @@ func (h *Holder) settledWalks(now time.Time) { } } +// overdueHolds says every held message whose first hold's limit has passed while a walk that took it over still holds, +// on its own, with its delivery line (ADR 0505 §7): no walk holds it past that limit. A walk whose own end or bound is +// due tells what it holds in its message instead. Called at each flush. +func (h *Holder) overdueHolds(now time.Time) { + h.mu.Lock() + due := map[string]HeldMessage{} + var touched []*Walk + for _, w := range h.walks { + changed := false + if !now.Before(w.holdEnd()) { + continue // its own end or bound tells what it holds, in its message + } + for key, hm := range w.Held { + if hm.past(now) { + if prev, ok := due[key]; !ok || hm.At.Before(prev.At) { + due[key] = hm + } + delete(w.Held, key) + changed = true + } + } + if changed { + touched = append(touched, w) + } + } + var recs []*Record + for key, hm := range due { + if r := h.open[key]; r != nil && r.Cleared.IsZero() && !r.silenced(now) && !lastSaid(r).After(hm.At) { + recs = append(recs, r) + } + } + h.mu.Unlock() + for _, w := range touched { + h.keepWalk(w) + } + sort.Slice(recs, func(i, j int) bool { return recs[i].Key < recs[j].Key }) + for _, r := range recs { + what := due[r.Key].What + h.record(Sent{At: now, Channel: "-", Key: r.Key, What: what, Outcome: "released: past its first hold's limit"}) + h.route(r, what) + } +} + // endSettled tells the end of a delivered walk whose settle has passed (ADR 0505 §3): what it held still open, one // line each, and the cleared counted; then it is only the tomb that keeps a late start from opening it again. func (h *Holder) endSettled(w *Walk, now time.Time) { @@ -964,6 +1025,26 @@ func (h *Holder) hold(r *Record, what string) bool { Outcome: "not held: its walk " + f.Outcome}) return false } + // The first hold's limit: kept by whichever walk holds it now, else the soonest end of the walks about to. + var until time.Time + for _, w := range h.walks { + if hm, ok := w.Held[r.Key]; ok && !hm.Until.IsZero() && (until.IsZero() || hm.Until.Before(until)) { + until = hm.Until + } + } + if until.IsZero() { + for _, w := range h.walks { + if w.holding(now) && w.touches(s) && (until.IsZero() || w.holdEnd().Before(until)) { + until = w.holdEnd() + } + } + } + if !until.IsZero() && !now.Before(until) { + // Held to its first hold's limit already: said now, with its delivery line (ADR 0505 §7). + h.mu.Unlock() + h.record(Sent{At: now, Channel: "-", Key: r.Key, What: what, Outcome: "not held: past its first hold's limit"}) + return false + } var by []*Walk var ids []string fresh := false @@ -972,7 +1053,7 @@ func (h *Holder) hold(r *Record, what string) bool { continue } prev, had := w.Held[r.Key] - hm := HeldMessage{At: now, What: what} + hm := HeldMessage{At: now, What: what, Until: until} if had { hm.At = prev.At if !r.told() && prev.What == "raised" { diff --git a/modules/messenger/cmd/messenger/walks_test.go b/modules/messenger/cmd/messenger/walks_test.go index 01fc8c49..84fd2e33 100644 --- a/modules/messenger/cmd/messenger/walks_test.go +++ b/modules/messenger/cmd/messenger/walks_test.go @@ -480,11 +480,12 @@ func TestWhatAWalkHeldIsSaidOnceAChannelCanSendAgain(t *testing.T) { } } -// Review of pull request 230, 2: a message held by walk A, about a machine walk B also touches, is held on at -// A's end while B — started after it was held — is still open, and told at B's end. +// Review of pull request 230, 2, as novox/hq ADR 0505 §7 bounds it: a message held by walk A, about a machine walk B +// also touches, is held on at A's end while B — started after it was held — is still open, and is said once A's own +// limit (its start, bound and settle) has passed, while B still runs, with B's delivery line. func TestAMessageIsHeldOnByALaterWalkThatTouchesIt(t *testing.T) { h, tg, dt, c, _ := newHolder(t) - walk(t, h, "plan-a", WalkStarted, "", c.now(), 1200, novox("mesh-tools")) + walk(t, h, "plan-a", WalkStarted, "", c.now(), 360, novox("mesh-tools")) // its limit: 6 min and the settle k := cond("machine.novox.network", Urgent, "novox lost its default route") k.Raised = c.now() h.Condition(EventRaised, k) @@ -499,10 +500,9 @@ func TestAMessageIsHeldOnByALaterWalkThatTouchesIt(t *testing.T) { if got := h.heldBy(k.Key, c.now()); len(got) != 1 || got[0] != "plan-b" { t.Fatalf("the second walk does not hold it on: %v", got) } - walk(t, h, "plan-b", WalkEnded, WalkDelivered, c.now(), 0) - runFor(h, c, DefaultSettle+time.Minute) - if !heard("Still open after the delivery of node-tools to novox:\n- Urgent: "+headlineOf(k.Key), tg, dt) { - t.Fatalf("the second walk's end did not tell it: %+v %+v", tg.sends, dt.sends) + runFor(h, c, 4*time.Minute) // past the first walk's limit, 8 minutes after it was held + if !heard("During the delivery of node-tools to novox.", tg, dt) || !heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("held past its first hold's limit: %+v %+v", tg.sends, dt.sends) } }