From aa62de2e786028666a40ee0f862611b491cc311c Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 18:51:26 +0200 Subject: [PATCH] issue 797: a dropped tell no longer crowds the conditions' cap out of its own history A dropped tell was kept in the same bounded send history (KeptSends) that the conditions' and notify's cap counts sends from. A flood of drops could push real, still-recent sends out of that bounded history, undercounting sentLastHour and letting the conditions' cap stop holding. A dropped tell is still said once an hour in the log and named in Tell's own "dropped" answer; it is simply never recorded into that shared history any more. --- .../messenger/cmd/messenger/holder_test.go | 44 +++++++++++++++++++ modules/messenger/cmd/messenger/status.go | 11 +++-- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/modules/messenger/cmd/messenger/holder_test.go b/modules/messenger/cmd/messenger/holder_test.go index e79286a9..f4cfd76e 100644 --- a/modules/messenger/cmd/messenger/holder_test.go +++ b/modules/messenger/cmd/messenger/holder_test.go @@ -635,6 +635,50 @@ func TestATellCapDropIsLoggedOnceAnHour(t *testing.T) { } } +// TestAFloodOfDroppedTellsDoesNotEvictTheConditionsCapHistory is review round 2 of issue 800's messenger +// finding (issue 797's area): a dropped tell used to be kept in the same bounded send history the conditions' +// cap counts from (KeptSends); a flood of them could evict real, still-recent sends from it, undercounting +// sentLastHour and letting the conditions' cap stop holding. Here the cap is filled exactly, then 300 tells +// are dropped — far more than KeptSends — and the next condition must still be held, exactly as it would be +// without the flood. +func TestAFloodOfDroppedTellsDoesNotEvictTheConditionsCapHistory(t *testing.T) { + h, tg, _, c, _ := newHolder(t) + h.Desk = nil + // Fill the conditions' cap exactly: CapPerHour sent, none held yet. + for i := 0; i < CapPerHour; i++ { + h.Condition(EventRaised, cond(fmt.Sprintf("machine.m%d.silent", i), Urgent, "a machine is silent")) + c.pass(BurstWindow + time.Second) + h.Flush() + } + if len(tg.sends) != CapPerHour { + t.Fatalf("sent %d conditions, want the cap of %d", len(tg.sends), CapPerHour) + } + + // Use up the tell cap, then flood 300 more: all past TellCapPerHour are dropped, far more than KeptSends. + for i := 0; i < TellCapPerHour; i++ { + if _, err := h.Tell("mail", []string{"line"}, ""); err != nil { + t.Fatal(err) + } + } + for i := 0; i < 300; i++ { + if _, err := h.Tell("mail", []string{"line"}, ""); err != nil { + t.Fatal(err) + } + } + + // One more condition, past the cap: it must still be held, as it would be without the flood of drops. + h.Condition(EventRaised, cond("machine.mlast.silent", Urgent, "a machine is silent")) + c.pass(BurstWindow + time.Second) + h.Flush() + want := CapPerHour + TellCapPerHour + 1 // the cap's own conditions, the tells that did send, and the cap message + if len(tg.sends) != want { + t.Fatalf("sent %d, want %d (the cap still held after a flood of dropped tells)", len(tg.sends), want) + } + if last := tg.sends[len(tg.sends)-1]; !strings.HasPrefix(last.Title, "Mesh messages paused for this hour") { + t.Fatalf("the cap was not said after the flood of dropped tells: %q", last.Title) + } +} + func TestATestMessageRespectsTheRule(t *testing.T) { h, tg, dt, _, _ := newHolder(t) if out := h.Test("telegram", "see https://example.org"); out["refused"] == "" || len(tg.sends) != 0 { diff --git a/modules/messenger/cmd/messenger/status.go b/modules/messenger/cmd/messenger/status.go index cc89348a..e4192a76 100644 --- a/modules/messenger/cmd/messenger/status.go +++ b/modules/messenger/cmd/messenger/status.go @@ -319,7 +319,10 @@ func (h *Holder) Notify(key, severity, summary, subject string, clear bool, plai // reduction, withheld parts as today). It offers Details, like any message, and is kept in the history so // `history` shows it — but its record is already marked cleared, so `open` never shows it and it is forgotten // after ReopenWindow, as any cleared record is. Its cap, TellCapPerHour a channel an hour, is its own and -// never shared with CapPerHour: past it, a tell is counted as a drop in the history and said once in the log. +// never shared with CapPerHour: past it, a tell is dropped, said once an hour in the log and named in this +// call's own "dropped" answer, but never kept in the bounded send history the conditions' cap counts from +// (KeptSends, outbox.go's sentLastHour) — a flood of drops must never evict a real, still-recent send from it +// and let that cap stop holding (review round 2 of issue 800, issue 797's area). func (h *Holder) Tell(headline string, lines []string, subject string) (map[string]any, error) { headline = strings.TrimSpace(headline) if headline == "" { @@ -371,8 +374,10 @@ func (h *Holder) Tell(headline string, lines []string, subject string) (map[stri if sayIt { h.Logf("[messenger] a tell was dropped on %s: it is at its own cap of %d tells an hour", name, TellCapPerHour) } - h.record(Sent{At: now, Channel: name, Key: key, What: "tell", - Outcome: fmt.Sprintf("dropped: at its own cap of %d tells an hour", TellCapPerHour)}) + // Not h.record'd: that history is bounded (KeptSends) and is what the conditions' cap counts from + // (sentLastHour, outbox.go); a flood of drops must never evict a real send from it and let the + // conditions' cap stop holding (review round 2 of issue 800, issue 797's area). This call's own + // "dropped" answer, and the once-an-hour log line above, already say it happened. continue } m := h.sayOn(rec, "tell", name)