Record notification delivery on the incident timeline

An incident's history went quiet after "Incident opened": nothing said
that anybody had been paged, reminded, or told it resolved. Delivery
lived only in the notifications outbox, which no API exposes, so when a
page failed to arrive there was nothing in the product that said whether
it had been sent.

The notifier now writes two event types. A notified event once ntfy
accepts the publish, carrying the kind in detail and the paged user in
user_id — absent when the page went to the shared fallback topic, which
belongs to nobody. And a notify_failed event when a notification
exhausts its retries, which is the one worth having: without it a page
that never landed leaves the timeline identical to one that did.

Both are written from the delivery result rather than at enqueue. A
queued notification is an intention, and the timeline is append-only, so
claiming somebody was told before ntfy accepted it would be a lie that
stays there. A failed timeline write is logged rather than returned, so
it cannot make a delivered row look unsent and send the page twice.

The topic is deliberately in neither: it is a shared secret with the
ntfy server, and every API key can read the timeline.

No migration — incident_events.type is free text, unlike
notifications.kind.

README.md

@@ -211,10 +211,17 @@ Three things get pushed:
 
 Notifications carry an **Acknowledge** button that acknowledges the incident
 without opening anything. It POSTs to `/api/notify/ack/{token}`, an
-unauthenticated route authorised by the 256-bit single-use token in its path —
-minted fresh per notification, scoped to one incident and one action, and valid
-for 24 hours. A real API key is never put in a notification, because the message
-is stored on the ntfy server and cached on the device.
+unauthenticated route authorised by the 256-bit token in its path — minted fresh
+per notification, scoped to one incident and one action, and valid for 24 hours.
+A real API key is never put in a notification, because the message is stored on
+the ntfy server and cached on the device.
+
+The token is **not** consumed by use. Acknowledging is idempotent, so a token
+stays valid for its full 24 hours and a second tap is a no-op that reports the
+incident's current state rather than an error — which is what you want when a
+tap is retried on a flaky mobile connection. What bounds it is scope, not a use
+count: one incident, one action, one day. Expired tokens are purged by the
+sweeper.
 
 Two consequences worth planning for:
 
@@ -228,6 +235,12 @@ Delivery is a queue, not an inline call: the webhook writes a row and a
 background notifier sends it within 30 seconds, retrying with exponential
 backoff up to 8 attempts. Nothing about ingestion blocks on ntfy being reachable.
 
+Every delivery is recorded on the incident's timeline: a `notified` event once
+ntfy accepts the publish, and a `notify_failed` event when a notification
+exhausts its retries. Written from the result rather than at enqueue, so the
+timeline says what actually happened — and a page that never landed is visible
+instead of looking the same as one that did.
+
 ### Stale alert expiry
 
 A resolved webhook is the only signal that an alert has stopped firing, so a
@@ -281,7 +294,7 @@ Authorization: Bearer <api-key>
 
 | Method | Path | Description |
 |---|---|---|
-| `POST` | `/api/notify/ack/{token}` | Acknowledge an incident from a push notification's Acknowledge button. No auth: the single-use token in the path is the credential. Must stay publicly reachable |
+| `POST` | `/api/notify/ack/{token}` | Acknowledge an incident from a push notification's Acknowledge button. No auth: the token in the path is the credential — one incident, one action, 24 hours, idempotent. Must stay publicly reachable |
 
 ### Incidents
 
@@ -346,8 +359,16 @@ name: degrade unknown values to "resolved, reason unknown".
 
 Types written today: `triggered`, `alert_added`, `alert_resolved`,
 `acknowledged`, `unacknowledged`, `assigned`, `snoozed`, `unsnoozed`, `resolved`,
-`note`. On an `assigned` event `user_id` is the **assignee**, not the actor. New
-types may be added; render unknown ones generically rather than dropping them.
+`note`, `notified`, `notify_failed`. On an `assigned` event `user_id` is the
+**assignee**, not the actor. New types may be added; render unknown ones
+generically rather than dropping them.
+
+On `notified` and `notify_failed`, `detail` carries the notification kind
+(`triggered` | `reminder` | `resolved`), and on a failure the reason after it.
+`user_id` is who was paged — absent means the page went to the shared fallback
+topic and so belongs to nobody. The topic itself is never written to the
+timeline: it is a shared secret with the ntfy server, and every API key can read
+this.
 
 ### Alerts
 

charts/terdut-server/values.yaml

@@ -42,7 +42,7 @@ notify:
   #
   # The Acknowledge button is a POST to /api/notify/ack/{token} from the
   # responder's phone, so that path has to stay publicly reachable — it is
-  # authorised by the single-use token in the URL, not by network placement.
+  # authorised by the scoped token in the URL, not by network placement.
   publicUrl: ""
   # Optional bearer token for an access-controlled ntfy, read from an existing
   # Secret. Leave name empty for an open ntfy.

internal/api/notifier.go

@@ -45,6 +45,19 @@ const (
 	notifyResolved  = "resolved"
 )
 
+// Timeline event types the notifier writes, so an incident's history says who
+// was paged and whether the page landed. Written from the delivery result
+// rather than at enqueue: a queued notification is an intention, and claiming
+// somebody was told before ntfy accepted it would be a lie the timeline keeps.
+//
+// The topic is deliberately absent from both. It is a shared secret with the
+// ntfy server — anyone holding it can publish to it — and the timeline is
+// readable by every API key.
+const (
+	eventNotified     = "notified"
+	eventNotifyFailed = "notify_failed"
+)
+
 // NotifyConfig is everything the notifier needs to reach ntfy and to build URLs
 // a phone can follow back to this server.
 type NotifyConfig struct {
@@ -208,6 +221,11 @@ func deliverPending(ctx context.Context, db *sql.DB, cfg NotifyConfig) {
 			time.Now().Unix(), n.id); err != nil {
 			log.Printf("notifier: mark sent %d: %v", n.id, err)
 		}
+		// Logged, not returned: the page has already gone out, and treating a
+		// failed timeline write as a failed delivery would send it again.
+		if err := logEvent(ctx, db, n.incidentID, eventNotified, n.userID, nil, &n.kind); err != nil {
+			log.Printf("notifier: log delivery of %d: %v", n.id, err)
+		}
 		sent++
 	}
 	if sent > 0 {
@@ -243,6 +261,11 @@ func pendingNotifications(ctx context.Context, db *sql.DB) ([]outboxRow, error)
 }
 
 // markFailed bumps the attempt count and pushes the row out to its next retry.
+//
+// The attempt that exhausts the budget also writes a timeline event. Without it
+// a page that never landed leaves the incident's history identical to one that
+// did, which is the failure most worth seeing: nobody was told, and nothing
+// says so.
 func markFailed(ctx context.Context, db *sql.DB, n outboxRow, cause error) {
 	next := time.Now().Add(retryDelay(n.attempts)).Unix()
 	if _, err := db.ExecContext(ctx,
@@ -250,6 +273,14 @@ func markFailed(ctx context.Context, db *sql.DB, n outboxRow, cause error) {
 		next, cause.Error(), n.id); err != nil {
 		log.Printf("notifier: mark failed %d: %v", n.id, err)
 	}
+
+	if n.attempts+1 < notifyMaxAttempts {
+		return
+	}
+	detail := fmt.Sprintf("%s: %s", n.kind, cause)
+	if err := logEvent(ctx, db, n.incidentID, eventNotifyFailed, n.userID, nil, &detail); err != nil {
+		log.Printf("notifier: log failure of %d: %v", n.id, err)
+	}
 }
 
 // retryDelay doubles the wait per attempt, up to notifyRetryMax.

internal/api/notify_test.go

@@ -199,6 +199,150 @@ func TestNotify_DeliveredOnlyOnce(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// Delivery on the timeline
+// ---------------------------------------------------------------------------
+
+// notifyEvents picks the notifier's entries out of an incident's timeline.
+// Asserted through the API rather than the table: the timeline is what the
+// clients read, so its shape is the contract worth covering.
+func notifyEvents(t *testing.T, s *ts, id int) []map[string]any {
+	t.Helper()
+	var out []map[string]any
+	for _, e := range timeline(t, s, id) {
+		if e["type"] == "notified" || e["type"] == "notify_failed" {
+			out = append(out, e)
+		}
+	}
+	return out
+}
+
+func TestNotify_DeliveryIsRecordedOnTheTimeline(t *testing.T) {
+	s, _ := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
+
+	fireCritical(t, s)
+	// Queued is not notified: nothing is on the timeline until ntfy accepts it.
+	if got := notifyEvents(t, s, 1); len(got) != 0 {
+		t.Fatalf("expected no event before delivery, got %v", got)
+	}
+
+	s.sweepNotify(t)
+
+	events := notifyEvents(t, s, 1)
+	if len(events) != 1 {
+		t.Fatalf("expected 1 notification event, got %v", events)
+	}
+	e := events[0]
+	if e["type"] != "notified" {
+		t.Errorf("expected a notified event, got %v", e["type"])
+	}
+	if e["detail"] != "triggered" {
+		t.Errorf("expected the kind in detail, got %v", e["detail"])
+	}
+	if e["username"] != "admin" {
+		t.Errorf("expected the paged user attached, got %v", e["username"])
+	}
+	// The topic is a shared secret with ntfy; the timeline is not the place for it.
+	for _, v := range e {
+		if s, ok := v.(string); ok && strings.Contains(s, "terdut-admin") {
+			t.Errorf("expected the topic kept out of the timeline, found it in %v", e)
+		}
+	}
+}
+
+// A redelivery-free pass must not double-log either.
+func TestNotify_TimelineRecordsOneEventPerDelivery(t *testing.T) {
+	s, _ := notifyTS(t, api.NotifyConfig{
+		PublicURL:   "https://terdut.example.com",
+		RepeatEvery: 15 * time.Minute,
+	})
+
+	fireCritical(t, s)
+	s.sweepNotify(t)
+	s.sweepNotify(t)
+	s.ageNotifications(t, 20*time.Minute)
+	s.sweepNotify(t)
+
+	events := notifyEvents(t, s, 1)
+	if len(events) != 2 {
+		t.Fatalf("expected one event per delivery, got %v", events)
+	}
+	if events[0]["detail"] != "triggered" || events[1]["detail"] != "reminder" {
+		t.Errorf("expected triggered then reminder, got %v and %v",
+			events[0]["detail"], events[1]["detail"])
+	}
+}
+
+func TestNotify_AllClearIsRecordedOnTheTimeline(t *testing.T) {
+	s, _ := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
+
+	fireCritical(t, s)
+	s.sweepNotify(t)
+
+	postWebhook(t, s, []map[string]any{
+		amAlert("fp-notify", "DiskFull", "resolved", "2026-05-20T10:00:00Z",
+			"2026-05-20T11:00:00Z", map[string]string{"severity": "critical"}),
+	}, "{}:{alertname=\"DiskFull\"}")
+	s.sweepNotify(t)
+
+	events := notifyEvents(t, s, 1)
+	if len(events) != 2 {
+		t.Fatalf("expected the all-clear recorded, got %v", events)
+	}
+	if events[1]["detail"] != "resolved" {
+		t.Errorf("expected a resolved event, got %v", events[1]["detail"])
+	}
+}
+
+// A page to the shared fallback belongs to nobody, and the timeline has to say
+// so rather than attributing it to whoever happens to be on call now.
+func TestNotify_FallbackDeliveryHasNoUser(t *testing.T) {
+	f := newFakeNtfy(t)
+	s := newTS(t, api.NotifyConfig{
+		BaseURL:       f.URL,
+		FallbackTopic: "terdut-oncall",
+		PublicURL:     "https://terdut.example.com",
+	})
+
+	fireCritical(t, s)
+	s.sweepNotify(t)
+
+	events := notifyEvents(t, s, 1)
+	if len(events) != 1 {
+		t.Fatalf("expected 1 notification event, got %v", events)
+	}
+	if got, ok := events[0]["username"]; ok && got != nil && got != "" {
+		t.Errorf("expected no user on a fallback-topic page, got %v", got)
+	}
+}
+
+// The failure worth seeing: nobody was paged, and the timeline says so instead
+// of looking exactly like a delivery that worked.
+func TestNotify_ExhaustedRetriesAreRecordedOnce(t *testing.T) {
+	s, f := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
+	f.failWith(http.StatusInternalServerError)
+
+	fireCritical(t, s)
+	// One pass per attempt, each made due by clearing the backoff the last one set.
+	for i := 0; i < 10; i++ {
+		s.sweepNotify(t)
+		s.exec(t, "UPDATE notifications SET send_after = ? WHERE sent_at IS NULL",
+			time.Now().Add(-time.Second).Unix())
+	}
+
+	events := notifyEvents(t, s, 1)
+	if len(events) != 1 {
+		t.Fatalf("expected exactly one failure event, got %v", events)
+	}
+	if events[0]["type"] != "notify_failed" {
+		t.Errorf("expected notify_failed, got %v", events[0]["type"])
+	}
+	detail, _ := events[0]["detail"].(string)
+	if !strings.HasPrefix(detail, "triggered: ") || !strings.Contains(detail, "500") {
+		t.Errorf("expected the kind and the reason in %q", detail)
+	}
+}
+
 // ---------------------------------------------------------------------------
 // Acknowledging from the notification
 // ---------------------------------------------------------------------------

internal/api/router.go

@@ -22,7 +22,7 @@ func NewRouter(db *sql.DB, notify NotifyConfig) http.Handler {
 
 	// Unauthenticated: bootstrap, the Alertmanager webhook receiver, and the
 	// Acknowledge button in a push notification. The last one is authorised by
-	// the single-use token in its path rather than an API key, and has to stay
+	// the scoped token in its path rather than an API key, and has to stay
 	// reachable from outside the cluster for the button to work.
 	r.Post("/api/bootstrap", handleBootstrap(db))
 	r.Post("/api/alertmanager/webhook", handleAlertmanagerWebhook(db, notify))