Let an on-call day be handed to somebody else

A date is held by exactly one person and POST /api/schedule plain-inserts,
so any date that was already taken came back 409. That made reassignment
impossible through the API: the only route was to delete the entry first,
and for a week that meant seven separate deletions. Worse, the reject is
all-or-nothing across the request, so assigning a week where a single day
happened to be taken failed entirely and placed none of the other six.

The refusal itself is worth keeping. Moving a shift off the person
expecting to be paged for it should not be something a plain call does by
accident, so the fix is to make it possible to ask for rather than to
remove the guard: "replace": true takes the dates anyway, and the flag
defaults to off so every existing caller behaves exactly as before.

The delete and the insert share the transaction that was already there.
That matters more than the flag does — a week of free and taken days now
lands as a unit, and a failure part way through leaves the rota as it was
instead of with a shift deleted and nothing put back. A rota with a hole
in it is worse than a rota that refused to change.

One consequence worth naming: under replace a date repeated inside one
request is idempotent rather than a conflict, because the second pass
clears what the first wrote.

README.md

@@ -190,6 +190,13 @@ A new incident is assigned to whoever holds today's schedule entry at the moment
 it opens (`GET /api/schedule/current`). If nobody is scheduled it opens
 unassigned. Reassign with `POST /api/incidents/{id}/assign`.
 
+One person holds a given day, so `POST /api/schedule` refuses a date somebody
+already has: taking a shift off the person expecting to be paged for it should
+not be something a plain call does by accident. Pass `"replace": true` to take
+them anyway. Either way the whole request is one transaction — a week where some
+days are free and some are taken moves as a unit, and a failure leaves the rota
+exactly as it was rather than with a hole in it.
+
 ### Push notifications
 
 With `TERDUT_NTFY_URL` set, an incident that opens is pushed to the on-call
@@ -211,10 +218,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 +242,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 +301,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 +366,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
 
@@ -446,7 +474,7 @@ unknown" rather than being rejected.
 
 | Method | Path | Description |
 |---|---|---|
-| `POST` | `/api/schedule` | Assign user to dates `{"user_id", "dates":["YYYY-MM-DD",...]}` — all-or-nothing |
+| `POST` | `/api/schedule` | Assign user to dates `{"user_id", "dates":["YYYY-MM-DD",...], "replace"}` — all-or-nothing |
 | `GET` | `/api/schedule` | List entries. Filters: `?from=YYYY-MM-DD`, `?to=YYYY-MM-DD` |
 | `GET` | `/api/schedule/current` | Today's on-call user (UTC), 404 if none |
 | `DELETE` | `/api/schedule/{id}` | Remove schedule entry |

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/api_test.go

@@ -310,6 +310,125 @@ func TestSchedule_MultiDateRollbackOnConflict(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// Schedule reassignment
+// ---------------------------------------------------------------------------
+
+// addUser creates a second person to hand a shift to. The bootstrap user is
+// admin, id 1.
+func addUser(t *testing.T, s *ts, username string) {
+	t.Helper()
+	resp := s.req(t, http.MethodPost, "/api/users",
+		map[string]any{"username": username, "email": username + "@test.com"})
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusCreated {
+		t.Fatalf("create user returned %d", resp.StatusCode)
+	}
+}
+
+// scheduleHolder reports who is on call for one date, or "" for nobody.
+func scheduleHolder(t *testing.T, s *ts, date string) string {
+	t.Helper()
+	var entries []map[string]any
+	decode(t, s.req(t, http.MethodGet, "/api/schedule?from="+date+"&to="+date, nil), &entries)
+	if len(entries) == 0 {
+		return ""
+	}
+	return entries[0]["username"].(string)
+}
+
+// Taking a day somebody else holds is possible, but only by asking for it.
+func TestSchedule_ReplaceTakesAnAssignedDate(t *testing.T) {
+	s := newTS(t)
+	addUser(t, s, "alex")
+
+	s.req(t, http.MethodPost, "/api/schedule",
+		map[string]any{"user_id": 1, "dates": []string{"2026-06-01"}}).Body.Close()
+
+	resp := s.req(t, http.MethodPost, "/api/schedule",
+		map[string]any{"user_id": 2, "dates": []string{"2026-06-01"}, "replace": true})
+	if resp.StatusCode != http.StatusCreated {
+		t.Fatalf("expected replace to succeed, got %d", resp.StatusCode)
+	}
+	resp.Body.Close()
+
+	if got := scheduleHolder(t, s, "2026-06-01"); got != "alex" {
+		t.Errorf("expected alex to hold the day, got %q", got)
+	}
+
+	// One row, not two: two entries for a date would mean two people believing
+	// they are on call for it.
+	var entries []map[string]any
+	decode(t, s.req(t, http.MethodGet, "/api/schedule?from=2026-06-01&to=2026-06-01", nil), &entries)
+	if len(entries) != 1 {
+		t.Errorf("expected exactly one entry for the date, got %d", len(entries))
+	}
+}
+
+// A week where only some days are taken is the case that was impossible before:
+// the free days and the taken ones have to land together.
+func TestSchedule_ReplaceMixedWeek(t *testing.T) {
+	s := newTS(t)
+	addUser(t, s, "alex")
+
+	s.req(t, http.MethodPost, "/api/schedule",
+		map[string]any{"user_id": 1, "dates": []string{"2026-06-02", "2026-06-04"}}).Body.Close()
+
+	week := []string{"2026-06-01", "2026-06-02", "2026-06-03", "2026-06-04", "2026-06-05"}
+	resp := s.req(t, http.MethodPost, "/api/schedule",
+		map[string]any{"user_id": 2, "dates": week, "replace": true})
+	if resp.StatusCode != http.StatusCreated {
+		t.Fatalf("expected the mixed week to succeed, got %d", resp.StatusCode)
+	}
+	resp.Body.Close()
+
+	for _, d := range week {
+		if got := scheduleHolder(t, s, d); got != "alex" {
+			t.Errorf("%s: expected alex, got %q", d, got)
+		}
+	}
+}
+
+// Without replace the guard stands: nobody loses a shift by accident.
+func TestSchedule_ReplaceDefaultsOff(t *testing.T) {
+	s := newTS(t)
+	addUser(t, s, "alex")
+
+	s.req(t, http.MethodPost, "/api/schedule",
+		map[string]any{"user_id": 1, "dates": []string{"2026-06-01"}}).Body.Close()
+
+	resp := s.req(t, http.MethodPost, "/api/schedule",
+		map[string]any{"user_id": 2, "dates": []string{"2026-06-01"}})
+	if resp.StatusCode != http.StatusConflict {
+		t.Fatalf("expected 409 without replace, got %d", resp.StatusCode)
+	}
+	resp.Body.Close()
+
+	if got := scheduleHolder(t, s, "2026-06-01"); got != "admin" {
+		t.Errorf("expected the original holder untouched, got %q", got)
+	}
+}
+
+// Replace makes a repeated date idempotent rather than a conflict: the second
+// pass clears what the first wrote and rewrites it. Worth pinning down, because
+// the same input without replace is a 409.
+func TestSchedule_ReplaceCollapsesRepeatedDates(t *testing.T) {
+	s := newTS(t)
+
+	resp := s.req(t, http.MethodPost, "/api/schedule",
+		map[string]any{"user_id": 1, "dates": []string{"2026-06-01", "2026-06-01"}, "replace": true})
+	if resp.StatusCode != http.StatusCreated {
+		t.Fatalf("expected a repeated date to be accepted under replace, got %d", resp.StatusCode)
+	}
+	resp.Body.Close()
+
+	var entries []map[string]any
+	decode(t, s.req(t, http.MethodGet, "/api/schedule?from=2026-06-01&to=2026-06-01", nil), &entries)
+	if len(entries) != 1 {
+		t.Errorf("expected one entry for the repeated date, got %d", len(entries))
+	}
+}
+
 // ---------------------------------------------------------------------------
 // Stats
 // ---------------------------------------------------------------------------

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))

internal/api/schedule.go

@@ -17,6 +17,12 @@ func handleCreateSchedule(db *sql.DB) http.HandlerFunc {
 		var req struct {
 			UserID int64    `json:"user_id"`
 			Dates  []string `json:"dates"`
+
+			// Replace takes dates that somebody else already holds. It defaults
+			// to off so that the plain call cannot quietly move a shift off the
+			// person expecting to be paged for it — reassigning has to be asked
+			// for.
+			Replace bool `json:"replace"`
 		}
 		if err := decodeJSON(r, &req); err != nil {
 			respond(w, http.StatusBadRequest, errResp("invalid request body"))
@@ -44,7 +50,10 @@ func handleCreateSchedule(db *sql.DB) http.HandlerFunc {
 			return
 		}
 
-		// All-or-nothing: if any date already has an assignment, reject the whole request.
+		// All-or-nothing, in both directions: without replace, one taken date
+		// rejects the whole request; with it, either every date moves or none
+		// does. The rota must never be left with a hole where a shift used to
+		// be, so the delete and the insert share one transaction.
 		tx, err := db.BeginTx(r.Context(), nil)
 		if err != nil {
 			respond(w, http.StatusInternalServerError, errResp("internal error"))
@@ -53,10 +62,18 @@ func handleCreateSchedule(db *sql.DB) http.HandlerFunc {
 		defer tx.Rollback()
 
 		for _, d := range req.Dates {
+			if req.Replace {
+				if _, err := tx.ExecContext(r.Context(),
+					"DELETE FROM schedule_entries WHERE date = ?", d); err != nil {
+					respond(w, http.StatusInternalServerError, errResp("internal error"))
+					return
+				}
+			}
 			if _, err := tx.ExecContext(r.Context(),
 				"INSERT INTO schedule_entries (user_id, date) VALUES (?, ?)", req.UserID, d); err != nil {
 				if strings.Contains(err.Error(), "UNIQUE constraint failed") {
-					respond(w, http.StatusConflict, errResp("date already assigned: "+d))
+					respond(w, http.StatusConflict,
+						errResp("date already assigned: "+d+" (pass replace to take it)"))
 					return
 				}
 				respond(w, http.StatusInternalServerError, errResp("internal error"))