From 6a8c718f440cda2e6c86deec821c60e0b135ea02 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 13:46:34 +0200 Subject: [PATCH] messenger: ask whether a channel is ready before composing its messages, so failed tries make no Details tickets (issue 371) The review found that each try at an unready channel composed its held messages and so made a Details ticket for each, and enough tries pushed out the tickets of messages already shown. --- .../messenger/cmd/messenger/details_test.go | 33 +++++++++++++++++++ modules/messenger/cmd/messenger/holder.go | 2 +- modules/messenger/cmd/messenger/outbox.go | 14 ++++---- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/modules/messenger/cmd/messenger/details_test.go b/modules/messenger/cmd/messenger/details_test.go index 8dba0d35..def1cb06 100644 --- a/modules/messenger/cmd/messenger/details_test.go +++ b/modules/messenger/cmd/messenger/details_test.go @@ -164,3 +164,36 @@ func TestDetailsOnAMessageWithAnOpenAskNeverAnswersIt(t *testing.T) { t.Errorf("Details ended the ask: %s", st) } } + +// A channel that stays unready makes no ticket at its tries: composing a message makes its Details ticket, +// and tickets made at every try would push out those of the messages already shown. +func TestAnUnreadyChannelPushesOutNoShownDetails(t *testing.T) { + h, tg, _, c, _ := newHolder(t) + h.Desk = nil + k := cond("machine.ace.silent", Warning, "the home server is silent") + k.Raised = c.now() + h.Condition(EventRaised, k) + settle(h, c) + tok := detailsOf(t, tg.sends[0]) + k.Severity = Urgent + h.Condition(EventChanged, k) + tg.notReady = fmt.Errorf("telegram is away") + for i := 0; i < 3*MostDetails; i++ { + c.pass(2 * time.Minute) + h.Flush() + } + if n := len(h.open[k.Key].Tokens); n != 1 { + t.Errorf("%d tickets after the tries, want the one shown", n) + } + tg.notReady = nil + c.pass(2 * time.Minute) + h.Flush() + if len(tg.sends) != 2 { + t.Fatalf("%d messages, want 2", len(tg.sends)) + } + for _, tk := range []string{tok, detailsOf(t, tg.sends[1])} { + if out, err := h.Chosen("telegram", choiceOf(tk, "42", c.now())); err != nil || out != "shown" { + t.Errorf("Details: %q %v", out, err) + } + } +} diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index f72279b3..4ef00ac2 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -142,7 +142,7 @@ type Record struct { Explanation string `json:"explanation,omitempty"` Resolved string `json:"resolved,omitempty"` // Needs is what the operator does, empty for nothing; Actions the answers offered, each by a token - // (Tokens: token -> the action's index, or -1 for the details) a channel hands back when chosen. + // (Tokens: token -> its grant; Index -1 is Details, one per message shown) a channel hands back when chosen. Needs string `json:"needs,omitempty"` Actions []Action `json:"actions,omitempty"` // Kept as "grants": the build before ADR 0258 kept "tokens" as token -> index, which this form cannot diff --git a/modules/messenger/cmd/messenger/outbox.go b/modules/messenger/cmd/messenger/outbox.go index d3c473a3..1b264e4d 100644 --- a/modules/messenger/cmd/messenger/outbox.go +++ b/modules/messenger/cmd/messenger/outbox.go @@ -243,6 +243,14 @@ func (h *Holder) flushChannel(ch Channel, now time.Time, force bool) { if len(list) == 0 || (!force && !h.due(ch, list, now)) { return } + taken := len(list) + // Asked before the messages are composed: composing one makes its Details ticket, and a channel that + // stays unready would make one at every try, pushing out the tickets of messages already shown (issue 371). + if err := ch.Ready(); err != nil { + h.noteChannel(name, err) + h.failed(ch, list, taken, now, err) + return + } var entries []entry for _, it := range list { r, m := h.resolve(it, now, name) @@ -251,16 +259,10 @@ func (h *Holder) flushChannel(ch Channel, now time.Time, force bool) { } entries = append(entries, entry{it, r, *m}) } - taken := len(list) if len(entries) == 0 { h.drop(name, taken) return } - if err := ch.Ready(); err != nil { - h.noteChannel(name, err) - h.failed(ch, list, taken, now, err) - return - } if !h.allow(name) { // The last line of defence: said once, in a message of its own, and the rest held. h.mu.Lock()