Merge the delivery boundary fixes

internal/delivery/dispatcher.go

@@ -137,9 +137,10 @@ func NewDispatcher(cfg Config) *Dispatcher {
 // picks for (severity, presence), sends via the matching sink, and records
 // one nudge row per successful send. returns the dispatches (one per channel).
 //
-// a Drop channel = no send, no record (the nudge was suppressed by routing,
+// a Drop channel = no send (the nudge was suppressed by routing, not by a
-// not by a failure — "a missed water nudge is noise"). a nil sink = channel
+// failure — "a missed water nudge is noise"), but it does leave a 'dropped'
-// not wired, skip silently. a send error stops the dispatch and returns what
+// outbox row so the suppression is visible. a nil sink = channel not wired,
+// skip silently. a send error stops the dispatch and returns what
 // got through — the daemon decides whether to retry.
 func (d *Dispatcher) DispatchNudge(ctx context.Context, pn PhrasedNudge, now time.Time) ([]Dispatch, error) {
 	c := pn.Candidate
@@ -148,6 +149,16 @@ func (d *Dispatcher) DispatchNudge(ctx context.Context, pn PhrasedNudge, now tim
 	for i := 0; i < len(channels); i++ {
 		ch := channels[i]
 		if ch == ChannelDrop {
+			// the routing table suppressed this nudge on purpose (a care nudge
+			// while you're away is noise). that stays — but it must not be
+			// invisible, or "she dropped it" and "the rule never fired" look
+			// the same afterwards. no nudges row: that table feeds the
+			// ignored_rate signal, and a nudge nobody could see must not
+			// count as ignored.
+			id := d.beginOutbox(ctx, "nudge", c.Rule.Name, 0, ch, pn.Summary, now)
+			d.completeOutbox(ctx, id, store.DeliveryDropped, now)
+			log.Printf("dispatcher: dropped %s (sev%d, presence=%s) — routing table suppressed it",
+				c.Rule.Name, c.Severity, c.State.Presence)
 			continue
 		}
 		s := Sendable{
@@ -396,6 +407,13 @@ func messageForChannel(s Sendable) string {
 	if !isAway(s.Channel) {
 		return s.Body
 	}
+	return AwayMessage(s)
+}
+
+// AwayMessage — the only text an off-box channel may ever carry. Exported so
+// the away sinks share this one rule instead of each inventing a fallback: the
+// summary if we have one, otherwise a fixed generic line. Never the body.
+func AwayMessage(s Sendable) string {
 	if s.Summary != "" {
 		return s.Summary
 	}

internal/delivery/durability_test.go

@@ -37,10 +37,12 @@ func TestVoiceNoSessionFallthroughLeavesOutboxTrail(t *testing.T) {
 			[]string{"voice", "ntfy"}, []string{store.DeliveryFailed, store.DeliverySent}},
 		{"sev4 falls through to telegram", loop.Sev4,
 			[]string{"voice", "telegram"}, []string{store.DeliveryFailed, store.DeliverySent}},
-		{"sev1 does not fall through", loop.Sev1,
+		// care severities still don't reach an away channel; since #370 the
-			[]string{"voice"}, []string{store.DeliveryFailed}},
+		// drop itself is a visible row instead of nothing.
-		{"sev2 does not fall through", loop.Sev2,
+		{"sev1 drops instead of falling through", loop.Sev1,
-			[]string{"voice"}, []string{store.DeliveryFailed}},
+			[]string{"voice", "drop"}, []string{store.DeliveryFailed, store.DeliveryDropped}},
+		{"sev2 drops instead of falling through", loop.Sev2,
+			[]string{"voice", "drop"}, []string{store.DeliveryFailed, store.DeliveryDropped}},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {

internal/delivery/ntfysink/ntfysink.go

@@ -2,10 +2,10 @@
 //
 // ntfy is the away-channel for sev3 (ops soft) nudges, sev4 (ops hard)
 // nudges when present (alongside voice), and reminders when away. the
-// message body is the Sendable's Summary — the minimal-body rule from the
+// message body is delivery.AwayMessage — the minimal-body rule from the
 // spec ("disk low on homesrv," not detail; no shoulder-surf exfil through
-// the relay). voice gets Body; away channels get Summary, enforced at the
+// the relay). the dispatcher already strips detail off away sendables; the
-// sink so a phraser bug can't exfil.
+// sink uses the same helper so it can't leak the body on its own either.
 //
 // ntfy runs locally (docker, 127.0.0.1:8085, deny-all auth). maven publishes
 // with a dedicated user (write-only to maven-* topics) — the credential is a
@@ -69,18 +69,14 @@ func New(cfg Config) (*Sink, error) {
 	}, nil
 }
 
-// Send publishes one notification to ntfy. the body is the Sendable's Summary
+// Send publishes one notification to ntfy. the body is the minimal away
-// (minimal body); Title is "maven" (consistent sender identity on the lock
+// message (never the full body); Title is "maven" (consistent sender identity
-// screen — the content is in the body). Priority maps from severity/kind so
+// on the lock screen — the content is in the body). Priority maps from severity/kind so
 // the phone client can ring differently for an alarm vs a soft ops nudge.
 func (s *Sink) Send(ctx context.Context, d delivery.Sendable) error {
-	body := d.Summary
+	// never fall back to d.Body: ntfy leaves the box, so an empty summary gets
-	if body == "" {
+	// a generic line instead of the full detail.
-		body = d.Body // terse full message beats no message
+	body := delivery.AwayMessage(d)
-	}
-	if body == "" {
-		return fmt.Errorf("ntfysink: empty message for %s", d.Channel)
-	}
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodPost, s.topicURL(), strings.NewReader(body))
 	if err != nil {

internal/delivery/ntfysink/ntfysink_test.go

@@ -147,9 +147,9 @@ func TestSendBodyIsSummaryNotFullBody(t *testing.T) {
 	}
 }
 
-func TestSendFallsBackToBodyWhenSummaryEmpty(t *testing.T) {
+func TestSendNeverSendsTheBodyWhenSummaryEmpty(t *testing.T) {
-	// a terse full message is better than no message; the phraser should
+	// #368: this used to fall back to the full body. ntfy leaves the box, so
-	// produce a summary for away-bound severities, but don't silently drop.
+	// an empty summary gets a fixed generic line plus the rule name instead.
 	rs := newRecordingServer(t, 200, "")
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()
@@ -160,12 +160,15 @@ func TestSendFallsBackToBodyWhenSummaryEmpty(t *testing.T) {
 		t.Fatalf("Send: %v", err)
 	}
 	_, _, body, _, _, _ := rs.snapshot()
-	if body != s.Body {
+	want := delivery.GenericAwayMessage + ": service_down"
-		t.Fatalf("fallback body: want %q, got %q", s.Body, body)
+	if body != want {
+		t.Fatalf("body: want %q, got %q", want, body)
 	}
 }
 
-func TestSendRejectsEmptyMessage(t *testing.T) {
+func TestSendNeverSendsAnEmptyMessage(t *testing.T) {
+	// with nothing at all to say we still send the generic line — an away
+	// channel can never carry detail, but it also never goes out blank.
 	rs := newRecordingServer(t, 200, "")
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()
@@ -173,9 +176,13 @@ func TestSendRejectsEmptyMessage(t *testing.T) {
 	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
 	s := nudgeSendable(loop.Sev3, "")
 	s.Body = ""
-	err := sink.Send(context.Background(), s)
+	s.RuleName = ""
-	if err == nil {
+	if err := sink.Send(context.Background(), s); err != nil {
-		t.Fatal("want error for empty message")
+		t.Fatalf("Send: %v", err)
+	}
+	_, _, body, _, _, _ := rs.snapshot()
+	if body != delivery.GenericAwayMessage {
+		t.Fatalf("body: want %q, got %q", delivery.GenericAwayMessage, body)
 	}
 }
 

internal/delivery/routing_table_test.go

@@ -208,10 +208,9 @@ func TestAwayChannelsGetMinimalBody(t *testing.T) {
 // TestCareAwayDropIsRecorded — DESIGN.md's drop is a decision ("a missed water
 // nudge is noise, a missed backup failure isn't"), so it should be visible
 // rather than vanish. Today drop is a bare `continue`: no nudge row, no outbox
-// attempt, no log — nothing an operator can see afterwards.
+// attempt, no log — nothing an operator can see afterwards. now it leaves a
+// 'dropped' outbox row.
 func TestCareAwayDropIsRecorded(t *testing.T) {
-	t.Skip("not implemented: dispatcher.go:149-151 skips a Drop channel with no record; there is no 'dropped' outcome in store/delivery.go:16-21")
-
 	ob := &fakeOutbox{}
 	d := NewDispatcher(Config{Voice: &fakeSink{}, Nudges: &fakeNudgeRecorder{}, Outbox: ob})
 

internal/delivery/telegramsink/telegramsink.go

@@ -2,11 +2,12 @@
 //
 // telegram is the away-channel for sev4 (ops hard) nudges — "disk-fire alarm
 // at 2am routes to telegram, repeat til ack." the message body is the
-// Sendable's Summary — the minimal-body rule from the spec ("disk low on
+// delivery.AwayMessage — the minimal-body rule from the spec ("disk low on
-// homesrv," not detail; no shoulder-surf exfil through the relay). voice gets
+// homesrv," not detail; no shoulder-surf exfil through the relay). the
-// Body; away channels get Summary, enforced at the sink so a phraser bug can't
+// dispatcher already strips detail off away sendables; the sink uses the same
-// exfil. additionally, protect_content=true is passed on every send so the
+// helper so it can't leak the body on its own either. additionally,
-// message can't be forwarded out of the chat — locks the minimal body further.
+// protect_content=true is passed on every send so the message can't be
+// forwarded out of the chat — locks the minimal body further.
 //
 // telegram's bot API is region-restricted for this homesrv — direct egress to
 // api.telegram.org is unreliable. the spec's "away channels leave the box —
@@ -140,18 +141,13 @@ type telegramResp struct {
 }
 
 // Send publishes one message to the configured telegram chat. the body is the
-// Sendable's Summary (minimal body); empty Summary falls back to Body (terse
+// minimal away message (never the full body). protect_content=true so even
-// full message beats no message). protect_content=true so a phraser bug (Body
+// that can't be forwarded onward by the user or a chat observer — locks the
-// leaking detail through Summary) can't be forwarded onward by the user or a
+// minimal-body rule at the channel's own last mile.
-// chat observer — locks the minimal-body rule at the channel's own last mile.
 func (s *Sink) Send(ctx context.Context, d delivery.Sendable) error {
-	body := d.Summary
+	// never fall back to d.Body: telegram leaves the box, so an empty summary
-	if body == "" {
+	// gets a generic line instead of the full detail.
-		body = d.Body
+	body := delivery.AwayMessage(d)
-	}
-	if body == "" {
-		return fmt.Errorf("telegramsink: empty message for %s", d.Channel)
-	}
 
 	payload := sendMessageReq{
 		ChatID:              s.cfg.ChatID,

internal/delivery/telegramsink/telegramsink_test.go

@@ -173,9 +173,9 @@ func TestSendBodyIsSummaryNotFullBody(t *testing.T) {
 	}
 }
 
-func TestSendFallsBackToBodyWhenSummaryEmpty(t *testing.T) {
+func TestSendNeverSendsTheBodyWhenSummaryEmpty(t *testing.T) {
-	// terse full message beats none; the phraser should produce a summary for
+	// #368: this used to fall back to the full body. telegram leaves the box,
-	// away-bound severities, but don't silently drop.
+	// so an empty summary gets a fixed generic line plus the rule name.
 	rs := newRecordingServer(t, 200, "")
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()
@@ -188,12 +188,14 @@ func TestSendFallsBackToBodyWhenSummaryEmpty(t *testing.T) {
 	_, _, body, _, _ := rs.snapshot()
 	var req sendMessageReq
 	_ = json.Unmarshal([]byte(body), &req)
-	if req.Text != s.Body {
+	want := delivery.GenericAwayMessage + ": service_down"
-		t.Fatalf("fallback text: want %q, got %q", s.Body, req.Text)
+	if req.Text != want {
+		t.Fatalf("text: want %q, got %q", want, req.Text)
 	}
 }
 
-func TestSendRejectsEmptyMessage(t *testing.T) {
+func TestSendNeverSendsAnEmptyMessage(t *testing.T) {
+	// with nothing at all to say we still send the generic line.
 	rs := newRecordingServer(t, 200, "")
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()
@@ -201,9 +203,15 @@ func TestSendRejectsEmptyMessage(t *testing.T) {
 	sink, _ := New(sinkCfg(srv.URL))
 	s := nudgeSendable(loop.Sev4, "")
 	s.Body = ""
-	err := sink.Send(context.Background(), s)
+	s.RuleName = ""
-	if err == nil {
+	if err := sink.Send(context.Background(), s); err != nil {
-		t.Fatal("want error for empty message")
+		t.Fatalf("Send: %v", err)
+	}
+	_, _, body, _, _ := rs.snapshot()
+	var req sendMessageReq
+	_ = json.Unmarshal([]byte(body), &req)
+	if req.Text != delivery.GenericAwayMessage {
+		t.Fatalf("text: want %q, got %q", delivery.GenericAwayMessage, req.Text)
 	}
 }
 

internal/store/delivery.go

@@ -13,11 +13,15 @@ import (
 // unknown = a pending row found stale at startup: the process that started it
 // is gone, and the send may or may not have reached the external channel.
 // Never auto-resolved into sent or failed — that would be guessing.
+// dropped = the routing table deliberately suppressed this one (a care nudge
+// while you're away). Nothing was sent and nothing went wrong; the row exists
+// so "she dropped it" and "the rule never fired" don't look the same later.
 const (
 	DeliveryPending = "pending"
 	DeliverySent    = "sent"
 	DeliveryFailed  = "failed"
 	DeliveryUnknown = "unknown"
+	DeliveryDropped = "dropped"
 )
 
 // BeginDeliveryAttempt durably records intent to send BEFORE the external
@@ -43,10 +47,11 @@ func (s *Store) BeginDeliveryAttempt(ctx context.Context, kind, rule string, rem
 }
 
 // CompleteDeliveryAttempt records the sink's outcome for a prior
-// BeginDeliveryAttempt. status is "sent" or "failed" — never "pending" or
+// BeginDeliveryAttempt. status is "sent", "failed" or "dropped" — never
-// "unknown" (those are set only by Begin and reconciliation respectively).
+// "pending" or "unknown" (those are set only by Begin and reconciliation
+// respectively).
 func (s *Store) CompleteDeliveryAttempt(ctx context.Context, id int64, status string, now time.Time) error {
-	if status != DeliverySent && status != DeliveryFailed {
+	if status != DeliverySent && status != DeliveryFailed && status != DeliveryDropped {
 		return fmt.Errorf("store: invalid delivery completion status %q", status)
 	}
 	_, err := s.db.ExecContext(ctx,

internal/store/delivery_test.go

@@ -0,0 +1,34 @@
+package store
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+// TestDroppedDeliveryAttemptRoundTrips — Vikunja #370. A suppressed nudge is
+// recorded as 'dropped'. The status column has a CHECK constraint, so this
+// only works if migration #12 widened it; a fake outbox in a unit test would
+// not catch that.
+func TestDroppedDeliveryAttemptRoundTrips(t *testing.T) {
+	s := newTestStore(t)
+	ctx := context.Background()
+	now := time.Now()
+
+	id, err := s.BeginDeliveryAttempt(ctx, "nudge", "water", 0, "drop", "abc123", now)
+	if err != nil {
+		t.Fatalf("BeginDeliveryAttempt: %v", err)
+	}
+	if err := s.CompleteDeliveryAttempt(ctx, id, DeliveryDropped, now); err != nil {
+		t.Fatalf("CompleteDeliveryAttempt: %v", err)
+	}
+
+	var status string
+	err = s.db.QueryRowContext(ctx, `SELECT status FROM delivery_attempts WHERE id = ?`, id).Scan(&status)
+	if err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	if status != DeliveryDropped {
+		t.Fatalf("status: want %q, got %q", DeliveryDropped, status)
+	}
+}

internal/store/migrations.go

@@ -88,6 +88,30 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2
 		key   TEXT PRIMARY KEY,
 		value TEXT NOT NULL
 	);`, // #11 — small key/value table for facts about the DB itself; first key is embedder_id (Vikunja #378)
+
+	// #12 — a suppressed nudge gets a 'dropped' row (Vikunja #370). sqlite
+	// can't widen a CHECK constraint in place, so the table is rebuilt; the
+	// index goes with the old table and is recreated. The columns are listed
+	// out rather than `SELECT *` — copying by position would silently shuffle
+	// every row if the old table's column order ever differed from this one.
+	`CREATE TABLE delivery_attempts_v12 (
+		id           INTEGER PRIMARY KEY AUTOINCREMENT,
+		kind         TEXT NOT NULL CHECK (kind IN ('nudge','reminder')),
+		rule         TEXT NOT NULL DEFAULT '',
+		reminder_id  INTEGER NOT NULL DEFAULT 0,
+		channel      TEXT NOT NULL,
+		body_hash    TEXT NOT NULL,
+		status       TEXT NOT NULL DEFAULT 'pending' CHECK (status IN ('pending','sent','failed','unknown','dropped')),
+		created_ts   INTEGER NOT NULL,
+		completed_ts INTEGER
+	);
+	INSERT INTO delivery_attempts_v12
+		(id, kind, rule, reminder_id, channel, body_hash, status, created_ts, completed_ts)
+		SELECT id, kind, rule, reminder_id, channel, body_hash, status, created_ts, completed_ts
+		FROM delivery_attempts;
+	DROP TABLE delivery_attempts;
+	ALTER TABLE delivery_attempts_v12 RENAME TO delivery_attempts;
+	CREATE INDEX IF NOT EXISTS idx_delivery_attempts_status ON delivery_attempts (status);`,
 }
 
 // migrate applies every migration with a number greater than the DB's current