From 12d78520824a8f2dda111f7da39ef2772d5e6365 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 23:13:34 +0200 Subject: [PATCH 1/2] mesh-mail-notifier: the watcher runs in the base invocation, not a separate notify process (issue 859) The notify process's stdin is empty under systemd, so stdio.Serve returned at EOF, main returned, and systemd restarted it every five seconds: the watch loop never ran long enough to show a notice. The watch loop now starts beside stdio.Serve in the one process the node's runtime launches and keeps alive, as mesh-knowledge's serve does. No resource declares a separate notify process any more. --- modules/mesh-mail-notifier/README.md | 2 +- .../cmd/mesh-mail-notifier/main.go | 30 ++++++----- .../cmd/mesh-mail-notifier/manifest_test.go | 38 +++++++++++++ .../cmd/mesh-mail-notifier/run_test.go | 53 +++++++++++++++++++ modules/mesh-mail-notifier/module.json | 13 ----- 5 files changed, 109 insertions(+), 27 deletions(-) create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go create mode 100644 modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go diff --git a/modules/mesh-mail-notifier/README.md b/modules/mesh-mail-notifier/README.md index a711efff..9891e96b 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. 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 | +| `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 — this is one process, judged only by its own tool (ADR 0227 rule 8); a watching process of its own is issue 841, not yet decided. Read-only | 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 36bc9180..bacff255 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go @@ -1,13 +1,16 @@ // 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 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. +// §2): the one process serves mail-notifier.status and the module's own health tool (mail_notifier_health, +// review round 1 of issue 800), and, beside stdio.Serve, hears mesh-mail's added events and shows them +// (exactly as modules/mesh-knowledge's serve starts go hear(k) and go background(k) before stdio.Serve) — not +// a separate `notify` process, which systemd gave an empty stdin and so exited at once, every five seconds +// (novox/hq issue 859). What this module says, it says on stderr; stdout is the MCP channel. package main import ( "encoding/json" "fmt" + "io" "os" "strings" "time" @@ -42,15 +45,6 @@ func main() { } 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 " + @@ -67,12 +61,22 @@ func main() { return Judge(failures, time.Now()), nil }}, } - if err := stdio.Serve("", tools); err != nil { + if err := run(n, watch, os.Stdin, os.Stdout, tools); err != nil { logf("%v", err) os.Exit(1) } } +// run is main's startup path, factored so a test can run it: the watcher started before stdio.Serve, exactly +// as mesh-knowledge's serve starts go hear(k) and go background(k) before stdio.Serve — so the watcher keeps +// running whatever stdio.Serve does with the stream it is given, including returning at once when stdin is +// already at EOF, as systemd gave the old separate `notify` process (novox/hq issue 859). The node's runtime +// launches this one process and keeps it alive. +func run(n *Notifier, watcher func(*Notifier), in io.Reader, out io.Writer, tools []stdio.Tool) error { + go watcher(n) + return stdio.ServeOn("", tools, in, out) +} + // 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). diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go new file mode 100644 index 00000000..75b78836 --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go @@ -0,0 +1,38 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +// The manifest's own shape (novox/hq issue 859): the watch loop runs in the base invocation beside +// stdio.Serve, so no resource declares a separate `notify` process any more — the thing that exited at once, +// every five seconds, because a systemd-run process's stdin is empty and stdio.Serve returns at EOF. + +type manifestResources struct { + Resources []map[string]any `json:"resources"` +} + +func readManifestResources(t *testing.T) manifestResources { + t.Helper() + raw, err := os.ReadFile(filepath.Join("..", "..", "module.json")) + if err != nil { + t.Fatal(err) + } + var m manifestResources + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatalf("module.json: %v", err) + } + return m +} + +func TestNoProcessNamedNotifyIsDeclared(t *testing.T) { + m := readManifestResources(t) + for _, r := range m.Resources { + if r["type"] == "process" { + t.Fatalf("module.json still declares a process resource (%v): the watcher runs in the base invocation now, not a separate process (novox/hq issue 859)", r) + } + } +} diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go new file mode 100644 index 00000000..0cdcf86f --- /dev/null +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go @@ -0,0 +1,53 @@ +package main + +import ( + "io" + "strings" + "testing" + "time" +) + +// TestRunStartsTheWatcherEvenWhenStdinIsAtEOF proves the watcher runs beside stdio.Serve in the one process the +// node's runtime launches (novox/hq ADR 0378 §9, as mesh-knowledge's serve starts go hear(k) and go +// background(k) before stdio.Serve), not only in a separate process something keeps asking over stdin. run is +// given stdin already at EOF, exactly what systemd gave the old separate `notify` process: it exited at once, +// every five seconds (novox/hq issue 859). The watcher must still be running once run has returned. +func TestRunStartsTheWatcherEvenWhenStdinIsAtEOF(t *testing.T) { + started := make(chan struct{}) + finished := make(chan struct{}) + stop := make(chan struct{}) + defer close(stop) + + fakeWatcher := func(n *Notifier) { + close(started) + <-stop // the real watch never returns either: it loops until the process itself ends + close(finished) + } + + n := &Notifier{Store: newFakeStore()} + n.init() + + done := make(chan error, 1) + go func() { done <- run(n, fakeWatcher, strings.NewReader(""), io.Discard, nil) }() + + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("the watcher was not started before stdio.Serve returned") + } + + select { + case err := <-done: + if err != nil { + t.Fatalf("run over stdin at EOF: %v", err) + } + case <-time.After(time.Second): + t.Fatal("run did not return once stdin reached EOF, as systemd gives a process nobody talks to") + } + + select { + case <-finished: + t.Fatal("the watcher must not stop just because stdio.Serve returned: it is the base invocation's own watcher now, not a separate `notify` process that exits at EOF") + default: + } +} diff --git a/modules/mesh-mail-notifier/module.json b/modules/mesh-mail-notifier/module.json index 1aa8910a..dec4d77a 100644 --- a/modules/mesh-mail-notifier/module.json +++ b/modules/mesh-mail-notifier/module.json @@ -47,19 +47,6 @@ "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": { From 31120ed5802960866de164204c20efa76e34cd75 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 23:17:14 +0200 Subject: [PATCH 2/2] mesh-mail-notifier: watch retries a subscribe refusal instead of exiting (issue 859, review round 1) A subscribe the runtime refuses no longer calls os.Exit(1), which took mail-notifier.status and mail_notifier_health down with it: watch now retries with backoff (2s doubling to 1 minute), logging each refusal, as mesh-knowledge's hear does. Also: the run() test proves liveness by having the watcher answer a ping after run returns, instead of checking a channel only the test's own deferred cleanup could close; and the manifest test is renamed to say what it checks (no process resource at all). --- .../cmd/mesh-mail-notifier/main.go | 33 +++++--- .../cmd/mesh-mail-notifier/manifest_test.go | 2 +- .../cmd/mesh-mail-notifier/run_test.go | 79 ++++++++++++++++--- 3 files changed, 92 insertions(+), 22 deletions(-) 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 bacff255..8f482fb3 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/main.go @@ -80,19 +80,28 @@ func run(n *Notifier, watcher func(*Notifier), in io.Reader, out io.Writer, tool // 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 { - 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 +func watch(n *Notifier) { watchWith(n, stdio.Subscribe, time.Sleep) } + +// watchWith is watch with its subscribe and sleep factored out, so a test can make a subscribe fail without +// the real bus and without the real wait. A subscribe the runtime refuses is retried, 2 seconds doubling to +// at most a minute, logging each refusal — exactly as modules/mesh-knowledge's hear does — never fatal: a +// refusal here must never take mail-notifier.status and mail_notifier_health down with it (novox/hq issue +// 859, review round 1; the old os.Exit(1) did exactly that). +func watchWith(n *Notifier, subscribe func(string, func(stdio.Envelope) error) error, sleep func(time.Duration)) { + for wait := 2 * time.Second; ; wait = min(wait*2, time.Minute) { + sleep(wait) + err := 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 { + break } - return n.Add(body) - }) - if err != nil { - logf("[mesh-mail-notifier] could not hear %s: %v", AddedPattern, err) - os.Exit(1) + logf("[mesh-mail-notifier] does not hear %s yet (%v); asking again in %s", AddedPattern, err, wait) } ticker := time.NewTicker(1 * time.Second) defer ticker.Stop() diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go index 75b78836..2512db68 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/manifest_test.go @@ -28,7 +28,7 @@ func readManifestResources(t *testing.T) manifestResources { return m } -func TestNoProcessNamedNotifyIsDeclared(t *testing.T) { +func TestNoProcessResourceIsDeclared(t *testing.T) { m := readManifestResources(t) for _, r := range m.Resources { if r["type"] == "process" { diff --git a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go index 0cdcf86f..8a725595 100644 --- a/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go +++ b/modules/mesh-mail-notifier/cmd/mesh-mail-notifier/run_test.go @@ -1,27 +1,33 @@ package main import ( + "errors" "io" "strings" + "sync/atomic" "testing" "time" + + stdio "git.novox.be/novox/mesh-sdk/go" ) // TestRunStartsTheWatcherEvenWhenStdinIsAtEOF proves the watcher runs beside stdio.Serve in the one process the // node's runtime launches (novox/hq ADR 0378 §9, as mesh-knowledge's serve starts go hear(k) and go // background(k) before stdio.Serve), not only in a separate process something keeps asking over stdin. run is // given stdin already at EOF, exactly what systemd gave the old separate `notify` process: it exited at once, -// every five seconds (novox/hq issue 859). The watcher must still be running once run has returned. +// every five seconds (novox/hq issue 859). The watcher must still be running once run has returned — proved by +// making it actually answer after run returns, not by the absence of a signal that nothing but the test's own +// cleanup could ever raise. func TestRunStartsTheWatcherEvenWhenStdinIsAtEOF(t *testing.T) { started := make(chan struct{}) - finished := make(chan struct{}) - stop := make(chan struct{}) - defer close(stop) + ping := make(chan struct{}) + pong := make(chan struct{}) fakeWatcher := func(n *Notifier) { close(started) - <-stop // the real watch never returns either: it loops until the process itself ends - close(finished) + for range ping { + pong <- struct{}{} + } } n := &Notifier{Store: newFakeStore()} @@ -45,9 +51,64 @@ func TestRunStartsTheWatcherEvenWhenStdinIsAtEOF(t *testing.T) { t.Fatal("run did not return once stdin reached EOF, as systemd gives a process nobody talks to") } + // run has returned (stdio.Serve hit EOF at once); the watcher must still be live and answering, not merely + // not yet torn down. A watcher that stopped when run returned would never take this ping. select { - case <-finished: - t.Fatal("the watcher must not stop just because stdio.Serve returned: it is the base invocation's own watcher now, not a separate `notify` process that exits at EOF") - default: + case ping <- struct{}{}: + case <-time.After(time.Second): + t.Fatal("the watcher is no longer running once run returned: it must not stop just because stdio.Serve did") + } + select { + case <-pong: + case <-time.After(time.Second): + t.Fatal("the watcher did not answer once run returned") + } + close(ping) +} + +// TestWatchRetriesASubscribeRefusalWithBackoffAndNeverExits proves watch no longer exits the process on a +// subscribe the runtime refuses — the old os.Exit(1) took mail-notifier.status and mail_notifier_health down +// with it — but retries with backoff and logs each refusal, as modules/mesh-knowledge's hear does (novox/hq +// issue 859, review round 1). A subscribe that fails twice then succeeds must leave the process serving (run +// still returns normally over stdin at EOF, never os.Exit) and the watcher subscribed (its third, successful +// call). +func TestWatchRetriesASubscribeRefusalWithBackoffAndNeverExits(t *testing.T) { + var calls int32 + subscribed := make(chan struct{}) + fakeSubscribe := func(pattern string, handle func(stdio.Envelope) error) error { + if atomic.AddInt32(&calls, 1) <= 2 { + return errors.New("the runtime refused this subscribe") + } + close(subscribed) + return nil + } + noSleep := func(time.Duration) {} + + n := &Notifier{Store: newFakeStore()} + n.init() + + done := make(chan error, 1) + go func() { + done <- run(n, func(n *Notifier) { watchWith(n, fakeSubscribe, noSleep) }, + strings.NewReader(""), io.Discard, nil) + }() + + select { + case err := <-done: + if err != nil { + t.Fatalf("run over stdin at EOF: %v", err) + } + case <-time.After(time.Second): + t.Fatal("run did not return once stdin reached EOF: a subscribe refusal must not hold up or kill the process") + } + + select { + case <-subscribed: + case <-time.After(time.Second): + t.Fatal("the watcher never subscribed after its first two refusals") + } + + if got := atomic.LoadInt32(&calls); got != 3 { + t.Fatalf("subscribe was called %d time(s), want 3 (two refusals, then the one that succeeded)", got) } }