novox/mesh-catalog
8
0
Fork 0
Code Issues Pull Requests 5 Actions Packages Projects Releases Wiki Activity

issue 800: fix review round 2 findings in mesh-mail-notifier

Browse Source 
A buffer was replaced, discarding whatever it still held unsent, once
Window had passed since its first mail — losing a failed send waiting
for its retry, or a folded mail waiting for its minute's end, if a new
mail arrived meanwhile. The window now slides on the gap since the
last mail, not on time elapsed since the first: a session's count
never resets, and its buffer is never replaced, while anything in it
is still unsent.

The fold notice now counts every mail of the account in that minute,
not only the overflow, and says "mail" for one and "mails" for more.
This commit is contained in:
jochen
2026-10-15 18:50:21 +02:00
parent b771794f1c
commit 66713c5a1e
2 changed files with 225 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go
+81 -37
View File
@@ -4,10 +4,12 @@ package main
// issue 800 part B). It hears mesh-mail's added events on its own account (runs-as, ADR 0376 §2) and shows each
// new inbox mail at most once, through node-notifier.send: quiet during a first sync (mesh-mail already says
// nothing then, imap.go ~387), for mail from the account's own address, for mail already marked read (\Seen),
// and for anything outside the inbox. Of one account's qualifying mail within a sliding minute, the first
// three are shown one by one; the rest of that minute's mail is folded into one notice naming the count and
// the latest sender, rather than shown one by one (novox/hq ADR 0378 §9, the lead's reading, review round 1 of
// issue 800).
// and for anything outside the inbox. Of one account's qualifying mail, a mail is shown at once when fewer
// than FoldOver of that account's mails were shown in the Window before it; otherwise it is folded and shown,
// with the other mail its session folds, once Window has passed since the first of them — the window slides
// on the gap between mails, not on time elapsed since the session's first one, so a session already past its
// quota stays folding for as long as mail keeps arriving within Window of the last (novox/hq ADR 0378 §9, the
// lead's reading, review rounds 1 and 2 of issue 800).
//
// Nothing here names Betterbird (ADR 0379 §3).
@@ -29,9 +31,14 @@ const SeenFlag = "\\Seen"
// that window is folded into a single notice (novox/hq ADR 0378 §9: "more than three").
const FoldOver = 3
// Window is the sliding minute ADR 0378 §9 judges a burst over: an account's first qualifying mail opens it,
// and everything that account qualifies for until it closes, FoldOver mails onward, is one window's count.
// There is no separate quiet-gap timer (review round 1 of issue 800: BurstWindow goes).
// Window is the sliding minute ADR 0378 §9 judges a burst over. It slides on the gap between one qualifying
// mail and the next, not on time elapsed since a session's first mail (review round 2 of issue 800): a mail
// is shown at once when fewer than FoldOver of the account's mails were shown in the Window before it; past
// that, it is folded and shown, with whatever else its session folds, once Window has passed since the first
// of them. A session's count never resets while mail keeps arriving within Window of the last one, however
// long the session has run; it is only ever replaced once idle for a full Window with nothing left pending
// (review round 2 of issue 800, BLOCKING: a buffer is never replaced while it holds an unsent mail). There is
// no separate quiet-gap timer beside this (review round 1 of issue 800: BurstWindow goes).
const Window = time.Minute
// RetryBackoff is how long the notifier waits, after an account's notice fails to send, before it tries that
@@ -140,14 +147,18 @@ type AccountStatus struct {
Summary string `json:"summary"`
}
// accountBuffer is one account's sliding window: the first FoldOver qualifying mails are shown one by one as
// soon as Flush runs; the rest, from the fourth onward, are folded and shown as one notice once the window has
// run its Minute (novox/hq ADR 0378 §9, the lead's reading).
// accountBuffer is one account's sliding session: it stays open, and its count never resets, as long as a
// qualifying mail keeps arriving within Window of the previous one (review round 2 of issue 800). The first
// FoldOver mails of a session are shown one by one as soon as Flush runs; the rest, from the fourth onward,
// are folded and shown as one notice once Window has passed since the first of them, whether or not the
// session itself is still open.
type accountBuffer struct {
windowStart time.Time
count int // qualifying mails counted in this window so far
immediate []EventBody
folded []EventBody
lastArrival time.Time // the latest qualifying mail's arrival: a gap of Window since this lets a fresh session open
foldStart time.Time // when this session's first folded mail arrived; zero while nothing is folded
count int // every qualifying mail this session has counted, shown or folded alike (bullet 3: the
// fold notice names this total, not just the overflow)
immediate []EventBody
folded []EventBody
}
// Notifier buffers qualifying mail per account and shows it: the first FoldOver mails of a window one by one,
@@ -218,15 +229,25 @@ func (n *Notifier) Add(e EventBody) error {
}
now := n.Now()
b := n.pending[e.Account]
if b == nil || now.Sub(b.windowStart) >= Window {
b = &accountBuffer{windowStart: now}
// A session stays open, and its count never resets, as long as a qualifying mail keeps arriving within
// Window of the last one — even once more than Window has passed since the session's own first mail
// (the sliding minute, review round 2 of issue 800). It is replaced only once it is both idle for a full
// Window and fully flushed: nothing pending is ever discarded to make room for a fresh session (BLOCKING:
// a buffer is never replaced while it holds an unsent mail — a failed send waiting for its retry, or a
// folded mail waiting for its minute's end).
if b == nil || (now.Sub(b.lastArrival) >= Window && len(b.immediate) == 0 && len(b.folded) == 0) {
b = &accountBuffer{}
n.pending[e.Account] = b
}
b.lastArrival = now
b.count++
n.buffered[handle] = true
if b.count <= FoldOver {
b.immediate = append(b.immediate, e)
} else {
if b.foldStart.IsZero() {
b.foldStart = now
}
b.folded = append(b.folded, e)
}
return nil
@@ -299,8 +320,10 @@ func (n *Notifier) Flush() error {
return nil
}
// flushAccount ships one account's ready notices (its immediate mail, and its folded mail once the window is
// over), and keeps whatever fails to send pending, under cooldown, for the next Flush.
// flushAccount ships one account's ready notices (its immediate mail, and its folded mail once Window has
// passed since the first of them), and keeps whatever fails to send pending, under cooldown, for the next
// Flush. Nothing is deleted before it is shown (review round 1 BLOCKING 2), and nothing is replaced, or lost,
// while it is still pending (review round 2 BLOCKING; see Add).
func (n *Notifier) flushAccount(account string, now time.Time) {
n.mu.Lock()
if cd, ok := n.cooldown[account]; ok && now.Before(cd) {
@@ -313,17 +336,19 @@ func (n *Notifier) flushAccount(account string, now time.Time) {
return
}
immediate := append([]EventBody(nil), b.immediate...)
windowDone := now.Sub(b.windowStart) >= Window
// The fold group is due Window after its own first mail — independent of whether the session itself is
// still open (review round 2 of issue 800: the window slides on gaps, so a session can still be taking in
// new mail well past its first fold's deadline).
foldDue := !b.foldStart.IsZero() && now.Sub(b.foldStart) >= Window
var fold []EventBody
if windowDone {
foldCount := b.count // every mail of the account this session has counted (bullet 3), not just the fold's own
if foldDue {
fold = append([]EventBody(nil), b.folded...)
}
n.mu.Unlock()
if len(immediate) == 0 && len(fold) == 0 {
if windowDone {
n.closeIfEmpty(account, b)
}
n.closeIfIdle(account, b, now)
return
}
@@ -338,7 +363,7 @@ func (n *Notifier) flushAccount(account string, now time.Time) {
}
foldSent := false
if failErr == nil && len(fold) > 0 {
if err := n.show(n.foldNotice(account, fold)); err != nil {
if err := n.show(n.foldNotice(account, fold, foldCount)); err != nil {
failErr = err
} else {
foldSent = true
@@ -349,13 +374,19 @@ func (n *Notifier) flushAccount(account string, now time.Time) {
if cur := n.pending[account]; cur == b {
cur.immediate = cur.immediate[sentImmediate:]
if foldSent {
cur.folded = nil
cur.folded = cur.folded[len(fold):]
if len(cur.folded) == 0 {
cur.foldStart = time.Time{}
} else {
// More mail folded in while this batch was being sent: its own Window starts now, counted from
// when it was first noticed. Nothing of it is lost either way.
cur.foldStart = now
}
}
// The window itself — its start and count — is only ever dropped once it has run its course: while it
// is still open, mail arriving later in it must still count against the same window, even if nothing
// is pending to send right this moment (the bug review round 1 of issue 800 found: emptying the
// buffer early reset the window and let a fourth mail be shown on its own instead of folded).
if windowDone && len(cur.immediate) == 0 && len(cur.folded) == 0 {
// The session itself — its lastArrival and count — is only ever dropped once it has gone idle for a
// full Window with nothing left pending (review round 2 of issue 800, BLOCKING: a buffer is never
// replaced, or its unsent mail lost, while it still holds one).
if now.Sub(cur.lastArrival) >= Window && len(cur.immediate) == 0 && len(cur.folded) == 0 {
delete(n.pending, account)
}
}
@@ -369,10 +400,13 @@ func (n *Notifier) flushAccount(account string, now time.Time) {
n.mu.Unlock()
}
func (n *Notifier) closeIfEmpty(account string, b *accountBuffer) {
// closeIfIdle drops an account's session once it has gone idle for a full Window with nothing left pending:
// the next qualifying mail then opens a fresh session with a fresh quota (see Add).
func (n *Notifier) closeIfIdle(account string, b *accountBuffer, now time.Time) {
n.mu.Lock()
defer n.mu.Unlock()
if cur := n.pending[account]; cur == b && len(cur.immediate) == 0 && len(cur.folded) == 0 {
if cur := n.pending[account]; cur == b && now.Sub(cur.lastArrival) >= Window &&
len(cur.immediate) == 0 && len(cur.folded) == 0 {
delete(n.pending, account)
}
}
@@ -418,15 +452,17 @@ func (n *Notifier) singleNotice(account string, e EventBody) Notice {
}
}
// foldNotice is one notice for everything an account's window folded — its fourth qualifying mail onward —
// naming how many and the latest sender (novox/hq ADR 0378 §9: "more than three new mails from one account
// within a minute become one notice that says how many and names the latest sender").
func (n *Notifier) foldNotice(account string, events []EventBody) Notice {
// foldNotice is one notice for everything an account's session folded — its fourth qualifying mail onward —
// naming the latest sender and how many mail the account has had in total this session, not only the
// overflow (novox/hq ADR 0378 §9: "more than three new mails from one account within a minute become one
// notice that says how many and names the latest sender"; review round 2 of issue 800, bullet 3: the count is
// every mail of the account in that minute, "mail" for one and "mails" for more).
func (n *Notifier) foldNotice(account string, events []EventBody, count int) Notice {
latest := events[len(events)-1]
notice := Notice{
Account: account,
Args: map[string]any{
"summary": strconv.Itoa(len(events)) + " new mails in " + account,
"summary": strconv.Itoa(count) + " new " + mailWord(count) + " in " + account,
"body": "latest from " + latest.sender(),
"app_name": "mesh-mail",
},
@@ -438,6 +474,14 @@ func (n *Notifier) foldNotice(account string, events []EventBody) Notice {
return notice
}
// mailWord says "mail" for a count of one, "mails" for anything else (review round 2 of issue 800, bullet 3).
func mailWord(count int) string {
if count == 1 {
return "mail"
}
return "mails"
}
// show sends one notice, marks every message it accounts for seen, keeps the account's status, and emits
// `shown`. Nothing is marked, kept or emitted when Send fails: the caller keeps the notice pending instead.
func (n *Notifier) show(notice Notice) error {
modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go
+144 -4
View File
@@ -246,8 +246,8 @@ func TestABurstShowsTheFirstThreeAtOnceAndFoldsTheRestAtTheWindowsEnd(t *testing
if len(sent) != 4 {
t.Fatalf("after the window ended, sent %d notice(s), want 4 (the folded fourth mail too): %+v", len(sent), sent)
}
if sent[3]["summary"] != "1 new mails in home" || !strings.Contains(sent[3]["body"].(string), "Sender 4") {
t.Errorf("the fold notice is %+v, want it to say 1 new mail in home, naming Sender 4", sent[3])
if sent[3]["summary"] != "4 new mails in home" || !strings.Contains(sent[3]["body"].(string), "Sender 4") {
t.Errorf("the fold notice is %+v, want it to count every mail of the session (4), naming Sender 4", sent[3])
}
}
@@ -298,8 +298,8 @@ func TestARedeliveredEventIsNeverShownOrCountedTwice(t *testing.T) {
if len(sent) != 4 {
t.Fatalf("sent %d notice(s), want 4 (one, mail 2, mail 3, and one fold notice): %+v", len(sent), sent)
}
if sent[3]["summary"] != "1 new mails in home" {
t.Errorf("the fold notice is %+v, want it to count the redelivered fourth mail once, not twice", sent[3])
if sent[3]["summary"] != "4 new mails in home" {
t.Errorf("the fold notice is %+v, want it to count the redelivered fourth mail once, not twice, among the session's 4", sent[3])
}
}
@@ -394,3 +394,143 @@ func TestQualifyingRulesStillHold(t *testing.T) {
t.Fatalf("sent %+v, want nothing: an already-read mail, the account's own mail and mail outside the inbox never qualify", sent)
}
}
// TestABufferIsNeverReplacedWhileItHoldsAnUnsentMail is review round 2 of issue 800's first test (BLOCKING):
// a send that fails keeps its mail pending; once the cooldown has passed, but before the next Flush retries
// it, a new qualifying mail arrives. Neither mail is lost: both are shown once the retry succeeds.
func TestABufferIsNeverReplacedWhileItHoldsAnUnsentMail(t *testing.T) {
clock := &fakeClock{now: time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC)}
store := newFakeStore()
var sent []map[string]any
failSend := true
n := &Notifier{
Now: clock.Now,
Send: func(args map[string]any) error {
if failSend {
return errors.New("node-notifier.send: no answer")
}
sent = append(sent, args)
return nil
},
Accounts: func() (map[string]string, error) { return map[string]string{}, nil },
Store: store,
EmitShown: func(string, string) error { return nil },
Logf: t.Logf,
}
account := "home"
if err := n.Add(mail(account, 1, "m1", "Mail 1", "Sender 1")); err != nil {
t.Fatalf("Add mail 1: %v", err)
}
if err := n.Flush(); err != nil {
t.Fatalf("Flush: %v", err)
}
if len(sent) != 0 {
t.Fatalf("sent %d notice(s), want 0: the send failed", len(sent))
}
// The cooldown passes, but no retry has happened yet (Flush is not called again before the new mail
// arrives): a buffer this old must not be thrown away for a fresh one.
clock.Advance(RetryBackoff)
if err := n.Add(mail(account, 2, "m2", "Mail 2", "Sender 2")); err != nil {
t.Fatalf("Add mail 2: %v", err)
}
failSend = false
if err := n.Flush(); err != nil {
t.Fatalf("Flush: %v", err)
}
if len(sent) != 2 {
t.Fatalf("sent %d notice(s), want 2 (mail 1's retry and mail 2, neither lost): %+v", len(sent), sent)
}
for i, want := range []string{"Mail 1", "Mail 2"} {
if sent[i]["summary"] != want {
t.Errorf("sent[%d] = %+v, want summary %q", i, sent[i], want)
}
}
}
// TestAFoldedMailIsNeverLostBetweenTheWindowsEndAndTheNextFlush is review round 2 of issue 800's second test
// for the same BLOCKING finding: no failure here, but the fourth mail is folded, its minute ends, and a new
// mail arrives before the next Flush. The folded mail is still shown, never lost.
func TestAFoldedMailIsNeverLostBetweenTheWindowsEndAndTheNextFlush(t *testing.T) {
clock := &fakeClock{now: time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC)}
store := newFakeStore()
var sent []map[string]any
n := newTestNotifier(t, clock, store, &sent)
account := "home"
for i := 1; i <= 4; i++ {
if err := n.Add(mail(account, i, fmt.Sprintf("m%d", i), fmt.Sprintf("Mail %d", i), fmt.Sprintf("Sender %d", i))); err != nil {
t.Fatalf("Add mail %d: %v", i, err)
}
}
if err := n.Flush(); err != nil {
t.Fatalf("Flush: %v", err)
}
if len(sent) != 3 {
t.Fatalf("sent %d notice(s), want 3 shown one by one, the fourth folded, not yet due: %+v", len(sent), sent)
}
// The fold's minute is over (a Window since mail 4, its first folded mail), but Flush has not run again
// yet: a new mail arrives in that gap.
clock.Advance(Window + time.Second)
if err := n.Add(mail(account, 5, "m5", "Mail 5", "Sender 5")); err != nil {
t.Fatalf("Add mail 5: %v", err)
}
if err := n.Flush(); err != nil {
t.Fatalf("Flush: %v", err)
}
if len(sent) != 4 {
t.Fatalf("sent %d notice(s), want 4 (one fold notice carrying both mail 4 and mail 5): %+v", len(sent), sent)
}
if sent[3]["summary"] != "5 new mails in home" || !strings.Contains(sent[3]["body"].(string), "Sender 5") {
t.Errorf("the fold notice is %+v, want it to count all 5 of the session, naming Sender 5 (the latest)", sent[3])
}
}
// TestTheWindowSlidesOnGapsNotOnElapsedTimeSinceItsFirstMail is review round 2 of issue 800's test for the
// sliding minute: a mail is shown at once when fewer than three of the account's mails were shown in the 60
// seconds before it; otherwise it is folded and shown with the rest 60 seconds after the first of them. Mails
// at 0, 50, 55, 70, 75 and 80 seconds: 0, 50 and 55 are each within 60s of fewer than 3 prior mails, so each
// is shown; 70 is not (50 and 55 both fall in its preceding 60s, and the session is already past its quota),
// so it and everything after it, within the same ongoing session, fold into one notice.
func TestTheWindowSlidesOnGapsNotOnElapsedTimeSinceItsFirstMail(t *testing.T) {
clock := &fakeClock{now: time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC)}
store := newFakeStore()
var sent []map[string]any
n := newTestNotifier(t, clock, store, &sent)
account := "home"
deltas := []time.Duration{0, 50 * time.Second, 5 * time.Second, 15 * time.Second, 5 * time.Second, 5 * time.Second}
for i, d := range deltas {
clock.Advance(d)
if err := n.Add(mail(account, i+1, fmt.Sprintf("m%d", i+1), fmt.Sprintf("Mail %d", i+1), fmt.Sprintf("Sender %d", i+1))); err != nil {
t.Fatalf("Add mail %d: %v", i+1, err)
}
if err := n.Flush(); err != nil {
t.Fatalf("Flush at mail %d: %v", i+1, err)
}
}
if len(sent) != 3 {
t.Fatalf("sent %d notice(s) before the fold's window ends, want 3 shown one by one (0s, 50s, 55s): %+v", len(sent), sent)
}
for i, want := range []string{"Mail 1", "Mail 2", "Mail 3"} {
if sent[i]["summary"] != want {
t.Errorf("sent[%d] = %+v, want summary %q", i, sent[i], want)
}
}
// The fold group's own window — a minute from its first mail, at 70s — ends at 130s.
clock.Advance(50 * time.Second)
if err := n.Flush(); err != nil {
t.Fatalf("Flush: %v", err)
}
if len(sent) != 4 {
t.Fatalf("sent %d notice(s), want 4 (70s, 75s and 80s folded into one, at 130s): %+v", len(sent), sent)
}
if sent[3]["summary"] != "6 new mails in home" || !strings.Contains(sent[3]["body"].(string), "Sender 6") {
t.Errorf("the fold notice is %+v, want it to count all 6 of the session, naming Sender 6 (80s, the latest)", sent[3])
}
}