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.

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

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

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