From 6c10288f0b182a0095094a8816bfd4d25c565702 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 17:16:53 +0200 Subject: [PATCH 01/10] mesh-mail declares the mail seats and claims mail-store (issue 800 part A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declares mail-store and mail-webclient (held once for the mesh) and mail-notifier (held once per machine, needing node-notifier) per ADR 0378 as amended by ADR 0379. Claims mail-store and serves its four verbs through the same handlers as the existing mail_* tools. Moves the event shape (EventKey, Changes) into its own file, mailevent.go, so mesh-mail-notifier (issue 800 part B) can carry a byte-identical copy. Adds checks/mailstore, refusing any manifest but mesh-mail's own that claims mail-store (ADR 0378 §3), wired into merge-check.sh. --- checks/mailstore/go.mod | 3 + checks/mailstore/main.go | 81 +++++++++++++++++++ checks/mailstore/main_test.go | 43 ++++++++++ merge-check.sh | 3 + modules/mesh-mail/README.md | 16 ++++ modules/mesh-mail/cmd/mesh-mail/imap.go | 8 +- modules/mesh-mail/cmd/mesh-mail/mailevent.go | 13 +++ .../mesh-mail/cmd/mesh-mail/manifest_test.go | 49 ++++++++--- modules/mesh-mail/cmd/mesh-mail/tools.go | 42 ++++++++++ modules/mesh-mail/module.json | 31 ++++++- 10 files changed, 272 insertions(+), 17 deletions(-) create mode 100644 checks/mailstore/go.mod create mode 100644 checks/mailstore/main.go create mode 100644 checks/mailstore/main_test.go create mode 100644 modules/mesh-mail/cmd/mesh-mail/mailevent.go diff --git a/checks/mailstore/go.mod b/checks/mailstore/go.mod new file mode 100644 index 00000000..0d161eef --- /dev/null +++ b/checks/mailstore/go.mod @@ -0,0 +1,3 @@ +module git.novox.be/novox/mesh-catalog/checks/mailstore + +go 1.22 diff --git a/checks/mailstore/main.go b/checks/mailstore/main.go new file mode 100644 index 00000000..589d49dd --- /dev/null +++ b/checks/mailstore/main.go @@ -0,0 +1,81 @@ +// Command mailstore refuses any manifest but modules/mesh-mail claiming the mail-store seat (novox/hq ADR 0378 +// §3, issue 800 part A): the operator's mail is read, searched and sent through this one seat, and a second +// claimant would be a second path to the same mail that ADR 0378 never accounted for. +// +// go run . +// +// Exits 1 on a finding. +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" +) + +// MailStoreSeat is the seat only mesh-mail may claim. +const MailStoreSeat = "mail-store" + +// MailStoreModule is the one module allowed to claim it. +const MailStoreModule = "mesh-mail" + +// Finding is one manifest refused: a module other than mesh-mail claiming mail-store. +type Finding struct { + Module string +} + +// Check reads every manifest under root/modules and answers which ones it refuses. +func Check(root string) ([]Finding, error) { + manifests, err := filepath.Glob(filepath.Join(root, "modules", "*", "module.json")) + if err != nil { + return nil, err + } + sort.Strings(manifests) + var out []Finding + for _, path := range manifests { + raw, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var m struct { + Module string `json:"module"` + Claims []struct { + Name string `json:"name"` + } `json:"claims"` + } + if err := json.Unmarshal(raw, &m); err != nil { + return nil, fmt.Errorf("%s: %w", path, err) + } + if m.Module == MailStoreModule { + continue + } + for _, c := range m.Claims { + if c.Name == MailStoreSeat { + out = append(out, Finding{Module: m.Module}) + break + } + } + } + return out, nil +} + +func main() { + root := "." + if len(os.Args) > 1 { + root = os.Args[1] + } + found, err := Check(root) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + for _, f := range found { + fmt.Printf("%s claims the mail-store seat: only %s may (novox/hq ADR 0378 §3)\n", f.Module, MailStoreModule) + } + if len(found) > 0 { + os.Exit(1) + } + fmt.Println("mail-store: claimed only by mesh-mail") +} diff --git a/checks/mailstore/main_test.go b/checks/mailstore/main_test.go new file mode 100644 index 00000000..e33470da --- /dev/null +++ b/checks/mailstore/main_test.go @@ -0,0 +1,43 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func manifest(t *testing.T, root, module, claims string) { + t.Helper() + dir := filepath.Join(root, "modules", module) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + body := `{"module":"` + module + `","claims":[` + claims + `]}` + if err := os.WriteFile(filepath.Join(dir, "module.json"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestAnotherModuleClaimingMailStoreIsRefused(t *testing.T) { + root := t.TempDir() + manifest(t, root, "mesh-mail", `{"name":"mail-store","scope":"mesh"}`) + manifest(t, root, "betterbird", `{"name":"mail-store","scope":"mesh"}`) + manifest(t, root, "dunst", `{"name":"node-notifier","scope":"node"}`) + found, err := Check(root) + if err != nil { + t.Fatal(err) + } + if len(found) != 1 || found[0].Module != "betterbird" { + t.Fatalf("refused %+v", found) + } +} + +func TestTheCatalogueHasNone(t *testing.T) { + found, err := Check("../..") + if err != nil { + t.Fatal(err) + } + if len(found) != 0 { + t.Fatalf("%+v", found) + } +} diff --git a/merge-check.sh b/merge-check.sh index 59f73b1e..549c22b0 100755 --- a/merge-check.sh +++ b/merge-check.sh @@ -33,11 +33,14 @@ # 6. the secrets (novox/hq issue 342): no mergeable file fills a key with a vault secret that a setting could # replace — the key is `protected`, or left out of the file's `takes-settings`. checks/secrets/known names # the nested ones the controller cannot protect alone yet, and only shrinks. +# 7. the mail-store seat (novox/hq ADR 0378 §3, issue 800): no manifest but modules/mesh-mail claims it. set -eu (cd checks/words && go test -count=1 . && go run . ../..) # The agent's home (novox/hq ADR 0266): no archive, and no file written into, under it. (cd checks/agenthome && go test -count=1 . && go run . ../..) +# The mail-store seat (novox/hq ADR 0378 §3): only mesh-mail may claim it. +(cd checks/mailstore && go test -count=1 . && go run . ../..) # The secrets (novox/hq issue 342): a password is not a setting, so no setting can replace the vault's. The # known list only shrinks: judged against the trunk's copy at the merge base, a line it does not hold is refused. secrets_base="" diff --git a/modules/mesh-mail/README.md b/modules/mesh-mail/README.md index c5104514..51d133d9 100644 --- a/modules/mesh-mail/README.md +++ b/modules/mesh-mail/README.md @@ -6,6 +6,18 @@ Reading is the first stage: accounts of the kind `imap`, read-only — nothing h Sending is `mail_send` alone (novox/hq ADR 0361 §2): it sends nothing itself, and reaches an account's outgoing server only once the operator approves it on a channel that proves who answered. +## Its seats (novox/hq ADR 0378 as amended by ADR 0379, issue 800 part A) + +It declares three mail seats and claims the one it holds: + +- **mail-store** (held once for the mesh) — serves `accounts`, `search`, `read` and `send`; mesh-mail claims it, + through the same handlers as its own `mail_*` tools. The catalogue's own check (`checks/mailstore`) refuses any + other manifest that claims it. +- **mail-webclient** (held once for the mesh) — serves `open` and `status`; unheld until issue 580. +- **mail-notifier** (held once per machine, needing `node-notifier` there) — serves `status`; held by + mesh-mail-notifier (issue 800 part B), a module of its own built from this one's source, which tells the + operator of each new inbox mail through `node-notifier.send`. + ## What it does - **Watches.** For each account it keeps one connection: IDLE on the inbox, ended and begun again every two minutes @@ -59,6 +71,10 @@ The caller is the runtime's word, never an argument. `mail_send` sends through t once the operator's Approve comes back on a channel that proves who answered, and once: a warrant delivered twice never sends twice (novox/hq ADR 0361 §2). +The mail-store seat's four verbs, `mail-store.accounts`, `mail-store.search`, `mail-store.read` and +`mail-store.send`, answer through the same handlers as `mail_accounts`, `mail_search`, `mail_read` and `mail_send` +above — the same grants, the same refusals, the same ask-then-Approve for a send. + ## Its processes `prepare` brings the index's schema to this version's (numbered migrations, and `schema.sql` for a fresh database). diff --git a/modules/mesh-mail/cmd/mesh-mail/imap.go b/modules/mesh-mail/cmd/mesh-mail/imap.go index 5583781c..5488e535 100644 --- a/modules/mesh-mail/cmd/mesh-mail/imap.go +++ b/modules/mesh-mail/cmd/mesh-mail/imap.go @@ -537,13 +537,7 @@ func sameStrings(a, b []string) bool { } // ---- the events --------------------------------------------------------------------------------------------------- - -// EventKey is where an account's change is said: mail..message. (research 049 -// document 02 §5), the account by its name in the setting, never its address. -func EventKey(account, change string) string { return "mail." + account + ".message." + change } - -// Changes are the three things said of a message. -var Changes = []string{"added", "changed", "removed"} +// EventKey and Changes, the event's shape, now live in mailevent.go of their own (issue 800 part A). // metadata is what an event says of a message: never its body, never an attachment's bytes (ADR 0359 §2). func metadata(m Message) map[string]any { diff --git a/modules/mesh-mail/cmd/mesh-mail/mailevent.go b/modules/mesh-mail/cmd/mesh-mail/mailevent.go new file mode 100644 index 00000000..fb9596fb --- /dev/null +++ b/modules/mesh-mail/cmd/mesh-mail/mailevent.go @@ -0,0 +1,13 @@ +package main + +// The shape of a mail-store message event (novox/hq ADR 0378 §2, issue 800 part A), moved here on its own so +// mesh-mail-notifier (issue 800 part B) carries a byte-identical copy, the way modules/networkmanager's +// uplink-tools carries ifwatchd's own file (novox/hq ADR 0241, copies_test.go). Nothing here reaches a mailbox or +// a server; it only says where an account's change is told on the bus and what the three changes are called. + +// EventKey is where an account's change is said: mail..message. (research 049 +// document 02 §5), the account by its name in the setting, never its address. +func EventKey(account, change string) string { return "mail." + account + ".message." + change } + +// Changes are the three things said of a message. +var Changes = []string{"added", "changed", "removed"} diff --git a/modules/mesh-mail/cmd/mesh-mail/manifest_test.go b/modules/mesh-mail/cmd/mesh-mail/manifest_test.go index d6563479..5f0a2c9a 100644 --- a/modules/mesh-mail/cmd/mesh-mail/manifest_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/manifest_test.go @@ -11,11 +11,20 @@ import ( // The manifest says what the code does (novox/hq ADR 0359): the database and the bucket it requires (§3), the own // secret family issued outside the mesh (§5, ADR 0283 §6), the two trusted settings (§4), the tools (§5), the events, -// and its two long-running processes with their health (§6) — and it is assigned to no machine by itself. +// its two long-running processes with their health (§6), and — ADR 0378 as amended by ADR 0379, issue 800 part A — +// the three mail seats it declares and the one, mail-store, it claims. It is assigned to no machine by itself. + +type seat struct { + Name string `json:"name"` + Scope string `json:"scope"` + Serves []string `json:"serves"` + Needs string `json:"needs"` +} type manifest struct { Module string `json:"module"` Slug string `json:"slug"` + Seats []seat `json:"seats"` Requires []string `json:"requires"` Contributes map[string]map[string]any `json:"contributes"` OwnSecrets map[string]map[string]string `json:"own-secrets"` @@ -30,10 +39,10 @@ type manifest struct { Events string `json:"events"` } `json:"grant"` } `json:"streams"` - Consumes []string `json:"consumes"` - Invokes []string `json:"invokes"` - Claims []json.RawMessage `json:"claims"` - Resources []map[string]any `json:"resources"` + Consumes []string `json:"consumes"` + Invokes []string `json:"invokes"` + Claims []seat `json:"claims"` + Resources []map[string]any `json:"resources"` Build struct { Artifacts []struct { From string `json:"from"` @@ -78,7 +87,11 @@ func TestTheManifestSaysWhatTheCodeDoes(t *testing.T) { for _, tool := range (&Bundle{}).Tools() { names = append(names, tool.Name) } - if !reflect.DeepEqual(m.Tools, names) || !reflect.DeepEqual(names, []string{"mail_accounts", "mail_search", "mail_read", "mail_health", "mail_send", "mail_send_status"}) { + wantTools := []string{ + "mail_accounts", "mail_search", "mail_read", "mail_health", "mail_send", "mail_send_status", + "mail-store.accounts", "mail-store.search", "mail-store.read", "mail-store.send", + } + if !reflect.DeepEqual(m.Tools, names) || !reflect.DeepEqual(names, wantTools) { t.Fatalf("tools %v, the code serves %v", m.Tools, names) } // Its events go to its own stream MAIL, kept a week, each account's part heard only by the consumers the trusted @@ -91,9 +104,27 @@ func TestTheManifestSaysWhatTheCodeDoes(t *testing.T) { m.Streams[0].MaxAgeDays != 7 || m.Streams[0].Grant.Config != "grants" || m.Streams[0].Grant.Events != "mail.{key}.**" { t.Fatalf("emits %v, streams %+v; the code says %v", m.Emits, m.Streams, events) } - // It reads its own mail and calls nothing, consumes nothing and claims no seat. - if len(m.Consumes)+len(m.Invokes)+len(m.Claims) != 0 { - t.Fatalf("consumes %v, invokes %v, claims %v", m.Consumes, m.Invokes, m.Claims) + // It calls nothing and consumes nothing: mesh-mail-notifier (issue 800 part B) is the one that invokes and + // consumes, not the store. + if len(m.Consumes)+len(m.Invokes) != 0 { + t.Fatalf("consumes %v, invokes %v", m.Consumes, m.Invokes) + } + // It declares exactly the three mail seats (ADR 0378 §1 as amended by ADR 0379 §4) — mail-store and + // mail-webclient each held once for the mesh, mail-notifier held once per machine and needing node-notifier + // there — and claims only mail-store, serving the same four verbs as its own tools (§2, §3). + wantSeats := []seat{ + {Name: "mail-store", Scope: "mesh", Serves: []string{"accounts", "search", "read", "send"}}, + {Name: "mail-webclient", Scope: "mesh", Serves: []string{"open", "status"}}, + {Name: "mail-notifier", Scope: "node", Serves: []string{"status"}, Needs: "node-notifier"}, + } + if !reflect.DeepEqual(m.Seats, wantSeats) { + t.Fatalf("seats %+v, want %+v", m.Seats, wantSeats) + } + wantClaims := []seat{ + {Name: "mail-store", Scope: "mesh", Serves: []string{"accounts", "search", "read", "send"}}, + } + if !reflect.DeepEqual(m.Claims, wantClaims) { + t.Fatalf("claims %+v, want %+v", m.Claims, wantClaims) } // The two settings are trusted, and their defaults are an empty account list and no grant. for id, want := range map[string]string{"accounts": `{"accounts":{}}`, "grants": `{"grants":{}}`} { diff --git a/modules/mesh-mail/cmd/mesh-mail/tools.go b/modules/mesh-mail/cmd/mesh-mail/tools.go index 3f99c806..b957f0c0 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools.go @@ -26,6 +26,16 @@ const ( SendStatusTool = "mail_send_status" ) +// Seat tool names: mesh-mail claims the mail-store seat (novox/hq ADR 0378 §2, §3; issue 800 part A) and serves +// its verbs through the same handlers as the mail_* tools above — the seat's name is the one a caller grants +// reach the same account's mail through, whichever name it calls. +const ( + SeatAccountsTool = "mail-store.accounts" + SeatSearchTool = "mail-store.search" + SeatReadTool = "mail-store.read" + SeatSendTool = "mail-store.send" +) + // Bounds of what a tool answers. const ( MostResults = 50 @@ -153,6 +163,38 @@ func (b *Bundle) Tools() []stdio.Tool { "to the request mail_send asked (so a warrant for another request, or one changed since, sends nothing here).", Input: schema([]string{"id"}, map[string]any{"id": text("the ask id mail_send gave")}), Run: b.sendStatusTool}, + {Name: SeatAccountsTool, + Description: "The mail-store seat's accounts verb (novox/hq ADR 0378 §2): the same as mail_accounts.", + Input: schema(nil, map[string]any{}), + Run: b.accountsTool}, + {Name: SeatSearchTool, + Description: "The mail-store seat's search verb (novox/hq ADR 0378 §2): the same as mail_search.", + Input: schema([]string{"query"}, map[string]any{ + "query": text("the words to find, at most 200 characters"), + "account": text("one account by its name in mail_accounts; every account you are granted when left out"), + "folder": text("one folder, such as INBOX; every folder when left out"), + "limit": integer("how many to answer, at most 50 (20 when left out)"), + }), + Run: b.searchTool}, + {Name: SeatReadTool, + Description: "The mail-store seat's read verb (novox/hq ADR 0378 §2): the same as mail_read.", + Input: schema([]string{"account", "folder", "uid"}, map[string]any{ + "account": text("the account, by its name in mail_accounts"), + "folder": text("the folder, such as INBOX"), + "uid": integer("the message's uid in that folder"), + "attachment": text("an attachment's key, the SHA-256 the message's references name, to answer its bytes"), + }), + Run: b.readTool}, + {Name: SeatSendTool, + Description: "The mail-store seat's send verb (novox/hq ADR 0378 §2): the same as mail_send.", + Input: schema([]string{"account", "to", "subject", "body"}, map[string]any{ + "account": text("the account to send from, by its name in mail_accounts"), + "to": map[string]any{"type": "array", "items": map[string]any{"type": "string"}, + "description": "the recipients' mail addresses, at most 20"}, + "subject": text("the message's subject, at most 200 characters"), + "body": text("the message's plain-text body"), + }), + Run: b.sendTool}, } } diff --git a/modules/mesh-mail/module.json b/modules/mesh-mail/module.json index 5969c5df..3f3bb3f9 100644 --- a/modules/mesh-mail/module.json +++ b/modules/mesh-mail/module.json @@ -2,6 +2,31 @@ "module": "mesh-mail", "version": "1", "slug": "mail", + "seats": [ + { + "name": "mail-store", + "scope": "mesh", + "serves": ["accounts", "search", "read", "send"] + }, + { + "name": "mail-webclient", + "scope": "mesh", + "serves": ["open", "status"] + }, + { + "name": "mail-notifier", + "scope": "node", + "needs": "node-notifier", + "serves": ["status"] + } + ], + "claims": [ + { + "name": "mail-store", + "scope": "mesh", + "serves": ["accounts", "search", "read", "send"] + } + ], "requires": [ "postgres-database", "s3-bucket" @@ -32,7 +57,11 @@ "mail_read", "mail_health", "mail_send", - "mail_send_status" + "mail_send_status", + "mail-store.accounts", + "mail-store.search", + "mail-store.read", + "mail-store.send" ], "uses": [ "operator-channel" -- 2.54.0 From dc89995d96d2b1ec13bc4c9a7b8b7165bd10eeed Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 17:33:15 +0200 Subject: [PATCH 02/10] Add mesh-mail-notifier, the default holder of mail-notifier (issue 800 part B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0378 §9 as amended by ADR 0379: a module of its own, built from mesh-mail's event shape, runs as its own account to hear mesh-mail's restricted MAIL stream (ADR 0376 §2), shows each new inbox mail through node-notifier.send at most once, folds more than three from one account into one notice, and emits its own shown event. --- modules/mesh-mail-notifier/README.md | 35 ++ .../cmd/mesh-mail-notifier/copies_test.go | 31 ++ .../cmd/mesh-mail-notifier/mailevent.go | 13 + .../cmd/mesh-mail-notifier/main.go | 167 ++++++++++ .../cmd/mesh-mail-notifier/notifier.go | 307 ++++++++++++++++++ .../cmd/mesh-mail-notifier/notifier_test.go | 169 ++++++++++ modules/mesh-mail-notifier/go.mod | 5 + modules/mesh-mail-notifier/go.sum | 2 + modules/mesh-mail-notifier/module.json | 79 +++++ 9 files changed, 808 insertions(+) create mode 100644 modules/mesh-mail-notifier/README.md create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/copies_test.go create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/mailevent.go create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go create mode 100644 modules/mesh-mail-notifier/go.mod create mode 100644 modules/mesh-mail-notifier/go.sum create mode 100644 modules/mesh-mail-notifier/module.json diff --git a/modules/mesh-mail-notifier/README.md b/modules/mesh-mail-notifier/README.md new file mode 100644 index 00000000..1f82055c --- /dev/null +++ b/modules/mesh-mail-notifier/README.md @@ -0,0 +1,35 @@ +# mesh-mail-notifier — the default holder of mail-notifier + +Built from mesh-mail's source (novox/hq ADR 0378 §9, as amended by ADR 0379, issue 800 part B): it carries a +byte-identical copy of mesh-mail's event shape (`mailevent.go`, pinned by `copies_test.go`, the way +`modules/networkmanager`'s `uplink-tools` carries `ifwatchd`'s own file, ADR 0241). Nothing here names +Betterbird (ADR 0379 §3), and nothing here reaches a mailbox: it only hears what mesh-mail already said. + +## What it does + +It claims `mail-notifier` (held once per machine, needing `node-notifier` there) and shows each new inbox mail +on this machine's notifier, through `node-notifier.send`, at most once: + +- quiet during a first sync or a resync (mesh-mail already says nothing then, `imap.go` ~387) +- quiet for a mail from the account's own address (its address, from `mail-store.accounts`; not known, the + notice is shown) +- quiet for a mail already marked read (`\Seen` in its flags) +- quiet for anything outside the inbox +- more than three new mails from one account arriving close together become one notice naming the count and + the latest sender, rather than one notice each + +Each notice shown emits this module's own event `shown` (account and message id, never sender or subject), and +is kept in its own state (`shown`) so a redelivered event is not shown twice. + +## Where it runs + +**Its own account** (`runs-as`), because it hears mesh-mail's restricted stream `MAIL` (novox/hq ADR 0376 §2): +the machine's runtime runs as the operator's account, which every agent shares and which never hears a +restricted stream. Held once per machine (wherever `mail-notifier` is assigned); the operator grants it the +accounts it announces, in mesh-mail's trusted setting `grants` (`mail-store`'s stream). + +## Tools + +| tool | does | +|---|---| +| `mail-notifier.status` | the accounts it announces on this machine, and when it last showed a notice; its rules. Read-only | diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/copies_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/copies_test.go new file mode 100644 index 00000000..eb4ff641 --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/copies_test.go @@ -0,0 +1,31 @@ +package main + +// mesh-mail-notifier carries a byte-identical copy of mesh-mail's event shape (novox/hq ADR 0378 §9, issue 800 +// part B), the way modules/networkmanager's uplink-tools carries ifwatchd's own file (ADR 0241, copies_test.go): +// each builds alone, so each holds its own copy, and this test holds them to one text. + +import ( + "bytes" + "os" + "path/filepath" + "testing" +) + +// source is where mesh-mail's own copy of the file lives. +var source = filepath.Join("..", "..", "..", "mesh-mail", "cmd", "mesh-mail") + +func TestCarriesTheSameMailEventAsMeshMail(t *testing.T) { + for _, f := range []string{"mailevent.go"} { + mine, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + theirs, err := os.ReadFile(filepath.Join(source, f)) + if err != nil { + t.Fatalf("mesh-mail's copy of %s could not be read: %v", f, err) + } + if !bytes.Equal(mine, theirs) { + t.Errorf("this copy of %s differs from mesh-mail's: change both together", f) + } + } +} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/mailevent.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/mailevent.go new file mode 100644 index 00000000..fb9596fb --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/mailevent.go @@ -0,0 +1,13 @@ +package main + +// The shape of a mail-store message event (novox/hq ADR 0378 §2, issue 800 part A), moved here on its own so +// mesh-mail-notifier (issue 800 part B) carries a byte-identical copy, the way modules/networkmanager's +// uplink-tools carries ifwatchd's own file (novox/hq ADR 0241, copies_test.go). Nothing here reaches a mailbox or +// a server; it only says where an account's change is told on the bus and what the three changes are called. + +// EventKey is where an account's change is said: mail..message. (research 049 +// document 02 §5), the account by its name in the setting, never its address. +func EventKey(account, change string) string { return "mail." + account + ".message." + change } + +// Changes are the three things said of a message. +var Changes = []string{"added", "changed", "removed"} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go new file mode 100644 index 00000000..bf74727d --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go @@ -0,0 +1,167 @@ +// mesh-mail-notifier: the default claimant of mail-notifier, built from mesh-mail's own event shape (novox/hq +// ADR 0378 §9, issue 800 part B). A Go bundle the node's runtime launches, its own account (runs-as, ADR 0376 +// §2): the base invocation serves mail-notifier.status; a separate process (`notify`) hears mesh-mail's added +// events and shows them. What this module says, it says on stderr; stdout is the MCP channel. +package main + +import ( + "encoding/json" + "fmt" + "os" + "strings" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" +) + +func logf(format string, a ...any) { fmt.Fprintf(os.Stderr, format+"\n", a...) } + +// AddedPattern is the mesh-mail stream's event this module consumes (its own `consumes` entry, novox/hq ADR +// 0363, ADR 0378 §9): a new, changed or removed message's `added` only. +const AddedPattern = "mesh-mail.mail.*.message.added" + +// StatusTool is the mail-notifier seat's one verb this module serves. +const StatusTool = "mail-notifier.status" + +// ShownEvent is this module's own event (its `emits`), one per notice shown. +const ShownEvent = "shown" + +func main() { + n := &Notifier{ + Send: func(args map[string]any) error { + _, err := stdio.Ask("seat:node-notifier.send", args) + return err + }, + Accounts: readAccounts, + Store: busStore{}, + EmitShown: func(account, messageID string) error { + return stdio.Emit(ShownEvent, map[string]any{"account": account, "message-id": messageID}) + }, + Logf: logf, + } + n.init() + + if len(os.Args) > 1 && os.Args[1] == "notify" { + go watch(n) + if err := stdio.Serve("", nil); err != nil { + logf("%v", err) + os.Exit(1) + } + return + } + + tools := []stdio.Tool{{Name: StatusTool, + Description: "The accounts mesh-mail-notifier announces on this machine, and when it last showed a " + + "notice of new mail (novox/hq ADR 0378 §7). Read-only.", + Run: func(map[string]any) (any, error) { return n.Status() }}} + if err := stdio.Serve("", tools); err != nil { + logf("%v", err) + os.Exit(1) + } +} + +// watch hears mesh-mail's added events and flushes what it buffered once each account has been quiet a +// moment (ADR 0378 §9: folding a burst into one notice). +func watch(n *Notifier) { + time.Sleep(500 * time.Millisecond) // Serve first: the bus is reached through it + err := stdio.Subscribe(AddedPattern, func(e stdio.Envelope) error { + var body EventBody + if err := json.Unmarshal(e.Body, &body); err != nil { + logf("[mesh-mail-notifier] an added event could not be read; taken and dropped: %v", err) + return nil + } + return n.Add(body) + }) + if err != nil { + logf("[mesh-mail-notifier] could not hear %s: %v", AddedPattern, err) + os.Exit(1) + } + ticker := time.NewTicker(1 * time.Second) + defer ticker.Stop() + for range ticker.C { + if err := n.Flush(); err != nil { + logf("[mesh-mail-notifier] a buffered notice could not be shown: %v", err) + } + } +} + +// readAccounts asks mail-store.accounts for the accounts this module is granted, by name to own address. +func readAccounts() (map[string]string, error) { + raw, err := stdio.Ask("seat:mail-store.accounts", map[string]any{}) + if err != nil { + return nil, err + } + var answer struct { + Accounts []struct { + Account string `json:"account"` + Address string `json:"address"` + } `json:"accounts"` + } + if err := json.Unmarshal(raw, &answer); err != nil { + return nil, err + } + out := map[string]string{} + for _, a := range answer.Accounts { + out[a.Account] = a.Address + } + return out, nil +} + +// busStore keeps the notifier's own state (the `shown` bucket the manifest declares): which messages were +// shown already, and each account's last notice. +type busStore struct{} + +func stateKey(parts ...string) string { + words := make([]string, len(parts)) + for i, p := range parts { + var b strings.Builder + for _, r := range p { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '-': + b.WriteRune(r) + default: + b.WriteRune('_') + } + } + words[i] = b.String() + } + return strings.Join(words, "_") +} + +func (busStore) HandleSeen(handle string) (bool, error) { + e, err := stdio.State("shown").Get("handle_" + stateKey(strings.Split(handle, "\x1f")...)) + return e != nil, err +} + +func (busStore) MarkHandle(handle string) error { + _, err := stdio.State("shown").Put("handle_"+stateKey(strings.Split(handle, "\x1f")...), true) + return err +} + +func (busStore) PutStatus(account string, s AccountStatus) error { + _, err := stdio.State("shown").Put("status_"+stateKey(account), s) + return err +} + +func (busStore) Statuses() (map[string]AccountStatus, error) { + keys, err := stdio.State("shown").Keys() + if err != nil { + return nil, err + } + out := map[string]AccountStatus{} + for _, k := range keys { + account, ok := strings.CutPrefix(k, "status_") + if !ok { + continue + } + e, err := stdio.State("shown").Get(k) + if err != nil || e == nil { + continue + } + var s AccountStatus + if json.Unmarshal(e.Value, &s) == nil { + out[account] = s + } + } + return out, nil +} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go new file mode 100644 index 00000000..5e2801b9 --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go @@ -0,0 +1,307 @@ +package main + +// mesh-mail-notifier: the default holder of mail-notifier (novox/hq ADR 0378 §7, as amended by ADR 0379 §4, +// 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. More than three new mails from one account arriving close together are +// folded into one notice naming the count and the latest sender, rather than shown one by one. +// +// Nothing here names Betterbird (ADR 0379 §3). + +import ( + "strconv" + "strings" + "sync" + "time" +) + +// Inbox is the one folder a new mail is shown for. +const Inbox = "INBOX" + +// Seen is the flag that means a mail was already read: shown for nothing it marks. +const SeenFlag = "\\Seen" + +// FoldOver is how many new mails from one account, arriving close together, are still shown one by one; one +// more than that and they become a single notice (novox/hq ADR 0378 §9: "more than three"). +const FoldOver = 3 + +// BurstWindow is how long the notifier waits, after the last qualifying mail of an account, before it decides +// whether to show what it buffered one by one or as a single notice. Well under the minute the decision is +// judged over (ADR 0378 §9), so a burst within that minute is still buffered together. +const BurstWindow = 5 * time.Second + +// Person is a sender or a recipient, as mesh-mail's event says it. +type Person struct { + Name string `json:"name,omitempty"` + Address string `json:"address"` +} + +// EventBody is what mesh-mail's added event says of a message (metadata only; mailevent.go names where it is +// said). Unknown fields are ignored: the notifier reads only what it needs. +type EventBody struct { + Account string `json:"account"` + Folder string `json:"folder"` + UID uint32 `json:"uid"` + MessageID string `json:"message-id"` + Subject string `json:"subject"` + From []Person `json:"from"` + Flags []string `json:"flags"` +} + +// Handle names one message, stably, for the dedupe state (novox/hq ADR 0378 §9: "a redelivered event is not +// shown twice"). +func (e EventBody) Handle() string { + return e.Account + "\x1f" + e.Folder + "\x1f" + strconv.FormatUint(uint64(e.UID), 10) +} + +func (e EventBody) hasFlag(flag string) bool { + for _, f := range e.Flags { + if strings.EqualFold(f, flag) { + return true + } + } + return false +} + +// sender is how a message's first sender reads in a notice: its name and address, or its address alone. +func (e EventBody) sender() string { + if len(e.From) == 0 { + return "(unknown sender)" + } + p := e.From[0] + if p.Name == "" { + return p.Address + } + return p.Name + " <" + p.Address + ">" +} + +func (e EventBody) fromAddress() string { + if len(e.From) == 0 { + return "" + } + return e.From[0].Address +} + +// Notice is one notification the notifier shows: its args for node-notifier.send, and the message its `shown` +// event names (the latest one, for a folded notice). +type Notice struct { + Account string + Args map[string]any + Handles []string // every message this notice accounts for, marked seen once shown + Latest EventBody +} + +// Store is what the notifier keeps of its own (the `shown` state the manifest declares): whether a message +// was shown already, and each account's last notice, for its status tool. +type Store interface { + HandleSeen(handle string) (bool, error) + MarkHandle(handle string) error + PutStatus(account string, s AccountStatus) error + Statuses() (map[string]AccountStatus, error) +} + +// AccountStatus is what the notifier last showed of one account. +type AccountStatus struct { + At time.Time `json:"at"` + Summary string `json:"summary"` +} + +// Notifier buffers qualifying mail per account and decides, once no more of that account's mail arrives for a +// moment, how to show what it buffered. +type Notifier struct { + // Send calls node-notifier.send on this machine. + Send func(args map[string]any) error + // Accounts reads the accounts the notifier is granted, by mail-store.accounts: each one's own address, for + // the quiet rule (novox/hq ADR 0378 §9's LEAD note). An account not known there is shown anyway. + Accounts func() (map[string]string, error) + Store Store + // EmitShown says mesh-mail-notifier's own event `shown`: account and message id, never sender or subject. + EmitShown func(account, messageID string) error + Now func() time.Time + Logf func(string, ...any) + Debounce time.Duration + + mu sync.Mutex + pending map[string][]EventBody + last map[string]time.Time +} + +func (n *Notifier) init() { + n.mu.Lock() + defer n.mu.Unlock() + if n.pending == nil { + n.pending = map[string][]EventBody{} + n.last = map[string]time.Time{} + } + if n.Now == nil { + n.Now = time.Now + } + if n.Logf == nil { + n.Logf = func(string, ...any) {} + } + if n.Debounce == 0 { + n.Debounce = BurstWindow + } +} + +// Add takes one added event. It returns an error only to have the event offered again (the handler +// Subscribe wraps this in follows that rule). +func (n *Notifier) Add(e EventBody) error { + n.init() + seen, err := n.Store.HandleSeen(e.Handle()) + if err != nil { + return err + } + if seen { + return nil // a redelivery of a message already shown (or already judged not to be) + } + own, known, err := n.ownAddress(e.Account) + if err != nil { + return err + } + if !n.qualifies(e, own, known) { + return nil + } + n.mu.Lock() + n.pending[e.Account] = append(n.pending[e.Account], e) + n.last[e.Account] = n.Now() + n.mu.Unlock() + return nil +} + +// qualifies says whether a mail is shown at all: a new inbox mail, not the account's own, not already read. +func (n *Notifier) qualifies(e EventBody, ownAddress string, ownKnown bool) bool { + if !strings.EqualFold(e.Folder, Inbox) { + return false + } + if e.hasFlag(SeenFlag) { + return false + } + if ownKnown && ownAddress != "" && strings.EqualFold(e.fromAddress(), ownAddress) { + return false + } + return true +} + +func (n *Notifier) ownAddress(account string) (string, bool, error) { + accounts, err := n.Accounts() + if err != nil { + // Not known: ADR 0378 §9's LEAD note says the notice is shown when the address cannot be read. Logged, + // not retried: mail-store being unreachable is not this event's fault. + n.Logf("[mesh-mail-notifier] the account's own address could not be read (%v); the notice is shown", err) + return "", false, nil + } + addr, ok := accounts[account] + return addr, ok, nil +} + +// Flush decides every account whose buffer has been quiet for Debounce: one notice each mail it holds, or +// one notice folding them all when there are more than FoldOver. +func (n *Notifier) Flush() error { + n.init() + now := n.Now() + n.mu.Lock() + var ready []string + for account, at := range n.last { + if now.Sub(at) >= n.Debounce { + ready = append(ready, account) + } + } + batches := map[string][]EventBody{} + for _, account := range ready { + batches[account] = n.pending[account] + delete(n.pending, account) + delete(n.last, account) + } + n.mu.Unlock() + for account, events := range batches { + if err := n.decide(account, events); err != nil { + return err + } + } + return nil +} + +// decide shows what one account buffered, and emits `shown` for the notice. +func (n *Notifier) decide(account string, events []EventBody) error { + if len(events) == 0 { + return nil + } + latest := events[len(events)-1] + var notice Notice + if len(events) > FoldOver { + notice = Notice{ + Account: account, + Args: map[string]any{ + "summary": strconv.Itoa(len(events)) + " new mails", + "body": "latest from " + latest.sender() + " — " + account, + "app_name": "mesh-mail", + }, + Latest: latest, + } + for _, e := range events { + notice.Handles = append(notice.Handles, e.Handle()) + } + if err := n.show(notice); err != nil { + return err + } + return nil + } + for _, e := range events { + notice := Notice{ + Account: account, + Args: map[string]any{ + "summary": e.Subject, + "body": e.sender() + " — " + account, + "app_name": "mesh-mail", + }, + Handles: []string{e.Handle()}, + Latest: e, + } + if err := n.show(notice); err != nil { + return err + } + } + return nil +} + +// show sends one notice, marks every message it accounts for seen, keeps the account's status, and emits +// `shown`. +func (n *Notifier) show(notice Notice) error { + if err := n.Send(notice.Args); err != nil { + n.Logf("[mesh-mail-notifier] %s's notice could not be shown: %v", notice.Account, err) + return err + } + for _, h := range notice.Handles { + if err := n.Store.MarkHandle(h); err != nil { + n.Logf("[mesh-mail-notifier] shown, but not kept as shown (%v): a redelivery may show it again", err) + } + } + summary, _ := notice.Args["summary"].(string) + if err := n.Store.PutStatus(notice.Account, AccountStatus{At: n.Now(), Summary: summary}); err != nil { + n.Logf("[mesh-mail-notifier] shown, but the status could not be kept: %v", err) + } + return n.EmitShown(notice.Account, notice.Latest.MessageID) +} + +// Status is the notifier's account of itself, for mail-notifier.status (novox/hq ADR 0378 §7: "accounts it +// announces there, when it last showed a notice"). +func (n *Notifier) Status() (map[string]any, error) { + statuses, err := n.Store.Statuses() + if err != nil { + return nil, err + } + accounts := map[string]any{} + for account, s := range statuses { + accounts[account] = map[string]any{"at": s.At.UTC().Format(time.RFC3339), "shown": s.Summary} + } + return map[string]any{ + "accounts": accounts, + "rules": []string{ + "it shows each new inbox mail at most once, through node-notifier.send on this machine (novox/hq ADR 0378 §7)", + "quiet during a first sync, for mail from the account's own address, for mail already marked read, and for anything outside the inbox", + "more than three new mails from one account arriving close together become one notice naming the count and the latest sender", + }, + }, nil +} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go new file mode 100644 index 00000000..99582247 --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go @@ -0,0 +1,169 @@ +package main + +import ( + "strings" + "sync" + "testing" + "time" +) + +// fakeStore is Store kept in memory, for the test alone. +type fakeStore struct { + mu sync.Mutex + handles map[string]bool + statuses map[string]AccountStatus +} + +func newFakeStore() *fakeStore { + return &fakeStore{handles: map[string]bool{}, statuses: map[string]AccountStatus{}} +} + +func (s *fakeStore) HandleSeen(handle string) (bool, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.handles[handle], nil +} + +func (s *fakeStore) MarkHandle(handle string) error { + s.mu.Lock() + defer s.mu.Unlock() + s.handles[handle] = true + return nil +} + +func (s *fakeStore) PutStatus(account string, st AccountStatus) error { + s.mu.Lock() + defer s.mu.Unlock() + s.statuses[account] = st + return nil +} + +func (s *fakeStore) Statuses() (map[string]AccountStatus, error) { + s.mu.Lock() + defer s.mu.Unlock() + out := map[string]AccountStatus{} + for k, v := range s.statuses { + out[k] = v + } + return out, nil +} + +// fakeClock is a clock the test moves by hand, so Flush's debounce needs no real sleep. +type fakeClock struct { + mu sync.Mutex + now time.Time +} + +func (c *fakeClock) Now() time.Time { + c.mu.Lock() + defer c.mu.Unlock() + return c.now +} + +func (c *fakeClock) Advance(d time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + c.now = c.now.Add(d) +} + +// TestFirstMailOfAnAccountIsShownOnceEachAndABurstIsFolded is the issue's first test (novox/hq ADR 0378 §9, +// issue 800 part B): given a new inbox mail from another sender, one already read, one from the account's own +// address, one in another folder, and five within a minute from one account, it sends exactly one notice, +// nothing, nothing, nothing, and one notice folding the five and naming the latest sender; it fails today +// because the module does not exist. +func TestFirstMailOfAnAccountIsShownOnceEachAndABurstIsFolded(t *testing.T) { + clock := &fakeClock{now: time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC)} + store := newFakeStore() + var mu sync.Mutex + var sent []map[string]any + var shown []struct{ account, id string } + + n := &Notifier{ + Now: clock.Now, + Send: func(args map[string]any) error { + mu.Lock() + defer mu.Unlock() + sent = append(sent, args) + return nil + }, + Accounts: func() (map[string]string, error) { + return map[string]string{"work": "me@work.example", "home": "me@home.example"}, nil + }, + Store: store, + EmitShown: func(account, messageID string) error { + mu.Lock() + defer mu.Unlock() + shown = append(shown, struct{ account, id string }{account, messageID}) + return nil + }, + Logf: t.Logf, + } + + // 1. A new inbox mail from another sender: shown. + one := EventBody{Account: "work", Folder: "INBOX", UID: 1, MessageID: "m1", Subject: "Hi", + From: []Person{{Name: "Alice", Address: "alice@example.com"}}} + // 2. Already read. + two := EventBody{Account: "work", Folder: "INBOX", UID: 2, MessageID: "m2", Subject: "Old", + From: []Person{{Address: "bob@example.com"}}, Flags: []string{"\\Seen"}} + // 3. From the account's own address. + three := EventBody{Account: "work", Folder: "INBOX", UID: 3, MessageID: "m3", Subject: "Sent", + From: []Person{{Address: "me@work.example"}}} + // 4. In another folder. + four := EventBody{Account: "work", Folder: "Archive", UID: 4, MessageID: "m4", Subject: "Filed", + From: []Person{{Address: "carol@example.com"}}} + for _, e := range []EventBody{one, two, three, four} { + if err := n.Add(e); err != nil { + t.Fatalf("Add(%+v): %v", e, err) + } + } + + // 5. Five new inbox mails from another account within a minute. + var burst []EventBody + for i := 1; i <= 5; i++ { + e := EventBody{Account: "home", Folder: "INBOX", UID: uint32(i), MessageID: "burst" + string(rune('0'+i)), + Subject: "Mail", From: []Person{{Name: "Sender " + string(rune('0'+i)), Address: "s@example.com"}}} + burst = append(burst, e) + clock.Advance(time.Second) + if err := n.Add(e); err != nil { + t.Fatalf("Add(%+v): %v", e, err) + } + } + latest := burst[len(burst)-1] + + clock.Advance(BurstWindow + time.Second) + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + + if len(sent) != 2 { + t.Fatalf("sent %d notice(s): %+v", len(sent), sent) + } + if sent[0]["summary"] != "Hi" || !strings.Contains(sent[0]["body"].(string), "Alice") { + t.Errorf("the first notice is %+v, want Alice's mail Hi", sent[0]) + } + if sent[1]["summary"] != "5 new mails" || !strings.Contains(sent[1]["body"].(string), latest.sender()) { + t.Errorf("the folded notice is %+v, want 5 new mails naming %s", sent[1], latest.sender()) + } + + if len(shown) != 2 { + t.Fatalf("emitted shown %d time(s): %+v", len(shown), shown) + } + if shown[0].account != "work" || shown[0].id != "m1" { + t.Errorf("the first shown is %+v, want work/m1", shown[0]) + } + if shown[1].account != "home" || shown[1].id != latest.MessageID { + t.Errorf("the folded shown is %+v, want home/%s (the latest)", shown[1], latest.MessageID) + } + + // Redelivering the first event again shows nothing more: it was marked seen. + if err := n.Add(one); err != nil { + t.Fatalf("Add(one) again: %v", err) + } + clock.Advance(BurstWindow + time.Second) + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 2 { + t.Fatalf("a redelivered event was shown again: sent now %+v", sent) + } +} diff --git a/modules/mesh-mail-notifier/go.mod b/modules/mesh-mail-notifier/go.mod new file mode 100644 index 00000000..7d050d08 --- /dev/null +++ b/modules/mesh-mail-notifier/go.mod @@ -0,0 +1,5 @@ +module mesh-mail-notifier + +go 1.22 + +require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44 diff --git a/modules/mesh-mail-notifier/go.sum b/modules/mesh-mail-notifier/go.sum new file mode 100644 index 00000000..3b200c4c --- /dev/null +++ b/modules/mesh-mail-notifier/go.sum @@ -0,0 +1,2 @@ +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44 h1:r5xIo5qWOXQdCSMxvqJy8kV4U+P/QWXFK6971Ha6wU8= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= diff --git a/modules/mesh-mail-notifier/module.json b/modules/mesh-mail-notifier/module.json new file mode 100644 index 00000000..1d1cb773 --- /dev/null +++ b/modules/mesh-mail-notifier/module.json @@ -0,0 +1,79 @@ +{ + "module": "mesh-mail-notifier", + "version": "1", + "runs-as": "mesh-mail-notifier", + "claims": [ + { + "name": "mail-notifier", + "scope": "node", + "serves": ["status"] + } + ], + "consumes": [ + "mesh-mail.mail.*.message.added" + ], + "invokes": [ + "seat:node-notifier.send", + "seat:mail-store.accounts" + ], + "emits": [ + "shown" + ], + "state": [ + { + "name": "shown", + "ttl-seconds": 2592000 + } + ], + "tools": [ + "mail-notifier.status" + ], + "own-secrets": { + "broker": "${dir:state}/broker" + }, + "secrets-owner": "mesh-mail-notifier", + "resources": [ + { + "id": "account", + "type": "user", + "name": "mesh-mail-notifier", + "shell": "/usr/bin/nologin", + "home": "/var/lib/mesh-mail-notifier" + }, + { + "id": "state", + "type": "directory", + "mode": "0700", + "owner": "mesh-mail-notifier", + "place": "." + }, + { + "id": "notify", + "type": "process", + "name": "mesh-mail-notifier-notify", + "artifact": "tools", + "run": [ + "./mesh-mail-notifier", + "notify" + ], + "health": { + "kind": "unit" + } + } + ], + "build": { + "artifacts": [ + { + "name": "tools", + "kind": "bundle", + "language": "go", + "system": "arch", + "from": "cmd/mesh-mail-notifier", + "binary": "mesh-mail-notifier", + "loads": [ + "mesh-mail-notifier" + ] + } + ] + } +} -- 2.54.0 From 89046e6613f80655b29a694fa69dc3a56924a942 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 17:34:00 +0200 Subject: [PATCH 03/10] mesh-mail's mail-notifier needs a list of node seats, as the controller reads it (issue 800) --- modules/mesh-mail/cmd/mesh-mail/manifest_test.go | 4 ++-- modules/mesh-mail/module.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/manifest_test.go b/modules/mesh-mail/cmd/mesh-mail/manifest_test.go index 5f0a2c9a..3ccfc83c 100644 --- a/modules/mesh-mail/cmd/mesh-mail/manifest_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/manifest_test.go @@ -18,7 +18,7 @@ type seat struct { Name string `json:"name"` Scope string `json:"scope"` Serves []string `json:"serves"` - Needs string `json:"needs"` + Needs []string `json:"needs"` } type manifest struct { @@ -115,7 +115,7 @@ func TestTheManifestSaysWhatTheCodeDoes(t *testing.T) { wantSeats := []seat{ {Name: "mail-store", Scope: "mesh", Serves: []string{"accounts", "search", "read", "send"}}, {Name: "mail-webclient", Scope: "mesh", Serves: []string{"open", "status"}}, - {Name: "mail-notifier", Scope: "node", Serves: []string{"status"}, Needs: "node-notifier"}, + {Name: "mail-notifier", Scope: "node", Serves: []string{"status"}, Needs: []string{"node-notifier"}}, } if !reflect.DeepEqual(m.Seats, wantSeats) { t.Fatalf("seats %+v, want %+v", m.Seats, wantSeats) diff --git a/modules/mesh-mail/module.json b/modules/mesh-mail/module.json index 3f3bb3f9..0cb00ea0 100644 --- a/modules/mesh-mail/module.json +++ b/modules/mesh-mail/module.json @@ -16,7 +16,7 @@ { "name": "mail-notifier", "scope": "node", - "needs": "node-notifier", + "needs": ["node-notifier"], "serves": ["status"] } ], -- 2.54.0 From 82475691b2945fd2de52f4c781b2353f40807a66 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 17:37:15 +0200 Subject: [PATCH 04/10] issue 797: messenger gains the tell verb for a plain notice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0380 §8: the operator channel's notify is shaped for conditions (a key that stays open, escalation, a plain rule that rewrites a subject), so a module has no way to say a plain notice such as new mail. tell sends a headline and one or two lines at once to every channel, bypassing routing, the burst window and the walk hold; it offers Details like any message, is kept in the history, but its record is marked cleared at once so it never stays open. Its cap (10 a channel an hour) is its own, apart from notify's and the conditions'. --- modules/messenger/cmd/messenger/answers.go | 4 +- modules/messenger/cmd/messenger/holder.go | 2 + .../messenger/cmd/messenger/holder_test.go | 52 +++++++++++++ modules/messenger/cmd/messenger/main.go | 21 ++++++ .../messenger/cmd/messenger/manifest_test.go | 2 +- modules/messenger/cmd/messenger/outbox.go | 23 ++++++ modules/messenger/cmd/messenger/status.go | 75 +++++++++++++++++++ modules/messenger/cmd/messenger/words.go | 4 + modules/messenger/module.json | 2 + 9 files changed, 183 insertions(+), 2 deletions(-) diff --git a/modules/messenger/cmd/messenger/answers.go b/modules/messenger/cmd/messenger/answers.go index 76a9cbcb..d92bf1af 100644 --- a/modules/messenger/cmd/messenger/answers.go +++ b/modules/messenger/cmd/messenger/answers.go @@ -86,7 +86,9 @@ func (h *Holder) choices(r *Record, kind string) []Choice { if rec := h.openAsk(r); rec != nil { out = h.offers(rec, kind) } - if r.Origin != "condition" || len(out) >= MostActions { + // A condition offers Details so the operator can read more; a tell does too (ADR 0380 §8: "Details is + // offered as for any message") — it is its own message, so Details just shows it again. + if (r.Origin != "condition" && r.Origin != "tell") || len(out) >= MostActions { return out } now := h.Now() diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index 2c237bb4..44a167b6 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -345,6 +345,8 @@ type Holder struct { wantSync bool history int stale int + // tellSeq makes every tell's key its own, even when two calls land in the same instant. + tellSeq int // answered is what came of each answer event, by its id: a redelivered one is acted on once. answered map[string]string // asks are the asks this router holds, by `.`, and tickets each ask's tickets to it. diff --git a/modules/messenger/cmd/messenger/holder_test.go b/modules/messenger/cmd/messenger/holder_test.go index b7a27309..241b5dc2 100644 --- a/modules/messenger/cmd/messenger/holder_test.go +++ b/modules/messenger/cmd/messenger/holder_test.go @@ -524,6 +524,58 @@ func TestNotifyIsKeptApartFromConditions(t *testing.T) { } } +func TestATellReachesEveryChannelOnceAndOffersOnlyDetails(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + c.pass(14 * time.Hour) // well past midnight: it is not held for being "night" — there is no such hold + out, err := h.Tell("new mail arrived for ace", []string{"One message from the inbox you watch."}, "mail") + if err != nil { + t.Fatal(err) + } + if len(tg.sends) != 1 || len(dt.sends) != 1 { + t.Fatalf("telegram %d, desktop %d sends", len(tg.sends), len(dt.sends)) + } + for _, m := range []Message{tg.sends[0], dt.sends[0]} { + if len(m.Actions) != 1 || m.Actions[0].Label != DetailsLabel { + t.Fatalf("actions %+v", m.Actions) + } + } + if sent, _ := out["sent_on"].([]string); len(sent) != 2 { + t.Fatalf("sent_on %v", out) + } + if len(h.Open()) != 0 { + t.Fatalf("a tell left an open record: %+v", h.Open()) + } + c.pass(RemindUrgent + time.Minute) + h.Tick() + if len(tg.sends) != 1 || len(dt.sends) != 1 { + t.Fatalf("a tell was reminded: telegram %d, desktop %d", len(tg.sends), len(dt.sends)) + } +} + +func TestATellIsDroppedPastItsOwnHourlyCap(t *testing.T) { + h, tg, _, _, _ := newHolder(t) + h.Desk = nil + for i := 0; i < TellCapPerHour; i++ { + if _, err := h.Tell("mail", []string{"line"}, ""); err != nil { + t.Fatal(err) + } + } + if len(tg.sends) != TellCapPerHour { + t.Fatalf("sent %d of %d", len(tg.sends), TellCapPerHour) + } + out, err := h.Tell("mail", []string{"line"}, "") + if err != nil { + t.Fatal(err) + } + if len(tg.sends) != TellCapPerHour { + t.Fatalf("the 11th tell was sent: %d", len(tg.sends)) + } + dropped, _ := out["dropped"].([]string) + if len(dropped) != 1 || dropped[0] != "telegram" { + t.Fatalf("dropped %v", out) + } +} + 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/main.go b/modules/messenger/cmd/messenger/main.go index 3a521339..c50dcecd 100644 --- a/modules/messenger/cmd/messenger/main.go +++ b/modules/messenger/cmd/messenger/main.go @@ -645,6 +645,27 @@ func tools(h *Holder, l *listening) []stdio.Tool { return h.Notify(strArg(a, "key"), strArg(a, "severity"), strArg(a, "summary"), strArg(a, "subject"), clear, strArg(a, "headline"), strArg(a, "explanation"), strArg(a, "resolved")) }}, + {Name: "operator-channel.tell", + Description: "Tell the operator a plain notice that needs no answer and opens no condition (ADR 0380): a " + + "headline and one or two lines, said at once on every channel, held to the content rule as notify's " + + "words are, and offering Details like any message. Never escalated, never kept open, never reminded; " + + "its own cap of 10 a channel an hour, apart from the conditions' and notify's.", + Input: map[string]any{ + "headline": str("a few plain words the operator reads as the title, e.g. new mail arrived for ace"), + "lines": map[string]any{"type": "array", "items": map[string]any{"type": "string"}, "description": "one or two plain lines"}, + "subject": str("what it is about: a machine's role, a module, a plan"), + }, + Run: func(a map[string]any) (any, error) { + var lines []string + if raw, ok := a["lines"].([]any); ok { + for _, l := range raw { + if s, ok := l.(string); ok { + lines = append(lines, s) + } + } + } + return h.Tell(strArg(a, "headline"), lines, strArg(a, "subject")) + }}, {Name: "messenger_status", Description: "Whether the operator can be reached, and why not: each channel kind the mesh holds — can it " + "send, what it said of itself, its last error, how many it sent in the last hour against the cap, how " + diff --git a/modules/messenger/cmd/messenger/manifest_test.go b/modules/messenger/cmd/messenger/manifest_test.go index 3ae820f8..4e94d8f3 100644 --- a/modules/messenger/cmd/messenger/manifest_test.go +++ b/modules/messenger/cmd/messenger/manifest_test.go @@ -62,7 +62,7 @@ func readManifest(t *testing.T) (manifest, string) { func TestItDeclaresTheSeatsOfAnAskAndHoldsTheOperatorChannel(t *testing.T) { m, _ := readManifest(t) - verbs := []string{"open", "history", "notify", "asks"} + verbs := []string{"open", "history", "notify", "tell", "asks"} want := []seat{ {Name: asks.Seat, Scope: "mesh", Serves: verbs, Accepts: []string{"ask", "cancel"}, Emits: []string{"decided"}, ByCaller: []string{"ask", "cancel", "decided"}, Records: []string{"asks"}}, diff --git a/modules/messenger/cmd/messenger/outbox.go b/modules/messenger/cmd/messenger/outbox.go index 7e9d2ae7..31159a63 100644 --- a/modules/messenger/cmd/messenger/outbox.go +++ b/modules/messenger/cmd/messenger/outbox.go @@ -626,3 +626,26 @@ func (h *Holder) sentLastHour(channel string) int { } return n } + +// TellCapPerHour: at most this many tells reach one channel in an hour — the tell verb's own cap +// (ADR 0380 §8), never shared with CapPerHour, the conditions' and notify's, so a burst of plain +// notices cannot crowd out a condition or an ask. +const TellCapPerHour = 10 + +// allowTell says whether a channel is under tell's own cap. +func (h *Holder) allowTell(channel string) bool { + return h.tellSentLastHour(channel) < TellCapPerHour +} + +func (h *Holder) tellSentLastHour(channel string) int { + h.mu.Lock() + defer h.mu.Unlock() + since := h.Now().Add(-time.Hour) + n := 0 + for _, s := range h.recent { + if s.Channel == channel && s.What == "tell" && s.Outcome == "sent" && s.At.After(since) { + n++ + } + } + return n +} diff --git a/modules/messenger/cmd/messenger/status.go b/modules/messenger/cmd/messenger/status.go index 2fecf348..17a0cab6 100644 --- a/modules/messenger/cmd/messenger/status.go +++ b/modules/messenger/cmd/messenger/status.go @@ -1,7 +1,9 @@ package main import ( + "fmt" "sort" + "strings" "time" ) @@ -309,3 +311,76 @@ func (h *Holder) Notify(key, severity, summary, subject string, clear bool, plai held := h.queued(key) return map[string]any{"key": key, "sent_on": sent, "held": held, "refused": refused, "unsent": pending}, nil } + +// Tell sends a plain notice that needs no answer and opens no condition (ADR 0380 §8): one message with a +// headline and one or two lines, said at once on every channel the router sends on — bypassing the severity +// routing, the burst window and the walk hold that a condition's message goes through, and never reminded, +// escalated or kept open. Composed and held to the content rule exactly as notify's words are (allowedWords' +// 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. +func (h *Holder) Tell(headline string, lines []string, subject string) (map[string]any, error) { + headline = strings.TrimSpace(headline) + if headline == "" { + return nil, errorf("headline is required") + } + if len(lines) == 0 || len(lines) > 2 { + return nil, errorf("lines takes one or two lines") + } + clean := make([]string, 0, len(lines)) + for _, l := range lines { + l = strings.TrimSpace(l) + if l == "" { + return nil, errorf("a line is empty") + } + clean = append(clean, l) + } + h.work.Lock() + defer h.work.Unlock() + h.mu.Lock() + h.init() + h.mu.Unlock() + now := h.Now() + h.mu.Lock() + h.tellSeq++ + seq := h.tellSeq + h.mu.Unlock() + key := fmt.Sprintf("tell.%d.%d", now.UnixNano(), seq) + rec := &Record{Key: key, Kind: "tell", Subject: subject, Origin: "tell", Headline: headline, + Explanation: strings.Join(clean, "\n"), Summary: headline, Raised: now, Seen: now, Count: 1, + Sent: map[string]string{}, Cleared: now} + h.mu.Lock() + h.open[key] = rec + h.mu.Unlock() + var sentOn, dropped []string + for _, ch := range h.channels() { + name := ch.Name() + if ch.Ready() != nil { + continue + } + if !h.allowTell(name) { + dropped = append(dropped, name) + 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)}) + continue + } + m := h.sayOn(rec, "tell", name) + id, err := ch.Send(m) + h.noteChannel(name, err) + if err != nil { + h.record(Sent{At: now, Channel: name, Key: key, What: "tell", Outcome: "failed: " + err.Error()}) + continue + } + h.mu.Lock() + rec.Sent[name] = id + h.mu.Unlock() + sentOn = append(sentOn, name) + h.record(Sent{At: now, Channel: name, Key: key, What: "tell", Outcome: "sent"}) + } + h.persist(rec) + sort.Strings(sentOn) + sort.Strings(dropped) + return map[string]any{"key": key, "sent_on": sentOn, "dropped": dropped, "refused": rec.Refused}, nil +} diff --git a/modules/messenger/cmd/messenger/words.go b/modules/messenger/cmd/messenger/words.go index b3546232..1a450d85 100644 --- a/modules/messenger/cmd/messenger/words.go +++ b/modules/messenger/cmd/messenger/words.go @@ -63,6 +63,10 @@ func compose(r *Record, what string, now time.Time, loc *time.Location, withhold if urgent { title = "Urgent: " + title } + case "tell": + // A plain notice (ADR 0380 §1, §8): the headline and its one or two lines, nothing else — no + // "since", no severity, no reminder: it waits for no answer and is never said again. + return Message{Title: headline, Body: explanation} default: title = headline if urgent { diff --git a/modules/messenger/module.json b/modules/messenger/module.json index 3a12a074..3a50f885 100644 --- a/modules/messenger/module.json +++ b/modules/messenger/module.json @@ -11,6 +11,7 @@ "open", "history", "notify", + "tell", "asks" ], "accepts": [ @@ -62,6 +63,7 @@ "open", "history", "notify", + "tell", "asks" ] } -- 2.54.0 From cc73b386716320a44f7fcc98ac65c0146a96689f Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 17:46:39 +0200 Subject: [PATCH 05/10] issue 797: tells no longer eat the conditions'/notify's cap, and a dropped tell is logged once an hour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 of mesh-catalog found sentLastHour counting tells against CapPerHour (ADR 0380 §8 says tells keep their own cap, never shared), and the drop log firing on every drop instead of once an hour per channel as the comment already said. --- modules/messenger/cmd/messenger/holder.go | 50 ++++++++-------- .../messenger/cmd/messenger/holder_test.go | 59 +++++++++++++++++++ modules/messenger/cmd/messenger/outbox.go | 6 +- modules/messenger/cmd/messenger/status.go | 11 +++- 4 files changed, 99 insertions(+), 27 deletions(-) diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index 44a167b6..9182672c 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -321,30 +321,31 @@ type Holder struct { work sync.Mutex // one event, call or tick at a time mu sync.Mutex // what the status reads - open map[string]*Record - recent []Sent - refusals []RefusalNote - outbox map[string][]item - lastSend map[string]time.Time - lastWarn map[string]time.Time - lastTry map[string]time.Time - capSaid map[string]time.Time - chanErr map[string]string - chanErrAt map[string]time.Time - chanOK map[string]time.Time - unreadable int - lastBad string - lastBadAt time.Time - saidOnce map[string]time.Time - storeErr string - heard map[string]int - lastHeard time.Time - started time.Time - syncedAt time.Time - syncErr string - wantSync bool - history int - stale int + open map[string]*Record + recent []Sent + refusals []RefusalNote + outbox map[string][]item + lastSend map[string]time.Time + lastWarn map[string]time.Time + lastTry map[string]time.Time + capSaid map[string]time.Time + tellCapSaid map[string]time.Time + chanErr map[string]string + chanErrAt map[string]time.Time + chanOK map[string]time.Time + unreadable int + lastBad string + lastBadAt time.Time + saidOnce map[string]time.Time + storeErr string + heard map[string]int + lastHeard time.Time + started time.Time + syncedAt time.Time + syncErr string + wantSync bool + history int + stale int // tellSeq makes every tell's key its own, even when two calls land in the same instant. tellSeq int // answered is what came of each answer event, by its id: a redelivered one is acted on once. @@ -376,6 +377,7 @@ func (h *Holder) init() { h.lastWarn = map[string]time.Time{} h.lastTry = map[string]time.Time{} h.capSaid = map[string]time.Time{} + h.tellCapSaid = map[string]time.Time{} h.chanErr = map[string]string{} h.chanErrAt = map[string]time.Time{} h.chanOK = map[string]time.Time{} diff --git a/modules/messenger/cmd/messenger/holder_test.go b/modules/messenger/cmd/messenger/holder_test.go index 241b5dc2..e79286a9 100644 --- a/modules/messenger/cmd/messenger/holder_test.go +++ b/modules/messenger/cmd/messenger/holder_test.go @@ -576,6 +576,65 @@ func TestATellIsDroppedPastItsOwnHourlyCap(t *testing.T) { } } +func TestATellDoesNotCountTowardTheConditionsCap(t *testing.T) { + h, tg, _, c, _ := newHolder(t) + h.Desk = nil + // Bring conditions close to, but under, their own cap: CapPerHour-5. + for i := 0; i < CapPerHour-5; 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-5 { + t.Fatalf("sent %d conditions, want %d", len(tg.sends), CapPerHour-5) + } + // A full hour of tells: these must not be counted against the conditions' cap (ADR 0380 §8). + for i := 0; i < TellCapPerHour; i++ { + if _, err := h.Tell("mail", []string{"line"}, ""); err != nil { + t.Fatal(err) + } + } + h.Condition(EventRaised, cond("machine.mlast.silent", Urgent, "a machine is silent")) + c.pass(BurstWindow + time.Second) + h.Flush() + want := CapPerHour - 5 + TellCapPerHour + 1 + if len(tg.sends) != want { + t.Fatalf("sent %d, want %d (the condition after the tells still sent)", 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 condition was held behind the tells' sends: %q", last.Title) + } +} + +func TestATellCapDropIsLoggedOnceAnHour(t *testing.T) { + h, tg, _, _, _ := newHolder(t) + h.Desk = nil + var logs []string + h.Logf = func(format string, args ...any) { logs = append(logs, fmt.Sprintf(format, args...)) } + for i := 0; i < TellCapPerHour; i++ { + if _, err := h.Tell("mail", []string{"line"}, ""); err != nil { + t.Fatal(err) + } + } + for i := 0; i < 3; i++ { + if _, err := h.Tell("mail", []string{"line"}, ""); err != nil { + t.Fatal(err) + } + } + if len(tg.sends) != TellCapPerHour { + t.Fatalf("sent %d of %d", len(tg.sends), TellCapPerHour) + } + dropLogs := 0 + for _, l := range logs { + if strings.Contains(l, "dropped") { + dropLogs++ + } + } + if dropLogs != 1 { + t.Fatalf("logged the drop %d times, want 1", dropLogs) + } +} + 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/outbox.go b/modules/messenger/cmd/messenger/outbox.go index 31159a63..ab11d4d3 100644 --- a/modules/messenger/cmd/messenger/outbox.go +++ b/modules/messenger/cmd/messenger/outbox.go @@ -609,7 +609,9 @@ func (h *Holder) sendNow(ch Channel, key, what string, m Message) bool { return true } -// allow says whether a channel is under its cap: messages sent in the last hour, as recorded. +// allow says whether a channel is under its cap: messages sent in the last hour, as recorded. A +// tell is not counted here: it has its own cap (TellCapPerHour) so a burst of tells cannot crowd +// out a condition or an ask (ADR 0380 §8). func (h *Holder) allow(channel string) bool { return h.sentLastHour(channel) < CapPerHour } @@ -620,7 +622,7 @@ func (h *Holder) sentLastHour(channel string) int { since := h.Now().Add(-time.Hour) n := 0 for _, s := range h.recent { - if s.Channel == channel && s.Outcome == "sent" && s.At.After(since) { + if s.Channel == channel && s.What != "tell" && s.Outcome == "sent" && s.At.After(since) { n++ } } diff --git a/modules/messenger/cmd/messenger/status.go b/modules/messenger/cmd/messenger/status.go index 17a0cab6..cc89348a 100644 --- a/modules/messenger/cmd/messenger/status.go +++ b/modules/messenger/cmd/messenger/status.go @@ -361,7 +361,16 @@ func (h *Holder) Tell(headline string, lines []string, subject string) (map[stri } if !h.allowTell(name) { dropped = append(dropped, name) - h.Logf("[messenger] a tell was dropped on %s: it is at its own cap of %d tells an hour", name, TellCapPerHour) + h.mu.Lock() + said := h.tellCapSaid[name] + sayIt := said.IsZero() || now.Sub(said) >= time.Hour + if sayIt { + h.tellCapSaid[name] = now + } + h.mu.Unlock() + 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)}) continue -- 2.54.0 From ed66a2e8735f5c9eb74220b356e963735347ed23 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 17:54:27 +0200 Subject: [PATCH 06/10] issue 800: fix review round 1 findings in mesh-mail-notifier Flush now sorts accounts instead of walking a map, so notice order is stable. A failed node-notifier.send keeps its mail pending and retries at most once a minute, without holding up other accounts' notices, and the failure is kept in the Store for a new health tool (mail_notifier_health) that is unhealthy past ten minutes of failure. The burst rule now follows the lead's reading of ADR 0378 S9: within an account's sliding minute, the first three qualifying mails are shown one by one and the rest are folded into one notice at the window's end; there is no separate quiet-gap timer. The dedupe key is the account and message id, falling back to folder/uidvalidity/uid only when no message id is given, so a UIDVALIDITY reset cannot suppress a new mail, and a redelivered event already buffered is never recounted. status now lists every granted account, not only those with a notice, and caches mail-store.accounts for a minute. --- modules/mesh-mail-notifier/README.md | 15 +- .../cmd/mesh-mail-notifier/health.go | 53 +++ .../cmd/mesh-mail-notifier/health_test.go | 39 ++ .../cmd/mesh-mail-notifier/main.go | 62 ++- .../cmd/mesh-mail-notifier/notifier.go | 385 +++++++++++++----- .../cmd/mesh-mail-notifier/notifier_test.go | 383 +++++++++++++---- modules/mesh-mail-notifier/module.json | 10 +- 7 files changed, 762 insertions(+), 185 deletions(-) create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health_test.go diff --git a/modules/mesh-mail-notifier/README.md b/modules/mesh-mail-notifier/README.md index 1f82055c..2ce347ac 100644 --- a/modules/mesh-mail-notifier/README.md +++ b/modules/mesh-mail-notifier/README.md @@ -15,11 +15,15 @@ on this machine's notifier, through `node-notifier.send`, at most once: notice is shown) - quiet for a mail already marked read (`\Seen` in its flags) - quiet for anything outside the inbox -- more than three new mails from one account arriving close together become one notice naming the count and - the latest sender, rather than one notice each +- 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 -Each notice shown emits this module's own event `shown` (account and message id, never sender or subject), and -is kept in its own state (`shown`) so a redelivered event is not shown twice. +A notice `node-notifier.send` refuses stays pending and is tried again, at most once a minute, without holding +up any other account's notice; nothing is deleted before it is shown. Each notice shown emits this module's +own event `shown` (account and message id, never sender or subject), and is kept in its own state (`shown`) so +a redelivered event is never counted or shown twice — by the mail's message id wherever it has one, so a +UIDVALIDITY reset cannot make an already-shown mail look new. ## Where it runs @@ -32,4 +36,5 @@ accounts it announces, in mesh-mail's trusted setting `grants` (`mail-store`'s s | tool | does | |---|---| -| `mail-notifier.status` | the accounts it announces on this machine, and when it last showed a notice; its rules. Read-only | +| `mail-notifier.status` | every account it is granted, when it last showed each one a notice, and any notice still failing to send (its count, last error, since when). Read-only | +| `mail_notifier_health` | how it is: unhealthy once a notice has failed to show for more than ten minutes, healthy otherwise, even mid-retry. Read-only | diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go new file mode 100644 index 00000000..324b6af2 --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go @@ -0,0 +1,53 @@ +package main + +// mesh-mail-notifier's own health (review round 1 of issue 800, which added this tool beside mail-notifier's +// seat verb status, as mesh-mail's own health tool is mail_health beside its seat tools). A notice that fails +// to send stays pending and is retried, at most once a minute (Flush, notifier.go); the account's +// failing-since stays in the Store, so this tool — its own process, asked separately by the node-engine — +// sees the same account the live notifier does, with no state of its own to lose between asks. +// +// Unhealthy once an account's notice has failed to show for more than UnhealthyAfter; healthy otherwise, even +// while a notice is mid-retry. + +import ( + "fmt" + "sort" + "strings" + "time" + + "git.novox.be/novox/mesh-sdk/go/health" +) + +// HealthTool is the module's own health tool the node-engine asks (beside mail-notifier.status, the seat's own verb). +const HealthTool = "mail_notifier_health" + +// UnhealthyAfter is how long a notice may fail to send before that is unhealthy, not merely retrying. +const UnhealthyAfter = 10 * time.Minute + +// Judge is mesh-mail-notifier's health, from the failures the Store keeps. +func Judge(failures map[string]FailureStatus, now time.Time) health.Answer { + if len(failures) == 0 { + return health.Healthy("every notice shown; none pending a retry") + } + accounts := make([]string, 0, len(failures)) + for account := range failures { + accounts = append(accounts, account) + } + sort.Strings(accounts) + + var problems, retrying []string + for _, account := range accounts { + f := failures[account] + age := now.Sub(f.Since).Round(time.Second) + if now.Sub(f.Since) > UnhealthyAfter { + problems = append(problems, fmt.Sprintf("%s: its notice has failed to show for %s (%d attempt(s), last: %s)", + account, age, f.Count, f.LastError)) + } else { + retrying = append(retrying, fmt.Sprintf("%s has been retrying for %s", account, age)) + } + } + if len(problems) > 0 { + return health.Unhealthy(strings.Join(problems, "; ")) + } + return health.Healthy(strings.Join(retrying, "; ")) +} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health_test.go new file mode 100644 index 00000000..750beae2 --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "strings" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/health" +) + +func TestJudgeIsUnhealthyOnlyPastUnhealthyAfter(t *testing.T) { + now := time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC) + + if a := Judge(nil, now); a.State != health.StateHealthy { + t.Fatalf("no failures: %+v, want healthy", a) + } + + recent := map[string]FailureStatus{"work": {Count: 1, LastError: "no answer", Since: now.Add(-2 * time.Minute)}} + if a := Judge(recent, now); a.State != health.StateHealthy { + t.Fatalf("a failure 2m old: %+v, want healthy (still retrying)", a) + } + + stale := map[string]FailureStatus{"work": {Count: 11, LastError: "no answer", Since: now.Add(-11 * time.Minute)}} + a := Judge(stale, now) + if a.State != health.StateUnhealthy { + t.Fatalf("a failure 11m old: %+v, want unhealthy (past UnhealthyAfter)", a) + } + if !strings.Contains(a.Why, "work") || !strings.Contains(a.Why, "no answer") { + t.Errorf("Why = %q, want it to name the account and the last error", a.Why) + } + + mixed := map[string]FailureStatus{ + "work": {Count: 11, LastError: "no answer", Since: now.Add(-11 * time.Minute)}, + "home": {Count: 1, LastError: "no answer", Since: now.Add(-time.Minute)}, + } + if a := Judge(mixed, now); a.State != health.StateUnhealthy || !strings.Contains(a.Why, "work") { + t.Errorf("one stale failure beside one fresh one: %+v, want unhealthy naming work", a) + } +} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go index bf74727d..36bc9180 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go @@ -1,6 +1,7 @@ // mesh-mail-notifier: the default claimant of mail-notifier, built from mesh-mail's own event shape (novox/hq // ADR 0378 §9, issue 800 part B). A Go bundle the node's runtime launches, its own account (runs-as, ADR 0376 -// §2): the base invocation serves mail-notifier.status; a separate process (`notify`) hears mesh-mail's added +// §2): the base invocation serves mail-notifier.status and the module's own health tool +// (mail_notifier_health, review round 1 of issue 800); a separate process (`notify`) hears mesh-mail's added // events and shows them. What this module says, it says on stderr; stdout is the MCP channel. package main @@ -50,18 +51,31 @@ func main() { return } - tools := []stdio.Tool{{Name: StatusTool, - Description: "The accounts mesh-mail-notifier announces on this machine, and when it last showed a " + - "notice of new mail (novox/hq ADR 0378 §7). Read-only.", - Run: func(map[string]any) (any, error) { return n.Status() }}} + tools := []stdio.Tool{ + {Name: StatusTool, + Description: "The accounts mesh-mail-notifier announces on this machine, and when it last showed a " + + "notice of new mail (novox/hq ADR 0378 §7). Read-only.", + Run: func(map[string]any) (any, error) { return n.Status() }}, + {Name: HealthTool, + Description: "How mesh-mail-notifier is: unhealthy once a notice has failed to show for more than " + + "ten minutes, healthy otherwise, even mid-retry (review round 1 of issue 800). Read-only.", + Run: func(map[string]any) (any, error) { + failures, err := busStore{}.Failures() + if err != nil { + return nil, err + } + return Judge(failures, time.Now()), nil + }}, + } if err := stdio.Serve("", tools); err != nil { logf("%v", err) os.Exit(1) } } -// watch hears mesh-mail's added events and flushes what it buffered once each account has been quiet a -// moment (ADR 0378 §9: folding a burst into one notice). +// watch hears mesh-mail's added events and, once a second, flushes what Notifier.Flush finds ready: an +// account's first mails of its window at once, and its folded overflow once the window is over (ADR 0378 §9, +// the lead's reading, review round 1 of issue 800). func watch(n *Notifier) { time.Sleep(500 * time.Millisecond) // Serve first: the bus is reached through it err := stdio.Subscribe(AddedPattern, func(e stdio.Envelope) error { @@ -108,7 +122,7 @@ func readAccounts() (map[string]string, error) { } // busStore keeps the notifier's own state (the `shown` bucket the manifest declares): which messages were -// shown already, and each account's last notice. +// shown already, each account's last notice, and each account's notice still failing to send. type busStore struct{} func stateKey(parts ...string) string { @@ -165,3 +179,35 @@ func (busStore) Statuses() (map[string]AccountStatus, error) { } return out, nil } + +func (busStore) PutFailure(account string, f FailureStatus) error { + _, err := stdio.State("shown").Put("failure_"+stateKey(account), f) + return err +} + +func (busStore) ClearFailure(account string) error { + return stdio.State("shown").Delete("failure_" + stateKey(account)) +} + +func (busStore) Failures() (map[string]FailureStatus, error) { + keys, err := stdio.State("shown").Keys() + if err != nil { + return nil, err + } + out := map[string]FailureStatus{} + for _, k := range keys { + account, ok := strings.CutPrefix(k, "failure_") + if !ok { + continue + } + e, err := stdio.State("shown").Get(k) + if err != nil || e == nil { + continue + } + var f FailureStatus + if json.Unmarshal(e.Value, &f) == nil { + out[account] = f + } + } + return out, nil +} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go index 5e2801b9..27e0d88d 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go @@ -4,12 +4,15 @@ 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. More than three new mails from one account arriving close together are -// folded into one notice naming the count and the latest sender, rather than shown one by one. +// 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). // // Nothing here names Betterbird (ADR 0379 §3). import ( + "sort" "strconv" "strings" "sync" @@ -22,14 +25,23 @@ const Inbox = "INBOX" // Seen is the flag that means a mail was already read: shown for nothing it marks. const SeenFlag = "\\Seen" -// FoldOver is how many new mails from one account, arriving close together, are still shown one by one; one -// more than that and they become a single notice (novox/hq ADR 0378 §9: "more than three"). +// FoldOver is how many new mails from one account, within one window, are still shown one by one; the rest of +// that window is folded into a single notice (novox/hq ADR 0378 §9: "more than three"). const FoldOver = 3 -// BurstWindow is how long the notifier waits, after the last qualifying mail of an account, before it decides -// whether to show what it buffered one by one or as a single notice. Well under the minute the decision is -// judged over (ADR 0378 §9), so a burst within that minute is still buffered together. -const BurstWindow = 5 * time.Second +// 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). +const Window = time.Minute + +// RetryBackoff is how long the notifier waits, after an account's notice fails to send, before it tries that +// account again: at most once a minute (review round 1 of issue 800), so a send that keeps failing is not +// retried on every Flush. +const RetryBackoff = time.Minute + +// AccountsCacheFor is how long mail-store.accounts's answer is reused before it is asked again (review round +// 1 of issue 800, bullet 5): neither the quiet rule nor status asks it more than once a minute. +const AccountsCacheFor = time.Minute // Person is a sender or a recipient, as mesh-mail's event says it. type Person struct { @@ -40,19 +52,27 @@ type Person struct { // EventBody is what mesh-mail's added event says of a message (metadata only; mailevent.go names where it is // said). Unknown fields are ignored: the notifier reads only what it needs. type EventBody struct { - Account string `json:"account"` - Folder string `json:"folder"` - UID uint32 `json:"uid"` - MessageID string `json:"message-id"` - Subject string `json:"subject"` - From []Person `json:"from"` - Flags []string `json:"flags"` + Account string `json:"account"` + Folder string `json:"folder"` + UID uint32 `json:"uid"` + UIDValidity uint32 `json:"uidvalidity"` + MessageID string `json:"message-id"` + Subject string `json:"subject"` + From []Person `json:"from"` + Flags []string `json:"flags"` } // Handle names one message, stably, for the dedupe state (novox/hq ADR 0378 §9: "a redelivered event is not -// shown twice"). +// shown twice"). The message id is the key wherever a message carries one, so a UIDVALIDITY reset — which +// changes every UID in a folder — can never make an already-shown mail look new, or a genuinely new one look +// already shown; only a message with no message id falls back to its folder, UIDVALIDITY and UID (review +// round 1 of issue 800). func (e EventBody) Handle() string { - return e.Account + "\x1f" + e.Folder + "\x1f" + strconv.FormatUint(uint64(e.UID), 10) + if e.MessageID != "" { + return e.Account + "\x1f" + e.MessageID + } + return e.Account + "\x1f" + e.Folder + "\x1f" + + strconv.FormatUint(uint64(e.UIDValidity), 10) + "\x1f" + strconv.FormatUint(uint64(e.UID), 10) } func (e EventBody) hasFlag(flag string) bool { @@ -92,13 +112,26 @@ type Notice struct { Latest EventBody } +// FailureStatus is what the notifier knows of an account's notice still failing to send (review round 1 of +// issue 800): how many attempts, the last error, and since when. Kept in the Store, not only in memory, so +// the health tool — its own process, asked separately by the node-engine — sees the same account the live +// notifier does. +type FailureStatus struct { + Count int `json:"count"` + LastError string `json:"last-error"` + Since time.Time `json:"since"` +} + // Store is what the notifier keeps of its own (the `shown` state the manifest declares): whether a message -// was shown already, and each account's last notice, for its status tool. +// was shown already, each account's last notice, and each account's notice still failing to send. type Store interface { HandleSeen(handle string) (bool, error) MarkHandle(handle string) error PutStatus(account string, s AccountStatus) error Statuses() (map[string]AccountStatus, error) + PutFailure(account string, f FailureStatus) error + ClearFailure(account string) error + Failures() (map[string]FailureStatus, error) } // AccountStatus is what the notifier last showed of one account. @@ -107,8 +140,19 @@ type AccountStatus struct { Summary string `json:"summary"` } -// Notifier buffers qualifying mail per account and decides, once no more of that account's mail arrives for a -// moment, how to show what it buffered. +// 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). +type accountBuffer struct { + windowStart time.Time + count int // qualifying mails counted in this window so far + immediate []EventBody + folded []EventBody +} + +// Notifier buffers qualifying mail per account and shows it: the first FoldOver mails of a window one by one, +// the rest folded once the window ends. A notice that fails to send is tried again, at most once a minute, +// without holding up any other account's notice. type Notifier struct { // Send calls node-notifier.send on this machine. Send func(args map[string]any) error @@ -120,19 +164,24 @@ type Notifier struct { EmitShown func(account, messageID string) error Now func() time.Time Logf func(string, ...any) - Debounce time.Duration - mu sync.Mutex - pending map[string][]EventBody - last map[string]time.Time + mu sync.Mutex + pending map[string]*accountBuffer + buffered map[string]bool // a message's Handle while it is waiting to be shown: never counted twice + cooldown map[string]time.Time // account -> not retried before this time, after a failed send + + accountsAt time.Time + accountsVal map[string]string + accountsErr error } func (n *Notifier) init() { n.mu.Lock() defer n.mu.Unlock() if n.pending == nil { - n.pending = map[string][]EventBody{} - n.last = map[string]time.Time{} + n.pending = map[string]*accountBuffer{} + n.buffered = map[string]bool{} + n.cooldown = map[string]time.Time{} } if n.Now == nil { n.Now = time.Now @@ -140,16 +189,14 @@ func (n *Notifier) init() { if n.Logf == nil { n.Logf = func(string, ...any) {} } - if n.Debounce == 0 { - n.Debounce = BurstWindow - } } // Add takes one added event. It returns an error only to have the event offered again (the handler // Subscribe wraps this in follows that rule). func (n *Notifier) Add(e EventBody) error { n.init() - seen, err := n.Store.HandleSeen(e.Handle()) + handle := e.Handle() + seen, err := n.Store.HandleSeen(handle) if err != nil { return err } @@ -163,10 +210,25 @@ func (n *Notifier) Add(e EventBody) error { if !n.qualifies(e, own, known) { return nil } + n.mu.Lock() - n.pending[e.Account] = append(n.pending[e.Account], e) - n.last[e.Account] = n.Now() - n.mu.Unlock() + defer n.mu.Unlock() + if n.buffered[handle] { + return nil // a redelivery of a message still waiting to be shown: never counted twice + } + now := n.Now() + b := n.pending[e.Account] + if b == nil || now.Sub(b.windowStart) >= Window { + b = &accountBuffer{windowStart: now} + n.pending[e.Account] = b + } + b.count++ + n.buffered[handle] = true + if b.count <= FoldOver { + b.immediate = append(b.immediate, e) + } else { + b.folded = append(b.folded, e) + } return nil } @@ -185,7 +247,7 @@ func (n *Notifier) qualifies(e EventBody, ownAddress string, ownKnown bool) bool } func (n *Notifier) ownAddress(account string) (string, bool, error) { - accounts, err := n.Accounts() + accounts, err := n.readAccounts() if err != nil { // Not known: ADR 0378 §9's LEAD note says the notice is shown when the address cannot be read. Logged, // not retried: mail-store being unreachable is not this event's fault. @@ -196,83 +258,198 @@ func (n *Notifier) ownAddress(account string) (string, bool, error) { return addr, ok, nil } -// Flush decides every account whose buffer has been quiet for Debounce: one notice each mail it holds, or -// one notice folding them all when there are more than FoldOver. +// readAccounts asks mail-store.accounts, reusing its last answer for AccountsCacheFor so neither the quiet +// rule nor status reads it more than once a minute (review round 1 of issue 800, bullet 5). +func (n *Notifier) readAccounts() (map[string]string, error) { + n.mu.Lock() + if n.accountsVal != nil && n.Now().Sub(n.accountsAt) < AccountsCacheFor { + val, err := n.accountsVal, n.accountsErr + n.mu.Unlock() + return val, err + } + n.mu.Unlock() + + val, err := n.Accounts() + n.mu.Lock() + n.accountsVal, n.accountsErr, n.accountsAt = val, err, n.Now() + n.mu.Unlock() + return val, err +} + +// Flush shows what is ready: each account's first FoldOver mails of its window, one by one, as soon as Flush +// runs, and the window's folded overflow once the window has run its minute. Accounts are judged in a sorted +// order, not a map's iteration order, so the tests do not depend on it (review round 1 of issue 800, BLOCKING +// 1). A notice that fails to send leaves its mail pending, tried again once RetryBackoff has passed, and never +// holds up another account's notice (BLOCKING 2): nothing is deleted before it is shown. func (n *Notifier) Flush() error { n.init() now := n.Now() + n.mu.Lock() - var ready []string - for account, at := range n.last { - if now.Sub(at) >= n.Debounce { - ready = append(ready, account) - } - } - batches := map[string][]EventBody{} - for _, account := range ready { - batches[account] = n.pending[account] - delete(n.pending, account) - delete(n.last, account) + accounts := make([]string, 0, len(n.pending)) + for account := range n.pending { + accounts = append(accounts, account) } n.mu.Unlock() - for account, events := range batches { - if err := n.decide(account, events); err != nil { - return err - } + sort.Strings(accounts) + + for _, account := range accounts { + n.flushAccount(account, now) } return nil } -// decide shows what one account buffered, and emits `shown` for the notice. -func (n *Notifier) decide(account string, events []EventBody) error { - if len(events) == 0 { - 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. +func (n *Notifier) flushAccount(account string, now time.Time) { + n.mu.Lock() + if cd, ok := n.cooldown[account]; ok && now.Before(cd) { + n.mu.Unlock() + return // backing off after a failed send; tried again once RetryBackoff has passed } + b := n.pending[account] + if b == nil { + n.mu.Unlock() + return + } + immediate := append([]EventBody(nil), b.immediate...) + windowDone := now.Sub(b.windowStart) >= Window + var fold []EventBody + if windowDone { + fold = append([]EventBody(nil), b.folded...) + } + n.mu.Unlock() + + if len(immediate) == 0 && len(fold) == 0 { + if windowDone { + n.closeIfEmpty(account, b) + } + return + } + + sentImmediate := 0 + var failErr error + for _, e := range immediate { + if err := n.show(n.singleNotice(account, e)); err != nil { + failErr = err + break + } + sentImmediate++ + } + foldSent := false + if failErr == nil && len(fold) > 0 { + if err := n.show(n.foldNotice(account, fold)); err != nil { + failErr = err + } else { + foldSent = true + } + } + + n.mu.Lock() + if cur := n.pending[account]; cur == b { + cur.immediate = cur.immediate[sentImmediate:] + if foldSent { + cur.folded = nil + } + // 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 { + delete(n.pending, account) + } + } + if failErr != nil { + n.cooldown[account] = now.Add(RetryBackoff) + n.recordFailureLocked(account, failErr, now) + } else { + delete(n.cooldown, account) + n.clearFailureLocked(account) + } + n.mu.Unlock() +} + +func (n *Notifier) closeIfEmpty(account string, b *accountBuffer) { + n.mu.Lock() + defer n.mu.Unlock() + if cur := n.pending[account]; cur == b && len(cur.immediate) == 0 && len(cur.folded) == 0 { + delete(n.pending, account) + } +} + +// recordFailureLocked keeps an account's failure in the Store (mu held by the caller, released around the +// Store calls since they may reach the bus): its count, its last error, and since when, so a later Flush and +// the health tool agree on how long it has been failing. +func (n *Notifier) recordFailureLocked(account string, sendErr error, now time.Time) { + n.mu.Unlock() + defer n.mu.Lock() + since := now + count := 1 + if prev, err := n.Store.Failures(); err == nil { + if p, ok := prev[account]; ok { + since = p.Since + count = p.Count + 1 + } + } + if err := n.Store.PutFailure(account, FailureStatus{Count: count, LastError: sendErr.Error(), Since: since}); err != nil { + n.Logf("[mesh-mail-notifier] a failure could not be kept (%v): the health tool may not see it", err) + } +} + +func (n *Notifier) clearFailureLocked(account string) { + n.mu.Unlock() + defer n.mu.Lock() + if err := n.Store.ClearFailure(account); err != nil { + n.Logf("[mesh-mail-notifier] a cleared failure could not be kept cleared: %v", err) + } +} + +// singleNotice is one mail shown on its own: one of an account's first FoldOver mails of its window. +func (n *Notifier) singleNotice(account string, e EventBody) Notice { + return Notice{ + Account: account, + Args: map[string]any{ + "summary": e.Subject, + "body": e.sender() + " — " + account, + "app_name": "mesh-mail", + }, + Handles: []string{e.Handle()}, + Latest: e, + } +} + +// 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 { latest := events[len(events)-1] - var notice Notice - if len(events) > FoldOver { - notice = Notice{ - Account: account, - Args: map[string]any{ - "summary": strconv.Itoa(len(events)) + " new mails", - "body": "latest from " + latest.sender() + " — " + account, - "app_name": "mesh-mail", - }, - Latest: latest, - } - for _, e := range events { - notice.Handles = append(notice.Handles, e.Handle()) - } - if err := n.show(notice); err != nil { - return err - } - return nil + notice := Notice{ + Account: account, + Args: map[string]any{ + "summary": strconv.Itoa(len(events)) + " new mails in " + account, + "body": "latest from " + latest.sender(), + "app_name": "mesh-mail", + }, + Latest: latest, } for _, e := range events { - notice := Notice{ - Account: account, - Args: map[string]any{ - "summary": e.Subject, - "body": e.sender() + " — " + account, - "app_name": "mesh-mail", - }, - Handles: []string{e.Handle()}, - Latest: e, - } - if err := n.show(notice); err != nil { - return err - } + notice.Handles = append(notice.Handles, e.Handle()) } - return nil + return notice } // show sends one notice, marks every message it accounts for seen, keeps the account's status, and emits -// `shown`. +// `shown`. Nothing is marked, kept or emitted when Send fails: the caller keeps the notice pending instead. func (n *Notifier) show(notice Notice) error { if err := n.Send(notice.Args); err != nil { n.Logf("[mesh-mail-notifier] %s's notice could not be shown: %v", notice.Account, err) return err } + n.mu.Lock() + for _, h := range notice.Handles { + delete(n.buffered, h) + } + n.mu.Unlock() for _, h := range notice.Handles { if err := n.Store.MarkHandle(h); err != nil { n.Logf("[mesh-mail-notifier] shown, but not kept as shown (%v): a redelivery may show it again", err) @@ -286,22 +463,46 @@ func (n *Notifier) show(notice Notice) error { } // Status is the notifier's account of itself, for mail-notifier.status (novox/hq ADR 0378 §7: "accounts it -// announces there, when it last showed a notice"). +// announces there, when it last showed a notice"; review round 1 of issue 800: the accounts +// mail-store.accounts gives it, and the failures it is retrying). func (n *Notifier) Status() (map[string]any, error) { statuses, err := n.Store.Statuses() if err != nil { return nil, err } + failures, err := n.Store.Failures() + if err != nil { + return nil, err + } + known, knownErr := n.readAccounts() accounts := map[string]any{} + for account := range known { + accounts[account] = map[string]any{} + } for account, s := range statuses { accounts[account] = map[string]any{"at": s.At.UTC().Format(time.RFC3339), "shown": s.Summary} } - return map[string]any{ + out := map[string]any{ "accounts": accounts, "rules": []string{ "it shows each new inbox mail at most once, through node-notifier.send on this machine (novox/hq ADR 0378 §7)", "quiet during a first sync, for mail from the account's own address, for mail already marked read, and for anything outside the inbox", - "more than three new mails from one account arriving close together become one notice naming the count and the latest sender", + "of one account's qualifying mail within a sliding minute, the first three are shown one by one; the rest of that minute is folded into one notice naming the count and the latest sender", }, - }, nil + } + if knownErr != nil { + out["accounts-error"] = knownErr.Error() + } + if len(failures) > 0 { + failing := map[string]any{} + for account, f := range failures { + failing[account] = map[string]any{ + "count": f.Count, + "last-error": f.LastError, + "since": f.Since.UTC().Format(time.RFC3339), + } + } + out["failing"] = failing + } + return out, nil } diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go index 99582247..5a191ad6 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go @@ -1,6 +1,8 @@ package main import ( + "errors" + "fmt" "strings" "sync" "testing" @@ -12,10 +14,15 @@ type fakeStore struct { mu sync.Mutex handles map[string]bool statuses map[string]AccountStatus + failures map[string]FailureStatus } func newFakeStore() *fakeStore { - return &fakeStore{handles: map[string]bool{}, statuses: map[string]AccountStatus{}} + return &fakeStore{ + handles: map[string]bool{}, + statuses: map[string]AccountStatus{}, + failures: map[string]FailureStatus{}, + } } func (s *fakeStore) HandleSeen(handle string) (bool, error) { @@ -48,7 +55,31 @@ func (s *fakeStore) Statuses() (map[string]AccountStatus, error) { return out, nil } -// fakeClock is a clock the test moves by hand, so Flush's debounce needs no real sleep. +func (s *fakeStore) PutFailure(account string, f FailureStatus) error { + s.mu.Lock() + defer s.mu.Unlock() + s.failures[account] = f + return nil +} + +func (s *fakeStore) ClearFailure(account string) error { + s.mu.Lock() + defer s.mu.Unlock() + delete(s.failures, account) + return nil +} + +func (s *fakeStore) Failures() (map[string]FailureStatus, error) { + s.mu.Lock() + defer s.mu.Unlock() + out := map[string]FailureStatus{} + for k, v := range s.failures { + out[k] = v + } + return out, nil +} + +// fakeClock is a clock the test moves by hand, so Flush's window needs no real sleep. type fakeClock struct { mu sync.Mutex now time.Time @@ -66,104 +97,300 @@ func (c *fakeClock) Advance(d time.Duration) { c.now = c.now.Add(d) } -// TestFirstMailOfAnAccountIsShownOnceEachAndABurstIsFolded is the issue's first test (novox/hq ADR 0378 §9, -// issue 800 part B): given a new inbox mail from another sender, one already read, one from the account's own -// address, one in another folder, and five within a minute from one account, it sends exactly one notice, -// nothing, nothing, nothing, and one notice folding the five and naming the latest sender; it fails today -// because the module does not exist. -func TestFirstMailOfAnAccountIsShownOnceEachAndABurstIsFolded(t *testing.T) { +func newTestNotifier(t *testing.T, clock *fakeClock, store *fakeStore, sent *[]map[string]any) *Notifier { + t.Helper() + return &Notifier{ + Now: clock.Now, + Send: func(args map[string]any) error { + *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, + } +} + +func mail(account string, uid int, messageID, subject, senderName string) EventBody { + return EventBody{Account: account, Folder: "INBOX", UID: uint32(uid), MessageID: messageID, Subject: subject, + From: []Person{{Name: senderName, Address: "s@example.com"}}} +} + +// TestFlushShowsAccountsInASortedOrderNotAMapsIterationOrder is the review's first test for BLOCKING 1: Flush +// used to walk a map, so which account's notice went out first was whichever order Go's map iteration gave +// that run. Run with -count=50: green every time only once Flush sorts the accounts itself. +func TestFlushShowsAccountsInASortedOrderNotAMapsIterationOrder(t *testing.T) { clock := &fakeClock{now: time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC)} store := newFakeStore() - var mu sync.Mutex var sent []map[string]any - var shown []struct{ account, id string } + n := newTestNotifier(t, clock, store, &sent) + for _, account := range []string{"zeta", "mango", "alpha", "delta"} { + if err := n.Add(mail(account, 1, account+"-m1", account, "Someone")); err != nil { + t.Fatalf("Add(%s): %v", account, err) + } + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + + want := []string{"alpha", "delta", "mango", "zeta"} + if len(sent) != len(want) { + t.Fatalf("sent %d notice(s), want %d: %+v", len(sent), len(want), sent) + } + for i, account := range want { + if sent[i]["summary"] != account { + t.Fatalf("sent[%d]'s summary is %v, want %q: accounts must be flushed in sorted order: %+v", + i, sent[i]["summary"], account, sent) + } + } +} + +// TestAFailedSendIsRetriedWithoutLosingItOrHoldingUpOtherAccounts is the review's second test for BLOCKING 2: +// a notice node-notifier.send refuses stays pending (nothing is deleted before it is shown), is tried again +// at the next Flush no sooner than RetryBackoff, and another account's notice still goes out meanwhile. +func TestAFailedSendIsRetriedWithoutLosingItOrHoldingUpOtherAccounts(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 { - mu.Lock() - defer mu.Unlock() + if args["summary"] == "Broken" && 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{"work": "me@work.example", "home": "me@home.example"}, nil - }, - Store: store, - EmitShown: func(account, messageID string) error { - mu.Lock() - defer mu.Unlock() - shown = append(shown, struct{ account, id string }{account, messageID}) - return nil - }, - Logf: t.Logf, + Accounts: func() (map[string]string, error) { return map[string]string{}, nil }, + Store: store, + EmitShown: func(string, string) error { return nil }, + Logf: t.Logf, } - // 1. A new inbox mail from another sender: shown. - one := EventBody{Account: "work", Folder: "INBOX", UID: 1, MessageID: "m1", Subject: "Hi", - From: []Person{{Name: "Alice", Address: "alice@example.com"}}} - // 2. Already read. - two := EventBody{Account: "work", Folder: "INBOX", UID: 2, MessageID: "m2", Subject: "Old", - From: []Person{{Address: "bob@example.com"}}, Flags: []string{"\\Seen"}} - // 3. From the account's own address. - three := EventBody{Account: "work", Folder: "INBOX", UID: 3, MessageID: "m3", Subject: "Sent", - From: []Person{{Address: "me@work.example"}}} - // 4. In another folder. - four := EventBody{Account: "work", Folder: "Archive", UID: 4, MessageID: "m4", Subject: "Filed", - From: []Person{{Address: "carol@example.com"}}} - for _, e := range []EventBody{one, two, three, four} { - if err := n.Add(e); err != nil { - t.Fatalf("Add(%+v): %v", e, err) - } + if err := n.Add(mail("broke", 1, "b1", "Broken", "Someone")); err != nil { + t.Fatalf("Add(broke): %v", err) } - - // 5. Five new inbox mails from another account within a minute. - var burst []EventBody - for i := 1; i <= 5; i++ { - e := EventBody{Account: "home", Folder: "INBOX", UID: uint32(i), MessageID: "burst" + string(rune('0'+i)), - Subject: "Mail", From: []Person{{Name: "Sender " + string(rune('0'+i)), Address: "s@example.com"}}} - burst = append(burst, e) - clock.Advance(time.Second) - if err := n.Add(e); err != nil { - t.Fatalf("Add(%+v): %v", e, err) - } + if err := n.Add(mail("ok", 1, "o1", "Fine", "Someone")); err != nil { + t.Fatalf("Add(ok): %v", err) } - latest := burst[len(burst)-1] - - clock.Advance(BurstWindow + time.Second) if err := n.Flush(); err != nil { t.Fatalf("Flush: %v", err) } - - if len(sent) != 2 { - t.Fatalf("sent %d notice(s): %+v", len(sent), sent) + if len(sent) != 1 || sent[0]["summary"] != "Fine" { + t.Fatalf("sent = %+v, want only ok's notice: broke's failure must not hold it up", sent) } - if sent[0]["summary"] != "Hi" || !strings.Contains(sent[0]["body"].(string), "Alice") { - t.Errorf("the first notice is %+v, want Alice's mail Hi", sent[0]) - } - if sent[1]["summary"] != "5 new mails" || !strings.Contains(sent[1]["body"].(string), latest.sender()) { - t.Errorf("the folded notice is %+v, want 5 new mails naming %s", sent[1], latest.sender()) + if failures, err := store.Failures(); err != nil || failures["broke"].Count != 1 { + t.Fatalf("Store.Failures() = %+v, %v; want broke recorded once", failures, err) } - if len(shown) != 2 { - t.Fatalf("emitted shown %d time(s): %+v", len(shown), shown) - } - if shown[0].account != "work" || shown[0].id != "m1" { - t.Errorf("the first shown is %+v, want work/m1", shown[0]) - } - if shown[1].account != "home" || shown[1].id != latest.MessageID { - t.Errorf("the folded shown is %+v, want home/%s (the latest)", shown[1], latest.MessageID) - } - - // Redelivering the first event again shows nothing more: it was marked seen. - if err := n.Add(one); err != nil { - t.Fatalf("Add(one) again: %v", err) - } - clock.Advance(BurstWindow + time.Second) + // Before RetryBackoff has passed, broke's notice is not retried yet. + clock.Advance(30 * time.Second) if err := n.Flush(); err != nil { t.Fatalf("Flush: %v", err) } - if len(sent) != 2 { - t.Fatalf("a redelivered event was shown again: sent now %+v", sent) + if len(sent) != 1 { + t.Fatalf("sent %d notice(s) before RetryBackoff elapsed, want 1 (broke not retried yet): %+v", len(sent), sent) + } + + // Once RetryBackoff has passed, the next Flush tries it again; nothing was lost. + failSend = false + clock.Advance(RetryBackoff) + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 2 || sent[1]["summary"] != "Broken" { + t.Fatalf("sent = %+v, want broke's notice too, after RetryBackoff", sent) + } + if failures, _ := store.Failures(); len(failures) != 0 { + t.Errorf("Store.Failures() = %+v, want it cleared once broke's notice was shown", failures) + } +} + +// TestABurstShowsTheFirstThreeAtOnceAndFoldsTheRestAtTheWindowsEnd is the review's third test (BLOCKING 3, +// ADR 0378 §9, the lead's reading): the first three of an account's qualifying mail within a minute are shown +// one by one as Flush runs; the fourth onward is folded and shown as one notice once the window — a minute +// from the first of them — is over. +func TestABurstShowsTheFirstThreeAtOnceAndFoldsTheRestAtTheWindowsEnd(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) + } + clock.Advance(10 * time.Second) + } + if len(sent) != 3 { + t.Fatalf("4 mails 10s apart: sent %d notice(s) before the window ends, want 3 shown one by one: %+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 window is over a minute after it opened (the first mail): the fourth, folded, is shown now. + clock.Advance(Window) + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + 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]) + } +} + +// TestARedeliveredEventIsNeverShownOrCountedTwice is the review's fourth test: a redelivery of a mail already +// shown is dropped (the Store knows it), and a redelivery of a mail still buffered, waiting to be folded, is +// never counted a second time toward the fold. +func TestARedeliveredEventIsNeverShownOrCountedTwice(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" + one := mail(account, 1, "m1", "Hi", "Alice") + if err := n.Add(one); err != nil { + t.Fatalf("Add: %v", err) + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 1 { + t.Fatalf("sent %d notice(s), want 1", len(sent)) + } + if err := n.Add(one); err != nil { // redelivered after it was shown + t.Fatalf("Add (redelivered, already shown): %v", err) + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 1 { + t.Fatalf("a redelivered, already-shown event was shown again: sent now %+v", sent) + } + + for i := 2; i <= 4; i++ { + if err := n.Add(mail(account, i, fmt.Sprintf("m%d", i), "Hi", fmt.Sprintf("Sender %d", i))); err != nil { + t.Fatalf("Add mail %d: %v", i, err) + } + } + fourth := mail(account, 4, "m4", "Hi", "Sender 4") + if err := n.Add(fourth); err != nil { // redelivered while still buffered, folded, not yet shown + t.Fatalf("Add (redelivered, buffered): %v", err) + } + + clock.Advance(Window) + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + 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]) + } +} + +// TestDedupeKeyIsTheMessageIDAcrossAUIDVALIDITYChange is the review's fifth test: the dedupe key is the +// account and the message id wherever one is given, so a UIDVALIDITY reset — which changes every UID in a +// folder — cannot make an already-shown mail look new. Only a message with no message id falls back to its +// folder, UIDVALIDITY and UID, and that fallback still tells two different mails apart. +func TestDedupeKeyIsTheMessageIDAcrossAUIDVALIDITYChange(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" + first := EventBody{Account: account, Folder: "INBOX", UID: 5, UIDValidity: 1, MessageID: "stable-id", + Subject: "Hi", From: []Person{{Address: "a@example.com"}}} + if err := n.Add(first); err != nil { + t.Fatalf("Add: %v", err) + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 1 { + t.Fatalf("sent %d, want 1", len(sent)) + } + + // The same message, redelivered after a UIDVALIDITY reset changed its UID: same message id, not shown again. + resent := EventBody{Account: account, Folder: "INBOX", UID: 99, UIDValidity: 2, MessageID: "stable-id", + Subject: "Hi", From: []Person{{Address: "a@example.com"}}} + if err := n.Add(resent); err != nil { + t.Fatalf("Add (UIDVALIDITY reset, same message id): %v", err) + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 1 { + t.Fatalf("a mail with the same message id after a UIDVALIDITY reset was shown again: sent now %+v", sent) + } + + // Two different mails with no message id, at the same UID but a different UIDVALIDITY, are not confused. + noID1 := EventBody{Account: account, Folder: "INBOX", UID: 7, UIDValidity: 1, Subject: "No id", + From: []Person{{Address: "a@example.com"}}} + noID2 := EventBody{Account: account, Folder: "INBOX", UID: 7, UIDValidity: 2, Subject: "No id, new validity", + From: []Person{{Address: "a@example.com"}}} + if err := n.Add(noID1); err != nil { + t.Fatalf("Add noID1: %v", err) + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if err := n.Add(noID2); err != nil { + t.Fatalf("Add noID2: %v", err) + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 3 { + t.Fatalf("sent %d, want 3: noID1 and noID2 are different mails, their UIDVALIDITY differs: %+v", len(sent), sent) + } +} + +// TestQualifyingRulesStillHold keeps the original qualifying rules proven (an already-read mail, a mail from +// the account's own address, and a mail outside the inbox are never shown), now that buffering is per window +// rather than per quiet period. +func TestQualifyingRulesStillHold(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 := &Notifier{ + Now: clock.Now, + Send: func(args map[string]any) error { sent = append(sent, args); return nil }, + Accounts: func() (map[string]string, error) { return map[string]string{"work": "me@work.example"}, nil }, + Store: store, + EmitShown: func(string, string) error { return nil }, + Logf: t.Logf, + } + already := EventBody{Account: "work", Folder: "INBOX", UID: 2, MessageID: "m2", Subject: "Old", + From: []Person{{Address: "bob@example.com"}}, Flags: []string{"\\Seen"}} + own := EventBody{Account: "work", Folder: "INBOX", UID: 3, MessageID: "m3", Subject: "Sent", + From: []Person{{Address: "me@work.example"}}} + elsewhere := EventBody{Account: "work", Folder: "Archive", UID: 4, MessageID: "m4", Subject: "Filed", + From: []Person{{Address: "carol@example.com"}}} + for _, e := range []EventBody{already, own, elsewhere} { + if err := n.Add(e); err != nil { + t.Fatalf("Add(%+v): %v", e, err) + } + } + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 0 { + t.Fatalf("sent %+v, want nothing: an already-read mail, the account's own mail and mail outside the inbox never qualify", sent) } } diff --git a/modules/mesh-mail-notifier/module.json b/modules/mesh-mail-notifier/module.json index 1d1cb773..15dbd40a 100644 --- a/modules/mesh-mail-notifier/module.json +++ b/modules/mesh-mail-notifier/module.json @@ -26,7 +26,8 @@ } ], "tools": [ - "mail-notifier.status" + "mail-notifier.status", + "mail_notifier_health" ], "own-secrets": { "broker": "${dir:state}/broker" @@ -57,7 +58,12 @@ "notify" ], "health": { - "kind": "unit" + "kind": "tool", + "tool": "mail_notifier_health", + "interval": "60s", + "timeout": "10s", + "looks": 2, + "grace": "120s" } } ], -- 2.54.0 From b771794f1c865d389d84545312b2ccf6836d3fd5 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 18:33:22 +0200 Subject: [PATCH 07/10] issue 800: keep notify's health as unit; the gate refuses a one-process module judged only by its own tool ADR 0227 rule 8 / ADR 0240 rule 2 refuse a module with no health check of another kind beside a tool check. mesh-mail-notifier has one process (notify); wiring mail_notifier_health as its manifest health, as the review note's wording suggested (mirroring mesh-mail's watch/mail_health), trips that rule because mesh-mail also has keep (kind unit) beside watch. mail_notifier_health stays built, tested and callable; notify's manifest health stays kind unit until the lead decides how a one-process module gets this automatically. --- modules/mesh-mail-notifier/README.md | 2 +- .../cmd/mesh-mail-notifier/health.go | 13 +++++++++++-- modules/mesh-mail-notifier/module.json | 7 +------ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/modules/mesh-mail-notifier/README.md b/modules/mesh-mail-notifier/README.md index 2ce347ac..a711efff 100644 --- a/modules/mesh-mail-notifier/README.md +++ b/modules/mesh-mail-notifier/README.md @@ -37,4 +37,4 @@ accounts it announces, in mesh-mail's trusted setting `grants` (`mail-store`'s s | tool | does | |---|---| | `mail-notifier.status` | every account it is granted, when it last showed each one a notice, and any notice still failing to send (its count, last error, since when). Read-only | -| `mail_notifier_health` | how it is: unhealthy once a notice has failed to show for more than ten minutes, healthy otherwise, even mid-retry. Read-only | +| `mail_notifier_health` | how it is: unhealthy once a notice has failed to show for more than ten minutes, healthy otherwise, even mid-retry. Not (yet) the node-engine's own check of `notify` — one process cannot be judged only by its own tool (ADR 0227 rule 8); `notify`'s manifest health stays `unit` until the lead decides how a one-process module gets this automatically. Read-only | diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go index 324b6af2..1ace5050 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/health.go @@ -3,11 +3,20 @@ package main // mesh-mail-notifier's own health (review round 1 of issue 800, which added this tool beside mail-notifier's // seat verb status, as mesh-mail's own health tool is mail_health beside its seat tools). A notice that fails // to send stays pending and is retried, at most once a minute (Flush, notifier.go); the account's -// failing-since stays in the Store, so this tool — its own process, asked separately by the node-engine — -// sees the same account the live notifier does, with no state of its own to lose between asks. +// failing-since stays in the Store, so this tool — its own process, asked separately by the node-engine or by +// mesh_call — sees the same account the live notifier does, with no state of its own to lose between asks. // // Unhealthy once an account's notice has failed to show for more than UnhealthyAfter; healthy otherwise, even // while a notice is mid-retry. +// +// STOP (review round 1 of issue 800, not decided by its note): this tool is served and tested, but module.json +// does not name it as the notify process's own health check — mesh-mail-notifier has one process (notify), and +// the catalogue's gate refuses a module judged only by its own tool, with no check of another kind beside it +// (ADR 0227 rule 8, ADR 0240 rule 2; internal/catalogue/health.go ~297 in mesh-controller). mesh-mail avoids +// this because it has two processes — keep (kind unit) beside watch (kind tool, mail_health); mesh-mail-notifier +// has only notify, so wiring this tool the same way trips the gate. notify's health stays kind unit (liveness, +// as it was before this round) until the lead decides how a one-process module gets an automatic, node-engine +// enforced health of this kind: a second process judged some other way, or a change to the gate's rule. import ( "fmt" diff --git a/modules/mesh-mail-notifier/module.json b/modules/mesh-mail-notifier/module.json index 15dbd40a..1aa8910a 100644 --- a/modules/mesh-mail-notifier/module.json +++ b/modules/mesh-mail-notifier/module.json @@ -58,12 +58,7 @@ "notify" ], "health": { - "kind": "tool", - "tool": "mail_notifier_health", - "interval": "60s", - "timeout": "10s", - "looks": 2, - "grace": "120s" + "kind": "unit" } } ], -- 2.54.0 From 66713c5a1e264d6ff691230f5737c7c1de5a31de Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 18:50:21 +0200 Subject: [PATCH 08/10] issue 800: fix review round 2 findings in mesh-mail-notifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../cmd/mesh-mail-notifier/notifier.go | 118 +++++++++----- .../cmd/mesh-mail-notifier/notifier_test.go | 148 +++++++++++++++++- 2 files changed, 225 insertions(+), 41 deletions(-) diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go index 27e0d88d..e4fa191a 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go @@ -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 { diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go index 5a191ad6..841dd86a 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go @@ -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]) + } +} -- 2.54.0 From aa62de2e786028666a40ee0f862611b491cc311c Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 18:51:26 +0200 Subject: [PATCH 09/10] 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) -- 2.54.0 From c2cd2e1eb15e7ed6fef83465249de3034a1911e8 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 19:03:05 +0200 Subject: [PATCH 10/10] issue 800: fix review round 3 finding, a true sliding window instead of a session quota A session that only reset after a quiet minute kept counting, and so folding, a steady trickle of mail forever even with no burst (one mail every 50s). N is now the account's qualifying mail arrived in the trailing 60 seconds, counted fresh on every arrival; a fold holds only its own overflow and the notice names just that count, not the whole history. --- .../cmd/mesh-mail-notifier/notifier.go | 127 +++++++------- .../cmd/mesh-mail-notifier/notifier_test.go | 157 +++++++++++------- 2 files changed, 151 insertions(+), 133 deletions(-) diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go index e4fa191a..6f8f997d 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier.go @@ -4,12 +4,17 @@ 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, 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). +// and for anything outside the inbox. For each new qualifying mail of an account, arriving at t, N counts that +// account's qualifying mail — shown or folded alike, each counted once — that arrived in the 60 seconds up to +// and including t: N <= FoldOver is shown at once; N > FoldOver is folded. The first folded mail opens the +// account's fold; every folded mail joins it; the fold is shown, as one notice naming how many it holds and +// the latest sender, once Window has passed since its own first mail — after which a new fold may open. The +// count is a true sliding window of arrivals, not a per-session quota that only resets after a quiet Window: +// review round 2's reading kept a session's count alive, and so folding, for as long as any gap stayed under +// Window, which folds a steady trickle (one mail every 50s, say) forever even though no burst ever happened; +// review round 3 of issue 800 replaces that reading with this one. Nothing pending — a folded mail awaiting +// its fold's deadline, or a failed send awaiting its retry — is ever dropped (novox/hq ADR 0378 §9, review +// round 3 of issue 800, which replaces the round 1 and round 2 readings). // // Nothing here names Betterbird (ADR 0379 §3). @@ -31,14 +36,11 @@ 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. 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). +// Window is the trailing 60 seconds ADR 0378 §9 counts a mail's N over (every qualifying mail of the account, +// shown or folded alike, that arrived in the 60 seconds up to and including this one), and the time a fold +// waits, from its own first mail, before it is shown (review round 3 of issue 800, which replaces the round 1 +// and round 2 readings: the count is a true sliding window of arrivals, not a per-session quota that stays +// alive, and so keeps folding, for as long as any gap between mails stays under Window). const Window = time.Minute // RetryBackoff is how long the notifier waits, after an account's notice fails to send, before it tries that @@ -147,16 +149,14 @@ type AccountStatus struct { Summary string `json:"summary"` } -// 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. +// accountBuffer is one account's state: history is every qualifying mail's arrival time, kept only for the +// trailing Window, that N is counted against (review round 3 of issue 800); immediate holds mail waiting to be +// shown one by one at the next Flush; folded holds the account's current fold (its first FoldOver mails within +// Window are never folded, so a fold only ever holds the overflow from the fourth qualifying mail of its own +// window onward), due as one notice once Window has passed since foldStart, its first mail's arrival. type accountBuffer struct { - 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) + history []time.Time + foldStart time.Time // when the current fold's first mail arrived; zero while nothing is folded immediate []EventBody folded []EventBody } @@ -229,23 +229,28 @@ func (n *Notifier) Add(e EventBody) error { } now := n.Now() b := n.pending[e.Account] - // 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) { + if b == nil { b = &accountBuffer{} n.pending[e.Account] = b } - b.lastArrival = now - b.count++ + // N is every qualifying mail of this account — shown or folded alike, each counted once — that arrived in + // the 60 seconds up to and including now: a true sliding window over arrivals, not a per-session quota that + // stays alive, and so keeps folding, for as long as any gap stays under Window (review round 3 of issue + // 800, which replaces the round 1 and round 2 readings; nothing in this count depends on whether an earlier + // mail has been shown yet). + cutoff := now.Add(-Window) + history := make([]time.Time, 0, len(b.history)+1) + for _, at := range b.history { + if !at.Before(cutoff) { + history = append(history, at) + } + } + b.history = append(history, now) n.buffered[handle] = true - if b.count <= FoldOver { + if len(b.history) <= FoldOver { b.immediate = append(b.immediate, e) } else { - if b.foldStart.IsZero() { + if len(b.folded) == 0 { b.foldStart = now } b.folded = append(b.folded, e) @@ -320,10 +325,10 @@ func (n *Notifier) Flush() error { return nil } -// 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). +// flushAccount ships one account's ready notices (its immediate mail, and its fold once Window has passed +// since the fold's own first mail), 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 pending is ever lost +// (review round 2 BLOCKING, kept under review round 3's rule; see Add). func (n *Notifier) flushAccount(account string, now time.Time) { n.mu.Lock() if cd, ok := n.cooldown[account]; ok && now.Before(cd) { @@ -336,19 +341,15 @@ func (n *Notifier) flushAccount(account string, now time.Time) { return } immediate := append([]EventBody(nil), b.immediate...) - // 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 + // The fold is due Window after its own first mail (review round 3 of issue 800). + foldDue := len(b.folded) > 0 && now.Sub(b.foldStart) >= Window var fold []EventBody - 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 { - n.closeIfIdle(account, b, now) return } @@ -363,7 +364,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, foldCount)); err != nil { + if err := n.show(n.foldNotice(account, fold, len(fold))); err != nil { failErr = err } else { foldSent = true @@ -376,19 +377,13 @@ func (n *Notifier) flushAccount(account string, now time.Time) { if foldSent { cur.folded = cur.folded[len(fold):] if len(cur.folded) == 0 { - cur.foldStart = time.Time{} + cur.foldStart = time.Time{} // nothing left folded: a new fold may open (review round 3 of issue 800) } 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 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) - } } if failErr != nil { n.cooldown[account] = now.Add(RetryBackoff) @@ -400,17 +395,6 @@ func (n *Notifier) flushAccount(account string, now time.Time) { n.mu.Unlock() } -// 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 && now.Sub(cur.lastArrival) >= Window && - len(cur.immediate) == 0 && len(cur.folded) == 0 { - delete(n.pending, account) - } -} - // recordFailureLocked keeps an account's failure in the Store (mu held by the caller, released around the // Store calls since they may reach the bus): its count, its last error, and since when, so a later Flush and // the health tool agree on how long it has been failing. @@ -452,17 +436,17 @@ func (n *Notifier) singleNotice(account string, e 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). +// foldNotice is one notice for everything the account's current fold holds — its qualifying mail from the +// fourth within some trailing 60 seconds onward — naming how many mail that fold holds (not the account's +// whole history of arrivals) and the latest sender (novox/hq ADR 0378 §9, review round 3 of issue 800, which +// replaces the round 1 and round 2 readings: " more new mail(s) in , latest from ", "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(count) + " new " + mailWord(count) + " in " + account, + "summary": strconv.Itoa(count) + " more new " + mailWord(count) + " in " + account, "body": "latest from " + latest.sender(), "app_name": "mesh-mail", }, @@ -474,7 +458,8 @@ func (n *Notifier) foldNotice(account string, events []EventBody, count int) Not return notice } -// mailWord says "mail" for a count of one, "mails" for anything else (review round 2 of issue 800, bullet 3). +// mailWord says "mail" for a count of one, "mails" for anything else (review round 2 of issue 800, bullet 3; +// kept by review round 3). func mailWord(count int) string { if count == 1 { return "mail" @@ -531,7 +516,7 @@ func (n *Notifier) Status() (map[string]any, error) { "rules": []string{ "it shows each new inbox mail at most once, through node-notifier.send on this machine (novox/hq ADR 0378 §7)", "quiet during a first sync, for mail from the account's own address, for mail already marked read, 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 is folded into one notice naming the count and the latest sender", + "of one account's qualifying mail, one arriving when fewer than three of the account's mails fell in the 60 seconds before it is shown at once; the rest is folded and shown, as one notice naming how many it holds and the latest sender, 60 seconds after the fold's own first mail", }, } if knownErr != nil { diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go index 841dd86a..3592c95b 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/notifier_test.go @@ -210,9 +210,9 @@ func TestAFailedSendIsRetriedWithoutLosingItOrHoldingUpOtherAccounts(t *testing. } // TestABurstShowsTheFirstThreeAtOnceAndFoldsTheRestAtTheWindowsEnd is the review's third test (BLOCKING 3, -// ADR 0378 §9, the lead's reading): the first three of an account's qualifying mail within a minute are shown -// one by one as Flush runs; the fourth onward is folded and shown as one notice once the window — a minute -// from the first of them — is over. +// ADR 0378 §9, the lead's reading, the fold notice's wording updated by review round 3 of issue 800): the +// first three of an account's qualifying mail within a minute are shown one by one as Flush runs; the fourth +// onward is folded and shown as one notice once the window — a minute from the first of them — is over. func TestABurstShowsTheFirstThreeAtOnceAndFoldsTheRestAtTheWindowsEnd(t *testing.T) { clock := &fakeClock{now: time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC)} store := newFakeStore() @@ -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"] != "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]) + if sent[3]["summary"] != "1 more new mail in home" || !strings.Contains(sent[3]["body"].(string), "Sender 4") { + t.Errorf("the fold notice is %+v, want it to count only what the fold holds (1), 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"] != "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]) + if sent[3]["summary"] != "1 more new mail in home" { + t.Errorf("the fold notice is %+v, want it to count the redelivered fourth mail once, not twice, among the fold's 1", sent[3]) } } @@ -450,53 +450,15 @@ func TestABufferIsNeverReplacedWhileItHoldsAnUnsentMail(t *testing.T) { } } -// 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) { +// TestTheFoldFollowsATrueSlidingWindowOfArrivalsNotASessionQuota is review round 3 of issue 800's first test, +// replacing round 2's (which contradicted round 3's rule): N, for a mail arriving at t, is every qualifying +// mail of the account — shown or folded alike — that arrived in the 60 seconds up to and including t. Mails at +// 0, 50, 55, 70, 75 and 80 seconds: at 70s the preceding 60 seconds (from 10s) hold 50, 55 and 70 themselves +// (0s has aged out), N=3, so 70 is still shown at once, not folded; at 75s the preceding 60 seconds (from 15s) +// still hold 50 too, so it, 55, 70 and 75 themselves make N=4, and 75 folds; 80 folds with it (its own +// preceding 60 seconds hold 50, 55, 70, 75 and 80, N=5). The fold is due 60 seconds after its own first mail +// (75s), at 135s, as one notice naming only what it holds (2) and the latest sender (80s's). +func TestTheFoldFollowsATrueSlidingWindowOfArrivalsNotASessionQuota(t *testing.T) { clock := &fakeClock{now: time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC)} store := newFakeStore() var sent []map[string]any @@ -513,24 +475,95 @@ func TestTheWindowSlidesOnGapsNotOnElapsedTimeSinceItsFirstMail(t *testing.T) { 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) + if len(sent) != 4 { + t.Fatalf("sent %d notice(s), want 4 shown one by one (0s, 50s, 55s, 70s): %+v", len(sent), sent) } - for i, want := range []string{"Mail 1", "Mail 2", "Mail 3"} { + for i, want := range []string{"Mail 1", "Mail 2", "Mail 3", "Mail 4"} { 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) + // The fold's own window — a minute from its first mail, 75s — ends at 135s. + clock.Advance(55 * time.Second) // now at 80s + 55s = 135s + if err := n.Flush(); err != nil { + t.Fatalf("Flush: %v", err) + } + if len(sent) != 5 { + t.Fatalf("sent %d notice(s), want 5 (75s and 80s folded into one, at 135s): %+v", len(sent), sent) + } + if sent[4]["summary"] != "2 more new mails in home" || !strings.Contains(sent[4]["body"].(string), "Sender 6") { + t.Errorf("the fold notice is %+v, want it to count only what it holds (2), naming Sender 6 (80s, the latest)", sent[4]) + } +} + +// TestASteadyTrickleNeverFoldsForever is review round 3 of issue 800's second test: the bug the round found +// (a session that only resets after a quiet minute folds a steady stream forever, even without a burst). One +// mail every 50 seconds, 20 of them: each one's preceding 60 seconds holds only the one before it, so N never +// exceeds 2, and all 20 are shown at once — none ever fold. +func TestASteadyTrickleNeverFoldsForever(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 <= 20; i++ { + if i > 1 { + clock.Advance(50 * time.Second) + } + 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 at mail %d: %v", i, err) + } + if len(sent) != i { + t.Fatalf("after mail %d, sent %d notice(s), want %d: a steady trickle must never fold: %+v", i, len(sent), i, sent) + } + } + for i, got := range sent { + want := fmt.Sprintf("Mail %d", i+1) + if got["summary"] != want { + t.Errorf("sent[%d] = %+v, want summary %q", i, got, want) + } + } +} + +// TestFiveMailsWithinTenSecondsFoldAfterThree is review round 3 of issue 800's third test: five mails within +// ten seconds — the first three shown at once, the fourth opens a fold, the fifth joins it, and the fold is +// shown 60 seconds after the fourth (the fold's own first mail) as one notice. +func TestFiveMailsWithinTenSecondsFoldAfterThree(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 <= 5; i++ { + if i > 1 { + clock.Advance(2 * time.Second) + } + 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 at mail %d: %v", i, err) + } + } + if len(sent) != 3 { + t.Fatalf("sent %d notice(s), want 3 shown one by one, the fourth and fifth folded, not yet due: %+v", len(sent), sent) + } + + // The fold is due 60 seconds after its own first mail (the fourth, at 6s). + clock.Advance(Window) 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) + t.Fatalf("sent %d notice(s), want 4 (one fold notice carrying the fourth and fifth): %+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]) + if sent[3]["summary"] != "2 more new mails in home" || !strings.Contains(sent[3]["body"].(string), "Sender 5") { + t.Errorf("the fold notice is %+v, want it to count only what it holds (2), naming Sender 5 (the latest)", sent[3]) } } -- 2.54.0