Merge pull request 'mesh-mail-reader: read messenger's sent_on as a list, so an unchanged set of mails is signalled once, not every 160 s (issue 1926)' (#469) from fix/1926-mail-signal-once into main

Reviewed-on: #469

modules/mesh-mail-reader/cmd/mesh-mail-reader/acceptance_integration_test.go

@@ -90,7 +90,9 @@ func TestAcceptancePublishedMailBecomesAnEntryNeverKeepingItsBody(t *testing.T)
 	}
 	r.stand.Judge = judgeEveryMailAs("neither", "newsletters", "", "")
 	r.stand.MessengerCheck = func(map[string]any) (any, error) { return map[string]any{"allowed": true}, nil }
-	r.stand.Tell = func(map[string]any) (any, error) { return map[string]any{"sent_on": ""}, nil }
+	r.stand.Tell = func(map[string]any) (any, error) {
+		return map[string]any{"sent_on": nil, "dropped": []string{"telegram"}}, nil
+	}
 
 	publishAdded(t, r.nc, "bob", "INBOX", 1, []map[string]any{{"name": "Alice", "address": "alice@example.org"}}, "hi", time.Now())
 
@@ -160,7 +162,7 @@ func TestAcceptanceOneMatteringMailSendsExactlyOneTellNamingIt(t *testing.T) {
 	var tells recorded[map[string]any]
 	r.stand.Tell = func(args map[string]any) (any, error) {
 		tells.add(args)
-		return map[string]any{"sent_on": time.Now().UTC().Format(time.RFC3339)}, nil
+		return map[string]any{"sent_on": []string{"telegram"}, "dropped": []string{}}, nil
 	}
 
 	publishAdded(t, r.nc, "bob", "INBOX", 1, []map[string]any{{"name": "Alice", "address": "alice@example.org"}}, "invoice", time.Now())
@@ -208,7 +210,7 @@ func TestAcceptanceSenderNameWithAHostNameGivesTheSafeForm(t *testing.T) {
 	var tells recorded[map[string]any]
 	r.stand.Tell = func(args map[string]any) (any, error) {
 		tells.add(args)
-		return map[string]any{"sent_on": time.Now().UTC().Format(time.RFC3339)}, nil
+		return map[string]any{"sent_on": []string{"telegram"}, "dropped": []string{}}, nil
 	}
 
 	publishAdded(t, r.nc, "bob", "INBOX", 1, []map[string]any{{"name": "mail.evil.example.com", "address": "x@example.org"}}, "hi", time.Now())
@@ -233,9 +235,9 @@ func TestAcceptanceDroppedTellLeavesMailUntoldAndCountedNextTime(t *testing.T) {
 	var tellCalls count
 	r.stand.Tell = func(map[string]any) (any, error) {
 		if tellCalls.inc() == 1 {
-			return map[string]any{"sent_on": ""}, nil // dropped: the operator channel's own cap
+			return map[string]any{"sent_on": nil, "dropped": []string{"telegram"}}, nil // dropped: the operator channel's own cap
 		}
-		return map[string]any{"sent_on": time.Now().UTC().Format(time.RFC3339)}, nil
+		return map[string]any{"sent_on": []string{"telegram"}, "dropped": []string{}}, nil
 	}
 
 	publishAdded(t, r.nc, "bob", "INBOX", 1, []map[string]any{{"name": "Alice", "address": "alice@example.org"}}, "hi", time.Now())
@@ -269,3 +271,38 @@ func TestAcceptanceDroppedTellLeavesMailUntoldAndCountedNextTime(t *testing.T) {
 	})
 	assertNeverSendsMovesFlagsOrDeletes(t, r)
 }
+
+// 1926/AC4: a second batch over an unchanged set of mails that matter (its own mail does not matter) sends no
+// second signal — the first, answered as the real operator-channel answers it (sent_on a list), marked the set
+// told.
+func TestAcceptanceRepeatedBatchesOverAnUnchangedSetSendOneSignal(t *testing.T) {
+	r := startReader(t)
+	r.stand.MailAccounts = oneAccountAnswer("bob")
+	r.stand.MailRead = func(map[string]any) (any, error) { return map[string]any{"text": "hi"}, nil }
+	var judged count
+	matters := judgeEveryMailAs("matters", "", "", "")
+	neither := judgeEveryMailAs("neither", "", "", "")
+	r.stand.Judge = func(args map[string]any) (any, error) {
+		if judged.inc() == 1 {
+			return matters(args)
+		}
+		return neither(args)
+	}
+	r.stand.MessengerCheck = func(map[string]any) (any, error) { return map[string]any{"allowed": true}, nil }
+	var tells count
+	r.stand.Tell = func(map[string]any) (any, error) {
+		tells.inc()
+		return map[string]any{"sent_on": []string{"telegram"}, "dropped": []string{}}, nil
+	}
+
+	publishAdded(t, r.nc, "bob", "INBOX", 1, []map[string]any{{"name": "Alice", "address": "alice@example.org"}}, "hi", time.Now())
+	waitFor(t, 30*time.Second, "the first tell", func() bool { return tells.get() >= 1 })
+	publishAdded(t, r.nc, "bob", "INBOX", 2, []map[string]any{{"name": "Bob", "address": "bob2@example.org"}}, "second", time.Now())
+	publishAdded(t, r.nc, "bob", "INBOX", 3, []map[string]any{{"name": "Carol", "address": "carol@example.org"}}, "third", time.Now())
+	waitFor(t, BatchGap+60*time.Second, "later batches judged", func() bool { return judged.get() >= 2 })
+	time.Sleep(2 * BatchGap) // a stray repeat of the signal would come after a batch
+	if got := tells.get(); got != 1 {
+		t.Fatalf("1926/AC4: tells = %d, want exactly 1 for an unchanged set", got)
+	}
+	assertNeverSendsMovesFlagsOrDeletes(t, r)
+}

modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go

@@ -321,7 +321,7 @@ func RunBatches(ctx context.Context, store Store, readText ReadTextFunc, judge J
 		if err := RunOneBatch(ctx, store, readText, judge, waiting, now, logf); err != nil {
 			logf("[mesh-mail-reader] a batch failed: %v", err)
 		}
-		if err := SendSignal(ctx, store, check, tell); err != nil {
+		if err := SendSignal(ctx, store, check, tell, now); err != nil {
 			logf("[mesh-mail-reader] the mail signal could not be sent: %v", err)
 		}
 	}

modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go

@@ -58,9 +58,9 @@ func TestAcceptanceRefusedJudgementIsUnhealthyThenCatchesUpInPublishedOrder(t *t
 	var tellCalls count
 	r.stand.Tell = func(map[string]any) (any, error) {
 		if tellCalls.inc() == 1 {
-			return map[string]any{"sent_on": ""}, nil // dropped: the operator channel's own cap (1522/AC2)
+			return map[string]any{"sent_on": nil, "dropped": []string{"telegram"}}, nil // dropped: the operator channel's own cap (1522/AC2)
 		}
-		return map[string]any{"sent_on": time.Now().UTC().Format(time.RFC3339)}, nil
+		return map[string]any{"sent_on": []string{"telegram"}, "dropped": []string{}}, nil
 	}
 
 	var mu sync.Mutex

modules/mesh-mail-reader/cmd/mesh-mail-reader/tell.go

@@ -8,6 +8,8 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
+	"time"
 )
 
 // MessengerCheck is one seat:messenger.messenger_check ask (ADR 0461 §4): whether text is safe to say on an
@@ -44,15 +46,75 @@ func NewTell(ask Ask) Tell {
 			return "", err
 		}
 		var out struct {
-			SentOn string `json:"sent_on"`
+			SentOn json.RawMessage `json:"sent_on"`
 		}
 		if err := json.Unmarshal(raw, &out); err != nil {
 			return "", fmt.Errorf("operator-channel.tell's answer could not be read: %.200s", raw)
 		}
-		return out.SentOn, nil
+		sentOn, err := readSentOn(out.SentOn)
+		if err != nil {
+			return "", fmt.Errorf("operator-channel.tell's answer could not be read: %.200s", raw)
+		}
+		return sentOn, nil
 	}
 }
 
+// readSentOn reads tell's sent_on: messenger answers the list of channels that said it (null or empty when
+// every channel dropped it), never one string (issue 1926: reading it as a string failed every real answer, so
+// no mail was ever marked told and the same signal went out after every batch). A plain string is still
+// accepted. The channels are joined with a comma; empty means nothing said it.
+func readSentOn(raw json.RawMessage) (string, error) {
+	if len(raw) == 0 || string(raw) == "null" {
+		return "", nil
+	}
+	var list []string
+	if err := json.Unmarshal(raw, &list); err == nil {
+		return strings.Join(list, ","), nil
+	}
+	var one string
+	if err := json.Unmarshal(raw, &one); err != nil {
+		return "", err
+	}
+	return one, nil
+}
+
+// signalEntries is what the signal counts out of the entries marked matters and not yet told (issue 1926):
+// a mail whose due date lies before today is left out (AC2: it no longer matters, the mail page still holds
+// it); the same mail seen twice (one message id in two folders, issue 1849) and the same notice in two
+// languages (the same sender address, mail day and due date) count once (AC3). The entries left out stay
+// as they were; the ones that count are returned in the order given.
+func signalEntries(entries []Entry, now time.Time) []Entry {
+	today := time.Date(now.UTC().Year(), now.UTC().Month(), now.UTC().Day(), 0, 0, 0, 0, time.UTC)
+	seen := map[string]bool{}
+	var out []Entry
+	for _, e := range entries {
+		if !e.DueDate.IsZero() && e.DueDate.UTC().Before(today) {
+			continue
+		}
+		var keys []string
+		if e.MessageID != "" {
+			keys = append(keys, "id\x00"+e.Account+"\x00"+e.MessageID)
+		}
+		if !e.DueDate.IsZero() && e.SenderAddress != "" {
+			keys = append(keys, "notice\x00"+e.Account+"\x00"+strings.ToLower(e.SenderAddress)+"\x00"+
+				e.Date.UTC().Format("2006-01-02")+"\x00"+e.DueDate.UTC().Format("2006-01-02"))
+		}
+		dup := false
+		for _, k := range keys {
+			if seen[k] {
+				dup = true
+			}
+		}
+		for _, k := range keys {
+			seen[k] = true
+		}
+		if !dup {
+			out = append(out, e)
+		}
+	}
+	return out
+}
+
 // SendSignal is 1522: every entry marked matters and not yet told — including one carried over from an
 // earlier batch that went untold (1429/AC4) — is gathered; its words are built (signal.go) and checked word
 // by word with check; any one refused falls back to the safe form (1522/AC1). Nothing is sent, and nothing
@@ -61,7 +123,7 @@ func NewTell(ask Ask) Tell {
 // own last outcome. The tell sent, every one of those entries is marked told only once it answers sent_on
 // (1522/AC2); a tell that answers sent_on empty leaves them exactly as they were, for the next signal to
 // count (1522/AC2, AC3).
-func SendSignal(ctx context.Context, store Store, check MessengerCheck, tell Tell) error {
+func SendSignal(ctx context.Context, store Store, check MessengerCheck, tell Tell, now func() time.Time) error {
 	health, err := store.ReaderHealth(ctx)
 	if err != nil {
 		return err
@@ -73,17 +135,23 @@ func SendSignal(ctx context.Context, store Store, check MessengerCheck, tell Tel
 	if err != nil {
 		return err
 	}
-	var toTell []Entry
+	var untold []Entry
-	var mails []SignalMail
 	for _, e := range entries {
 		if e.Mark == MarkMatters && !e.Told {
-			toTell = append(toTell, e)
+			untold = append(untold, e)
-			mails = append(mails, SignalMail{SenderName: e.SenderName, Summary: e.Summary, Date: e.Date, DueDate: e.DueDate})
 		}
 	}
+	// What counts (signalEntries) is what is said; every untold entry is marked told once it is said, so an
+	// unchanged set is never said twice (issue 1926/AC1) and one that only holds past-due or duplicate mails
+	// says nothing.
+	toTell := signalEntries(untold, now())
 	if len(toTell) == 0 {
 		return nil
 	}
+	var mails []SignalMail
+	for _, e := range toTell {
+		mails = append(mails, SignalMail{SenderName: e.SenderName, Summary: e.Summary, Date: e.Date, DueDate: e.DueDate})
+	}
 
 	words := BuildSignal(mails)
 	allowed := true
@@ -108,7 +176,7 @@ func SendSignal(ctx context.Context, store Store, check MessengerCheck, tell Tel
 	if sentOn == "" {
 		return nil
 	}
-	for _, e := range toTell {
+	for _, e := range untold {
 		e.Told = true
 		if err := store.UpdateEntry(ctx, e); err != nil {
 			return err

modules/mesh-mail-reader/cmd/mesh-mail-reader/tell_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"encoding/json"
 	"testing"
 	"time"
 )
@@ -38,7 +39,7 @@ func TestSendSignalChecksWordsThenTellsWithSubjectMail(t *testing.T) {
 	check, asked := recordingCheck(true)
 	tell, headlines, subjects := recordingTell("2026-10-21T10:00:00Z")
 
-	if err := SendSignal(ctx, store, check, tell); err != nil {
+	if err := SendSignal(ctx, store, check, tell, time.Now); err != nil {
 		t.Fatal(err)
 	}
 	if len(*asked) == 0 || (*asked)[0] != "1 mail matters" {
@@ -64,7 +65,7 @@ func TestSendSignalFallsBackToTheSafeFormWhenRefused(t *testing.T) {
 	check, _ := recordingCheck(false)
 	tell, headlines, _ := recordingTell("sent")
 
-	if err := SendSignal(ctx, store, check, tell); err != nil {
+	if err := SendSignal(ctx, store, check, tell, time.Now); err != nil {
 		t.Fatal(err)
 	}
 	if len(*headlines) != 1 {
@@ -81,7 +82,7 @@ func TestSendSignalSentOnEmptyLeavesMailsUntold(t *testing.T) {
 	check, _ := recordingCheck(true)
 	tellDropped, calls, _ := recordingTell("") // every channel dropped it
 
-	if err := SendSignal(ctx, store, check, tellDropped); err != nil {
+	if err := SendSignal(ctx, store, check, tellDropped, time.Now); err != nil {
 		t.Fatal(err)
 	}
 	entry, _, _ := store.Entry(ctx, id)
@@ -94,7 +95,7 @@ func TestSendSignalSentOnEmptyLeavesMailsUntold(t *testing.T) {
 
 	// The next signal counts the still-untold mail (1429/AC4): SendSignal asks about it again.
 	tellSent, headlines, _ := recordingTell("sent-now")
-	if err := SendSignal(ctx, store, check, tellSent); err != nil {
+	if err := SendSignal(ctx, store, check, tellSent, time.Now); err != nil {
 		t.Fatal(err)
 	}
 	if len(*headlines) != 1 {
@@ -114,7 +115,7 @@ func TestSendSignalSendsNothingForScamWarningsAlone(t *testing.T) {
 	check, asked := recordingCheck(true)
 	tell, headlines, _ := recordingTell("sent")
 
-	if err := SendSignal(ctx, store, check, tell); err != nil {
+	if err := SendSignal(ctx, store, check, tell, time.Now); err != nil {
 		t.Fatal(err)
 	}
 	if len(*asked) != 0 || len(*headlines) != 0 {
@@ -132,7 +133,7 @@ func TestSendSignalSendsNothingWhileUnhealthy(t *testing.T) {
 	check, asked := recordingCheck(true)
 	tell, headlines, _ := recordingTell("sent")
 
-	if err := SendSignal(ctx, store, check, tell); err != nil {
+	if err := SendSignal(ctx, store, check, tell, time.Now); err != nil {
 		t.Fatal(err)
 	}
 	if len(*asked) != 0 || len(*headlines) != 0 {
@@ -141,7 +142,7 @@ func TestSendSignalSendsNothingWhileUnhealthy(t *testing.T) {
 
 	// After recovery, the first signal counts the mail that went untold while unhealthy.
 	store.SetReaderHealth(ctx, ReaderHealth{Healthy: true})
-	if err := SendSignal(ctx, store, check, tell); err != nil {
+	if err := SendSignal(ctx, store, check, tell, time.Now); err != nil {
 		t.Fatal(err)
 	}
 	if len(*headlines) != 1 {
@@ -166,3 +167,120 @@ func TestTheManifestInvokesTellAndMessengerCheck(t *testing.T) {
 		}
 	}
 }
+
+var signalNow = func() time.Time { return time.Date(2026, 10, 23, 21, 0, 0, 0, time.UTC) }
+
+// 1926/AC1: the real operator-channel.tell answers sent_on as the list of channels that said it (messenger's
+// own shape); read as a string it failed, nothing was marked told and every batch sent the same signal again.
+func TestNewTellReadsSentOnAsMessengerAnswersIt(t *testing.T) {
+	for name, answer := range map[string]string{
+		"a list":   `{"key":"tell.1","sent_on":["telegram"],"dropped":[]}`,
+		"a string": `{"sent_on":"telegram"}`,
+	} {
+		tell := NewTell(func(string, map[string]any) (json.RawMessage, error) { return json.RawMessage(answer), nil })
+		sentOn, err := tell("h", nil, "mail")
+		if err != nil || sentOn == "" {
+			t.Fatalf("1926/AC1: %s: sentOn = %q, err = %v, want a channel", name, sentOn, err)
+		}
+	}
+	for name, answer := range map[string]string{
+		"null":       `{"sent_on":null,"dropped":["telegram"]}`,
+		"empty list": `{"sent_on":[],"dropped":["telegram"]}`,
+		"absent":     `{"dropped":["telegram"]}`,
+	} {
+		tell := NewTell(func(string, map[string]any) (json.RawMessage, error) { return json.RawMessage(answer), nil })
+		sentOn, err := tell("h", nil, "mail")
+		if err != nil || sentOn != "" {
+			t.Fatalf("1926/AC1: %s: sentOn = %q, err = %v, want empty and no error", name, sentOn, err)
+		}
+	}
+}
+
+// 1926/AC1: a set already told is not sent again; a mail added to it is sent alone.
+func TestSendSignalSendsAnUnchangedSetOnce(t *testing.T) {
+	ctx := context.Background()
+	store := newMemStore()
+	for i := 0; i < 3; i++ {
+		store.AddEntry(ctx, mattersEntry("Alice", "pay the bill"))
+	}
+	check, _ := recordingCheck(true)
+	tell, headlines, _ := recordingTell("telegram")
+	for i := 0; i < 3; i++ {
+		if err := SendSignal(ctx, store, check, tell, signalNow); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if len(*headlines) != 1 || (*headlines)[0] != "3 mails matter" {
+		t.Fatalf("1926/AC1: tell headlines = %v, want one \"3 mails matter\"", *headlines)
+	}
+	store.AddEntry(ctx, mattersEntry("Bob", "new"))
+	if err := SendSignal(ctx, store, check, tell, signalNow); err != nil {
+		t.Fatal(err)
+	}
+	if len(*headlines) != 2 || (*headlines)[1] != "1 mail matters" {
+		t.Fatalf("1926/AC1: tell headlines = %v, want the changed set sent again", *headlines)
+	}
+}
+
+// 1926/AC2: a mail whose due date has passed is not counted and does not lead; one due today still leads.
+func TestSendSignalLeavesPastDueMailsOut(t *testing.T) {
+	ctx := context.Background()
+	store := newMemStore()
+	past := mattersEntry("bpost", "parcel notice")
+	past.DueDate = time.Date(2026, 10, 22, 0, 0, 0, 0, time.UTC)
+	today := mattersEntry("Alice", "pay today")
+	today.DueDate = time.Date(2026, 10, 23, 0, 0, 0, 0, time.UTC)
+	store.AddEntry(ctx, past)
+	store.AddEntry(ctx, today)
+	check, _ := recordingCheck(true)
+	var headlines []string
+	var lines [][]string
+	tell := func(h string, l []string, _ string) (string, error) {
+		headlines, lines = append(headlines, h), append(lines, l)
+		return "telegram", nil
+	}
+	if err := SendSignal(ctx, store, check, tell, signalNow); err != nil {
+		t.Fatal(err)
+	}
+	if len(headlines) != 1 || headlines[0] != "1 mail matters" || len(lines[0]) != 1 || lines[0][0] != "Alice — pay today — due 2026-10-23" {
+		t.Fatalf("1926/AC2: told %v %v, want only Alice's mail due today", headlines, lines)
+	}
+
+	// A set holding only past-due mails says nothing.
+	store2 := newMemStore()
+	store2.AddEntry(ctx, past)
+	headlines = nil
+	if err := SendSignal(ctx, store2, check, tell, signalNow); err != nil {
+		t.Fatal(err)
+	}
+	if len(headlines) != 0 {
+		t.Fatalf("1926/AC2: told %v for past-due mails alone, want nothing", headlines)
+	}
+}
+
+// 1926/AC3: the same notice in two languages (same sender, same day, same due date) counts once, and so does
+// the same message id in two folders (issue 1849); two different mails of one sender stay two.
+func TestSendSignalCountsADuplicateNoticeOnce(t *testing.T) {
+	ctx := context.Background()
+	store := newMemStore()
+	due := time.Date(2026, 10, 30, 0, 0, 0, 0, time.UTC)
+	day := time.Date(2026, 10, 21, 8, 0, 0, 0, time.UTC)
+	en := Entry{Account: "bob", MessageID: "<a>", SenderName: "bpost", SenderAddress: "no-reply@bpost.example", Summary: "parcel waiting", Mark: MarkMatters, Date: day, DueDate: due}
+	nl := en
+	nl.MessageID, nl.Summary, nl.Date = "<b>", "pakje wacht", day.Add(time.Minute)
+	inbox := Entry{Account: "bob", MessageID: "<c>", Folder: "INBOX", SenderName: "Carol", SenderAddress: "carol@x.example", Summary: "hello", Mark: MarkMatters, Date: day}
+	allMail := inbox
+	allMail.Folder = "All Mail"
+	other := Entry{Account: "bob", MessageID: "<d>", SenderName: "Carol", SenderAddress: "carol@x.example", Summary: "second", Mark: MarkMatters, Date: day}
+	for _, e := range []Entry{en, nl, inbox, allMail, other} {
+		store.AddEntry(ctx, e)
+	}
+	check, _ := recordingCheck(true)
+	tell, headlines, _ := recordingTell("telegram")
+	if err := SendSignal(ctx, store, check, tell, signalNow); err != nil {
+		t.Fatal(err)
+	}
+	if len(*headlines) != 1 || (*headlines)[0] != "3 mails matter" {
+		t.Fatalf("1926/AC3: tell headlines = %v, want \"3 mails matter\"", *headlines)
+	}
+}