Never hold a message past its first hold's limit, so a fault on a machine walk after walk touches still reaches the operator (ADR 0505 §7)

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)
+	}
+}

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

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" {

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)
 	}
 }