From 753e8fc202c32408257b80a21a7cd294ba14713c Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 23:27:23 +0200 Subject: [PATCH] mesh-mail-reader: read tell's sent_on as the list it is, so a told set is not sent again; leave past-due and duplicate mails out (issue 1926) --- .../acceptance_integration_test.go | 47 ++++++- .../cmd/mesh-mail-reader/batch.go | 2 +- .../catchup_integration_test.go | 4 +- .../cmd/mesh-mail-reader/tell.go | 84 +++++++++-- .../cmd/mesh-mail-reader/tell_test.go | 132 +++++++++++++++++- 5 files changed, 246 insertions(+), 23 deletions(-) diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/acceptance_integration_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/acceptance_integration_test.go index ca013fff..aa8ebf4f 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/acceptance_integration_test.go +++ b/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) +} diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go index 99693fa4..04e3eb9d 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go +++ b/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) } } diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go index b81cb62b..665114f8 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go +++ b/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 diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/tell.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/tell.go index 90bcc2e4..77463628 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/tell.go +++ b/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 mails []SignalMail + var untold []Entry for _, e := range entries { if e.Mark == MarkMatters && !e.Told { - toTell = append(toTell, e) - mails = append(mails, SignalMail{SenderName: e.SenderName, Summary: e.Summary, Date: e.Date, DueDate: e.DueDate}) + untold = append(untold, e) } } + // 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 diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/tell_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/tell_test.go index 707c7f3f..3ecf24f5 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/tell_test.go +++ b/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: "", SenderName: "bpost", SenderAddress: "no-reply@bpost.example", Summary: "parcel waiting", Mark: MarkMatters, Date: day, DueDate: due} + nl := en + nl.MessageID, nl.Summary, nl.Date = "", "pakje wacht", day.Add(time.Minute) + inbox := Entry{Account: "bob", MessageID: "", 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: "", 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) + } +}