I-005: notification retry loop + dead-letter queue

Critical alerts can no longer be silently dropped by a transient
notifier failure. Failed notification attempts now ride an exponential
backoff retry loop, with a 5-attempt budget before promotion to the
dead-letter queue for operator intervention.

Schema (migration 000016, idempotent):
- retry_count INTEGER NOT NULL DEFAULT 0
- next_retry_at TIMESTAMPTZ
- last_error TEXT
- idx_notification_events_retry_sweep partial index
  (next_retry_at) WHERE status='failed' AND next_retry_at IS NOT NULL
  Dead rows clear next_retry_at so the index stops matching them.

Service contract:
- NotificationService.RetryFailedNotifications drives 2^n-minute
  exponential backoff capped at 1h (notifRetryBackoffCap) with
  5-attempt budget (notifRetryMaxAttempts).
- Exhaustion (RetryCount >= notifRetryMaxAttempts-1) promotes to
  status='dead' via MarkAsDead.
- Non-terminal failures record via RecordFailedAttempt.
- Success path promotes to 'sent' without touching retry_count
  (audit preserves "delivered on attempt N").
- Missing-notifier branch defensively promotes to 'sent' to avoid
  wedging a row on a deleted channel.
- RequeueNotification operator escape hatch atomically resets
  retry_count -> 0, next_retry_at -> NULL, last_error -> NULL,
  status -> pending via notifRepo.Requeue.

Scheduler:
- New always-on notificationRetryLoop wired into the base loop set at
  CERTCTL_NOTIFICATION_RETRY_INTERVAL (default 2m).
- sync/atomic.Bool idempotency guard.
- sync.WaitGroup shutdown drain via WaitForCompletion.

StatsService:
- SetNotifRepo setter pattern preserves 9 pre-existing
  NewStatsService call sites (main.go + stats_test.go + 8 digest
  tests) without touching the constructor signature.
- DashboardSummary.NotificationsDead populated via
  notifRepo.CountByStatus(ctx, "dead") — nil-safe when unwired
  (reports zero on systems without a notification repository).
- CountByStatus error is non-fatal (dashboard summary is
  best-effort for this field).
- Prometheus certctl_notification_dead_total counter emitted from
  the same snapshot.

Handler:
- New POST /api/v1/notifications/{id}/requeue endpoint.
- dead status surfaces to MCP + CLI.

Frontend:
- NotificationsPage gains two-tab toolbar ("All" / "Dead letter")
  with queryKey: ['notifications', activeTab] so switching tabs
  doesn't serve stale data until the 30s refetch.
- Dead rows surface "Retry {n}/5" + truncated last_error with
  full-text title tooltip.
- Requeue mutation wrapped as
    mutationFn: (id: string) => requeueNotification(id)
  to prevent react-query v5's positional context argument from
  leaking into the API client — pinned against future refactors
  by strict-match toHaveBeenCalledWith('notif-dead-001') in
  NotificationsPage.test.tsx:181.

Closes I-005.
This commit is contained in:
Shankar
2026-04-19 15:17:27 +00:00
parent 6836286c37
commit 15daf008aa
33 changed files with 3758 additions and 228 deletions
+242
View File
@@ -10,6 +10,40 @@ import (
"github.com/shankar0123/certctl/internal/repository"
)
// I-005 retry + DLQ knobs. These pin the operator-approved retry budget and
// the defense-in-depth ceiling on the exponential backoff curve used by
// RetryFailedNotifications.
//
// Values match those the Phase 1 Red tests assert against (see
// i005MaxAttempts / i005BackoffCap in notification_test.go:600-608) — the
// production identifiers are distinct because this file and its tests share
// `package service`, so a single shared name would collide at compile time.
// The test comment explicitly notes "Phase 2 is free to thread this from
// config"; when that wiring lands, these become package-level defaults the
// scheduler can override. For now they are the single source of truth.
const (
// notifRetryMaxAttempts is the attempt budget *before* the current
// attempt: a row at retry_count == notifRetryMaxAttempts-1 that fails
// this tick transitions to 'dead' instead of being re-armed. The
// repository's ListRetryEligible filter also uses this value as a
// guard (`AND retry_count < $2`) so a DLQ row is never re-swept.
notifRetryMaxAttempts = 5
// notifRetryBackoffCap is the 1h ceiling on `2^retry_count` minutes.
// With max_attempts=5 the deepest actually-schedulable wait is 2^3=8m
// (retry_count=3 → 8m, then retry_count=4 → 'dead'), so the cap is a
// ceiling-assertion today — but it must stay in place so a later
// increase in max_attempts cannot push next_retry_at past 1h without
// an explicit policy decision.
notifRetryBackoffCap = time.Hour
// notifRetrySweepLimit caps a single retry tick at this many rows so
// a large burst of dead-letter-bound mail cannot monopolize the 2m
// tick budget. Mirrors the 1000-row cap on ProcessPendingNotifications
// at notification.go:244 for operational symmetry.
notifRetrySweepLimit = 1000
)
// NotificationService provides business logic for managing notifications.
type NotificationService struct {
notifRepo repository.NotificationRepository
@@ -373,3 +407,211 @@ func (s *NotificationService) GetNotification(ctx context.Context, id string) (*
func (s *NotificationService) MarkAsRead(ctx context.Context, id string) error {
return s.notifRepo.UpdateStatus(ctx, id, "read", time.Now())
}
// ─── I-005 retry + DLQ surface (Phase 2 Green) ───────────────────────────
//
// The three methods below close the retry loop the Phase 1 Red tests pin at
// notification_test.go:600-917 and notification_handler_test.go:443-519:
//
// 1. RetryFailedNotifications — scheduler entry point. Pulls failed rows
// whose next_retry_at has elapsed, retries delivery, rewrites retry
// bookkeeping per the pre-increment backoff contract, and transitions
// exhausted rows to 'dead' (DLQ). Per-row errors never bubble — a
// single bad recipient cannot stall the tick. Mirrors the ordering
// the ProcessPendingNotifications loop uses at notification.go:242.
//
// 2. RequeueNotification — operator-driven escape hatch from 'dead' back
// to 'pending'. Pass-through to the repo's Requeue method with clean
// error wrapping so repo-layer failures ("pg: deadlock detected")
// surface in the UI instead of silently succeeding.
//
// 3. ListNotificationsByStatus — Dead letter tab support. Thin filter
// wrapper around the existing List query; the Phase 2 Green handler
// routes `?status=…` through this method while preserving the
// unfiltered path through ListNotifications (handler_test pins both).
//
// Sibling scheduler loops I-001 (job retry) and I-003 (job timeout) already
// ship the 10-loop topology these methods plug into; the 11th loop added
// by this milestone calls RetryFailedNotifications on a 2m tick, matching
// the CERTCTL_NOTIFICATION_RETRY_INTERVAL default pinned in config/
// scheduler Phase 2 Green edits that follow this one.
// RetryFailedNotifications is the scheduler entry point for the I-005
// retry sweep. Semantics (pinned by notification_test.go:635-843):
//
// - A ListRetryEligible failure short-circuits with a wrapped error so
// the caller's tick counter reflects the outage. Crucially, zero
// notifier.Send calls fire in this path — we never got a canonical
// set of rows, and issuing any sends risks double-delivery when the
// DB comes back.
//
// - Per-row failures are logged but NEVER returned. That contract comes
// straight from ProcessPendingNotifications (notification.go:242-267);
// the retry loop inherits it so a single 4xx response can't freeze
// every downstream row in the sweep.
//
// - Success promotes the row directly to 'sent' via UpdateStatus. The
// retry_count field is *not* incremented on success — that would
// falsify the audit-trail signal "this row was delivered on attempt
// N". The mock's UpdateStatus does a plain status write with no retry
// mutation (testutil_test.go:446-459), matching the postgres impl.
//
// - Failure uses pre-increment exponential backoff:
// wait = min(2^retry_count * time.Minute, notifRetryBackoffCap)
// where retry_count is the row's value *before* this attempt. The
// repo layer's RecordFailedAttempt then increments retry_count by 1
// server-side. This asymmetry keeps the service stateless — the
// service reads retry_count to compute the wait, but never writes it
// directly; the write is exclusively the repo's responsibility.
//
// - Exhaustion transitions to 'dead' when retry_count == max-1, because
// RecordFailedAttempt's ++ would push retry_count to max and the next
// sweep's `retry_count < max` filter in ListRetryEligible would then
// silently skip the row forever (a zombie-failed row nobody sees).
// MarkAsDead clears next_retry_at to evict the row from the partial
// retry-sweep index as well, so it stops scanning past dead rows.
//
// - A row whose Channel has no registered notifier is promoted to
// 'sent' (demo-mode parity with sendNotification's fallback at
// notification.go:272-279). This branch should not normally fire for
// retry rows — they were created *by* a notifier that failed — but
// defensive handling guards against config drift (notifier disabled
// between Create and retry) that would otherwise wedge the row.
func (s *NotificationService) RetryFailedNotifications(ctx context.Context) error {
now := time.Now()
rows, err := s.notifRepo.ListRetryEligible(ctx, now, notifRetryMaxAttempts, notifRetrySweepLimit)
if err != nil {
return fmt.Errorf("failed to list retry-eligible notifications: %w", err)
}
for _, row := range rows {
if row == nil {
continue
}
notifier, ok := s.notifierRegistry[string(row.Channel)]
if !ok {
// No notifier wired for this channel — promote to 'sent' to
// avoid looping forever over a row that has nowhere to go.
// See notification.go:272-279 for the sibling demo-mode path.
if updateErr := s.notifRepo.UpdateStatus(ctx, row.ID, string(domain.NotificationStatusSent), time.Now()); updateErr != nil {
slog.Error("failed to promote retry row with missing notifier to sent",
"notification_id", row.ID, "channel", row.Channel, "error", updateErr)
}
continue
}
sendErr := notifier.Send(ctx, row.Recipient, string(row.Type), row.Message)
if sendErr == nil {
// Success: promote straight to 'sent' without touching
// retry_count — the audit trail must preserve "this row was
// delivered on attempt N", and the mock's UpdateStatus is a
// plain status write (no retry_count reset). Errors here are
// logged, never returned.
if updateErr := s.notifRepo.UpdateStatus(ctx, row.ID, string(domain.NotificationStatusSent), time.Now()); updateErr != nil {
slog.Error("failed to mark retried notification as sent",
"notification_id", row.ID, "error", updateErr)
}
continue
}
// Failure path. Compute pre-increment backoff first so the
// exhaustion branch and the reschedule branch see an identical
// `wait` derivation — easier to audit against the test window
// assertions at notification_test.go:739-743 and :796-801.
wait := time.Duration(1<<row.RetryCount) * time.Minute
if wait > notifRetryBackoffCap {
wait = notifRetryBackoffCap
}
// Exhaustion: this attempt consumes the final slot of the attempt
// budget. Transition to 'dead' and let MarkAsDead clear
// next_retry_at so the retry-sweep index stops hitting the row.
if row.RetryCount >= notifRetryMaxAttempts-1 {
if markErr := s.notifRepo.MarkAsDead(ctx, row.ID, sendErr.Error()); markErr != nil {
slog.Error("failed to mark exhausted notification as dead",
"notification_id", row.ID, "retry_count", row.RetryCount,
"send_error", sendErr, "mark_error", markErr)
}
continue
}
// Non-terminal: hand the lastError + nextRetryAt off to the repo,
// which increments retry_count by exactly 1 and keeps the row in
// 'failed' state so the next tick picks it up.
nextRetryAt := time.Now().Add(wait)
if recErr := s.notifRepo.RecordFailedAttempt(ctx, row.ID, sendErr.Error(), nextRetryAt); recErr != nil {
slog.Error("failed to record notification retry attempt",
"notification_id", row.ID, "retry_count", row.RetryCount,
"next_retry_at", nextRetryAt, "send_error", sendErr, "record_error", recErr)
}
}
return nil
}
// RequeueNotification is the operator-driven escape hatch from 'dead' back
// to 'pending'. It resets all retry bookkeeping — retry_count → 0,
// next_retry_at → NULL, last_error → NULL — so ProcessPendingNotifications
// treats the requeued row as a fresh attempt on its next tick. Identical on
// the wire to a newly-created notification.
//
// Behavior contract (pinned by notification_test.go:849-917):
//
// - Success path delegates to the repo's Requeue, which performs the
// status/retry_count/next_retry_at/last_error reset atomically. The
// service adds no extra bookkeeping; the audit trail already captures
// the transition via the upstream API call.
//
// - Error path wraps the repo error with context so a failure like
// "pg: deadlock detected" surfaces in the handler response and the
// operator UI. The service has no fallback — a silent "success" that
// didn't actually mutate the row would be worse than a loud error.
func (s *NotificationService) RequeueNotification(ctx context.Context, id string) error {
if err := s.notifRepo.Requeue(ctx, id); err != nil {
return fmt.Errorf("failed to requeue notification: %w", err)
}
return nil
}
// ListNotificationsByStatus returns paginated notifications filtered by
// status. It mirrors ListNotifications's shape but threads a Status filter
// into the NotificationFilter so the Phase 2 Green handler can route
// `?status=dead` (Dead letter tab) through this method while keeping the
// unfiltered path on ListNotifications for backward compat.
//
// Pinned by notification_handler_test.go:443-519 — the handler test asserts
// that a request with `?status=dead&page=1&per_page=50` lands on exactly
// this signature (`status string, page, perPage int`) and that requests
// without a status param do NOT call it. Keep the returned shape identical
// to ListNotifications so the handler can reuse its JSON-encoding path.
func (s *NotificationService) ListNotificationsByStatus(ctx context.Context, status string, page, perPage int) ([]domain.NotificationEvent, int64, error) {
if page < 1 {
page = 1
}
if perPage < 1 {
perPage = 50
}
filter := &repository.NotificationFilter{
Status: status,
Page: page,
PerPage: perPage,
}
notifications, err := s.notifRepo.List(ctx, filter)
if err != nil {
return nil, 0, fmt.Errorf("failed to list notifications by status: %w", err)
}
result := make([]domain.NotificationEvent, 0, len(notifications))
for _, n := range notifications {
if n != nil {
result = append(result, *n)
}
}
total := int64(len(result))
return result, total, nil
}
+350
View File
@@ -565,3 +565,353 @@ func TestGetNotificationHistory(t *testing.T) {
func stringPtr(s string) *string {
return &s
}
// ─── I-005 retry + DLQ service contract (Phase 1 Red) ─────────────────────
//
// These tests pin the service-layer contract the I-005 fix must satisfy. The
// Red signals they produce are, in compile order:
//
// 1. service.NotificationService.RetryFailedNotifications undefined
// 2. service.NotificationService.RequeueNotification undefined
// 3. mockNotifRepo.ListRetryEligible undefined (surfaced after the service
// method exists and starts calling it)
// 4. mockNotifRepo.RecordFailedAttempt undefined
// 5. mockNotifRepo.MarkAsDead undefined
// 6. mockNotifRepo.Requeue undefined
// 7. NotificationEvent.RetryCount / NextRetryAt / LastError undefined — but
// domain/notification_test.go already pins these, so they ride in on the
// Phase 2 Green domain edit and compile by the time the service-layer
// tests run.
//
// The contract under test, re-derived from notification.go:282-288:
// * A failed notifier.Send used to stamp status='failed' with a zero
// time.Time and return. I-005 reframes that row as retry-eligible with
// bookkeeping (retry_count, next_retry_at, last_error) so a sibling
// scheduler loop can promote it back to 'pending' until max_attempts,
// then to 'dead' (DLQ) for operator triage.
// * Backoff is 2^retry_count minutes, capped at 1h, mirroring the
// operator decision captured in the I-005 design notes.
// * Success on a retry promotes the row straight to 'sent' via
// UpdateStatus (no retry bookkeeping change).
// * Requeue is the operator-driven escape hatch from 'dead' back to
// 'pending' with retry_count reset to 0; service-layer impl is a
// pass-through to repo.Requeue so the audit trail is consistent.
const (
// i005MaxAttempts must match the same constant used by the Green
// service implementation. Declared here only so the test assertions
// read cleanly; Phase 2 is free to thread this from config.
i005MaxAttempts = 5
// i005BackoffCap mirrors the 1h ceiling on 2^retry_count minutes.
i005BackoffCap = time.Hour
)
// newFailedNotification builds a minimal failed-state row suitable for seeding
// the mock repo. retry_count is the number of attempts already consumed (so
// the next attempt becomes retry_count+1, and retry_count == max-1 puts the
// row at the exhaustion threshold).
func newFailedNotification(id string, retryCount int, nextRetryAt time.Time) *domain.NotificationEvent {
nextCopy := nextRetryAt
last := "connection refused"
return &domain.NotificationEvent{
ID: id,
Type: domain.NotificationTypeExpirationWarning,
Channel: domain.NotificationChannelEmail,
Recipient: "owner-i005@example.com",
Message: "retry me: " + id,
Status: string(domain.NotificationStatusFailed),
RetryCount: retryCount,
NextRetryAt: &nextCopy,
LastError: &last,
CreatedAt: time.Now().Add(-time.Hour),
}
}
// TestNotificationService_RetryFailedNotifications_NoEligibleRows asserts the
// no-op path: an empty retry queue must not trigger any notifier.Send calls
// and must not surface as an error. This pins that the retry loop's cost is
// O(retry-eligible), not O(total).
func TestNotificationService_RetryFailedNotifications_NoEligibleRows(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
notifier := newMockNotifier()
registry := map[string]Notifier{"Email": notifier}
svc := NewNotificationService(notifRepo, registry)
if err := svc.RetryFailedNotifications(ctx); err != nil {
t.Fatalf("RetryFailedNotifications on empty queue returned error: %v", err)
}
if got := notifier.getSentCount(); got != 0 {
t.Errorf("notifier.Send call count = %d, want 0 (no retry-eligible rows)", got)
}
}
// TestNotificationService_RetryFailedNotifications_ListError asserts that a
// ListRetryEligible failure short-circuits the loop. Notifier.Send must not
// fire — we never got a canonical set of rows to act on, so sending anything
// would risk double-delivery when the DB comes back.
func TestNotificationService_RetryFailedNotifications_ListError(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
notifRepo.ListErr = fmt.Errorf("simulated DB outage")
notifier := newMockNotifier()
registry := map[string]Notifier{"Email": notifier}
svc := NewNotificationService(notifRepo, registry)
err := svc.RetryFailedNotifications(ctx)
if err == nil {
t.Fatalf("RetryFailedNotifications must surface the list error; got nil")
}
if !strings.Contains(err.Error(), "simulated DB outage") {
t.Errorf("expected wrapped list error to mention 'simulated DB outage', got: %v", err)
}
if got := notifier.getSentCount(); got != 0 {
t.Errorf("notifier.Send must not fire when list fails; got %d sends", got)
}
}
// TestNotificationService_RetryFailedNotifications_SuccessPromotes asserts
// the happy path for a retry that succeeds: the row is promoted directly to
// 'sent' via UpdateStatus (mirroring ProcessPendingNotifications), and no
// retry bookkeeping mutation (RecordFailedAttempt / MarkAsDead) fires.
func TestNotificationService_RetryFailedNotifications_SuccessPromotes(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
notifier := newMockNotifier() // default: no error — Send succeeds
registry := map[string]Notifier{"Email": notifier}
svc := NewNotificationService(notifRepo, registry)
row := newFailedNotification("notif-success", 2, time.Now().Add(-time.Minute))
notifRepo.AddNotification(row)
if err := svc.RetryFailedNotifications(ctx); err != nil {
t.Fatalf("RetryFailedNotifications should not error on per-row success: %v", err)
}
if notifier.getSentCount() != 1 {
t.Errorf("expected exactly 1 notifier.Send call, got %d", notifier.getSentCount())
}
if row.Status != string(domain.NotificationStatusSent) {
t.Errorf("successful retry must promote status to 'sent', got %q", row.Status)
}
// retry_count must NOT increment on success — that would falsify the
// "this row was delivered on attempt N" signal the audit trail relies on.
if row.RetryCount != 2 {
t.Errorf("retry_count must not change on success, got %d (want 2)", row.RetryCount)
}
}
// TestNotificationService_RetryFailedNotifications_ExponentialBackoff asserts
// that a still-retriable failure schedules the next attempt at 2^retry_count
// minutes from now, matching the operator-approved curve 1m, 2m, 4m, 8m, 16m.
// The assertion is a window check against time.Now() because the service
// reads its own clock.
func TestNotificationService_RetryFailedNotifications_ExponentialBackoff(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
notifier := newMockNotifier()
notifier.SendErr = fmt.Errorf("smtp 451 temporary failure")
registry := map[string]Notifier{"Email": notifier}
svc := NewNotificationService(notifRepo, registry)
// retry_count=2 → next attempt is #3, backoff = 2^2 = 4 minutes.
row := newFailedNotification("notif-backoff", 2, time.Now().Add(-time.Minute))
notifRepo.AddNotification(row)
before := time.Now()
if err := svc.RetryFailedNotifications(ctx); err != nil {
t.Fatalf("RetryFailedNotifications should not bubble per-row send errors: %v", err)
}
after := time.Now()
// Still in 'failed' — not yet exhausted (retry_count+1 = 3, below max 5).
if row.Status != string(domain.NotificationStatusFailed) {
t.Errorf("status after non-terminal retry must stay 'failed', got %q", row.Status)
}
if row.RetryCount != 3 {
t.Errorf("retry_count must increment on failure, got %d (want 3)", row.RetryCount)
}
if row.NextRetryAt == nil {
t.Fatalf("NextRetryAt must be set on non-terminal retry failure; got nil")
}
expectedMin := before.Add(4 * time.Minute)
expectedMax := after.Add(4 * time.Minute)
if row.NextRetryAt.Before(expectedMin) || row.NextRetryAt.After(expectedMax) {
t.Errorf("NextRetryAt outside 2^2=4m window [%v, %v]; got %v",
expectedMin, expectedMax, *row.NextRetryAt)
}
if row.LastError == nil || !strings.Contains(*row.LastError, "smtp 451 temporary failure") {
t.Errorf("LastError must preserve the notifier error body for triage; got %v", row.LastError)
}
}
// TestNotificationService_RetryFailedNotifications_BackoffCap asserts the
// defense-in-depth 1h ceiling on next_retry_at. The retry curve under the
// operator-approved formula is pre-increment `2^retry_count` minutes — 1m,
// 2m, 4m, 8m — and with max_attempts=5 the deepest still-retriable row is
// retry_count=4 (next wait = 2^4 = 16m), which would transition to 'dead'
// before ever scheduling. So the largest actually-schedulable wait is
// 2^3=8m at retry_count=3, well under the 1h cap.
//
// That makes this test a ceiling-assertion, not a saturation-assertion: we
// pick retry_count=3 (matching ExponentialBackoff's formula but one step
// deeper) and verify (a) the window lands at 2^3=8m and (b) the cap is
// never exceeded. When max_attempts becomes configurable in a later
// milestone, this test becomes the natural home for a true cap-saturation
// fixture; for now it pins the arithmetic the Phase 2 Green implementation
// has to hit exactly.
func TestNotificationService_RetryFailedNotifications_BackoffCap(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
notifier := newMockNotifier()
notifier.SendErr = fmt.Errorf("webhook 502 bad gateway")
registry := map[string]Notifier{"Email": notifier}
svc := NewNotificationService(notifRepo, registry)
// retry_count=3 → pre-increment wait = 2^3 = 8 minutes. Post-increment
// retry_count becomes 4, which is still below max_attempts=5, so the
// row stays in 'failed' rather than transitioning to 'dead'.
row := newFailedNotification("notif-backoff-cap", 3, time.Now().Add(-time.Minute))
notifRepo.AddNotification(row)
before := time.Now()
if err := svc.RetryFailedNotifications(ctx); err != nil {
t.Fatalf("RetryFailedNotifications should not bubble per-row send errors: %v", err)
}
after := time.Now()
if row.Status != string(domain.NotificationStatusFailed) {
t.Errorf("mid-retry status must stay 'failed', got %q", row.Status)
}
if row.RetryCount != 4 {
t.Errorf("retry_count must increment on failure, got %d (want 4)", row.RetryCount)
}
if row.NextRetryAt == nil {
t.Fatalf("NextRetryAt must be set; got nil")
}
// retry_count=3 → pre-increment 2^3 = 8m, matching the curve pinned by
// ExponentialBackoff (retry_count=2 → 2^2=4m).
expectedMin := before.Add(8 * time.Minute)
expectedMax := after.Add(8 * time.Minute)
if row.NextRetryAt.Before(expectedMin) || row.NextRetryAt.After(expectedMax) {
t.Errorf("NextRetryAt outside 2^3=8m window [%v, %v]; got %v",
expectedMin, expectedMax, *row.NextRetryAt)
}
// And regardless of retry_count, the ceiling must hold: next_retry_at
// must never be more than i005BackoffCap (1h) from now. This is the
// defense-in-depth assertion — it would fail loudly if a future
// refactor swapped to post-increment and overshot on a deeper row.
if row.NextRetryAt.After(after.Add(i005BackoffCap + time.Second)) {
t.Errorf("NextRetryAt violates 1h cap; scheduled %v in the future",
row.NextRetryAt.Sub(after))
}
}
// TestNotificationService_RetryFailedNotifications_MarkDeadOnExhaustion
// asserts the terminal transition: once retry_count crosses max_attempts,
// the row moves to 'dead' (DLQ) and stops participating in the retry sweep.
// next_retry_at must be cleared — otherwise the partial retry-sweep index
// would still pick it up and we'd loop forever.
func TestNotificationService_RetryFailedNotifications_MarkDeadOnExhaustion(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
notifier := newMockNotifier()
notifier.SendErr = fmt.Errorf("connection refused after max attempts")
registry := map[string]Notifier{"Email": notifier}
svc := NewNotificationService(notifRepo, registry)
// retry_count = max-1: this attempt makes it max, so the row must
// transition to 'dead', not get rescheduled.
row := newFailedNotification("notif-dead", i005MaxAttempts-1, time.Now().Add(-time.Minute))
notifRepo.AddNotification(row)
if err := svc.RetryFailedNotifications(ctx); err != nil {
t.Fatalf("RetryFailedNotifications must not bubble per-row exhaustion: %v", err)
}
if row.Status != string(domain.NotificationStatusDead) {
t.Errorf("exhausted row must be in 'dead' status, got %q", row.Status)
}
if row.NextRetryAt != nil {
t.Errorf("dead row must have next_retry_at cleared (else retry sweep keeps picking it up); got %v", *row.NextRetryAt)
}
if row.LastError == nil || !strings.Contains(*row.LastError, "connection refused after max attempts") {
t.Errorf("LastError on dead row must preserve final failure reason; got %v", row.LastError)
}
}
// TestNotificationService_RequeueNotification_Success asserts the operator
// escape hatch: Requeue flips a dead row back to 'pending' with
// retry_count=0 so ProcessPendingNotifications can pick it up on the very
// next tick. The service delegates to repo.Requeue and propagates no error.
func TestNotificationService_RequeueNotification_Success(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
registry := map[string]Notifier{"Email": newMockNotifier()}
svc := NewNotificationService(notifRepo, registry)
next := time.Now().Add(10 * time.Minute)
last := "max attempts exceeded"
dead := &domain.NotificationEvent{
ID: "notif-requeue",
Type: domain.NotificationTypeExpirationWarning,
Channel: domain.NotificationChannelEmail,
Recipient: "owner@example.com",
Message: "please requeue me",
Status: string(domain.NotificationStatusDead),
RetryCount: i005MaxAttempts,
NextRetryAt: &next,
LastError: &last,
CreatedAt: time.Now().Add(-2 * time.Hour),
}
notifRepo.AddNotification(dead)
if err := svc.RequeueNotification(ctx, dead.ID); err != nil {
t.Fatalf("RequeueNotification(%s) returned error: %v", dead.ID, err)
}
if dead.Status != string(domain.NotificationStatusPending) {
t.Errorf("Requeue must flip status to 'pending', got %q", dead.Status)
}
if dead.RetryCount != 0 {
t.Errorf("Requeue must reset retry_count to 0, got %d", dead.RetryCount)
}
if dead.NextRetryAt != nil {
t.Errorf("Requeue must clear next_retry_at (pending rows never have it), got %v", *dead.NextRetryAt)
}
if dead.LastError != nil {
t.Errorf("Requeue must clear last_error (pending is a fresh attempt), got %v", *dead.LastError)
}
}
// TestNotificationService_RequeueNotification_RepoError asserts that a
// failed Requeue at the repository layer surfaces cleanly. The service has
// no fallback here — if the DB can't update the row, the operator action
// must fail loudly rather than silently "succeed" in the UI.
func TestNotificationService_RequeueNotification_RepoError(t *testing.T) {
ctx := context.Background()
notifRepo := newMockNotificationRepository()
notifRepo.UpdateErr = fmt.Errorf("pg: deadlock detected")
registry := map[string]Notifier{"Email": newMockNotifier()}
svc := NewNotificationService(notifRepo, registry)
// Seed a dead row so the service has something to act on (the error
// must come from the repo write, not from a missing ID).
dead := &domain.NotificationEvent{
ID: "notif-requeue-err",
Type: domain.NotificationTypeExpirationWarning,
Channel: domain.NotificationChannelEmail,
Status: string(domain.NotificationStatusDead),
}
notifRepo.AddNotification(dead)
err := svc.RequeueNotification(ctx, dead.ID)
if err == nil {
t.Fatalf("RequeueNotification must surface repo errors; got nil")
}
if !strings.Contains(err.Error(), "pg: deadlock detected") {
t.Errorf("expected wrapped repo error to mention 'pg: deadlock detected', got: %v", err)
}
}
+46 -11
View File
@@ -15,6 +15,12 @@ type StatsService struct {
certRepo repository.CertificateRepository
jobRepo repository.JobRepository
agentRepo repository.AgentRepository
// notifRepo is injected post-construction via SetNotifRepo so that
// NewStatsService's nine call sites (main.go + stats_test.go + 8 digest
// tests) keep their existing signatures. When nil, the dead-letter count
// falls through to zero — see GetDashboardSummary. I-005 coverage-gap
// closure.
notifRepo repository.NotificationRepository
}
// NewStatsService creates a new stats service.
@@ -30,19 +36,35 @@ func NewStatsService(
}
}
// SetNotifRepo injects the notification repository used to populate
// DashboardSummary.NotificationsDead. Setter pattern (matching the
// certificateService.SetTargetRepo / SetProfileRepo / SetDigestService
// precedent) keeps the NewStatsService signature stable across its
// pre-existing call sites. I-005 coverage-gap closure.
func (s *StatsService) SetNotifRepo(notifRepo repository.NotificationRepository) {
s.notifRepo = notifRepo
}
// DashboardSummary represents a high-level summary of system state.
type DashboardSummary struct {
TotalCertificates int64 `json:"total_certificates"`
ExpiringCertificates int64 `json:"expiring_certificates"`
ExpiredCertificates int64 `json:"expired_certificates"`
RevokedCertificates int64 `json:"revoked_certificates"`
ActiveAgents int64 `json:"active_agents"`
OfflineAgents int64 `json:"offline_agents"`
TotalAgents int64 `json:"total_agents"`
PendingJobs int64 `json:"pending_jobs"`
FailedJobs int64 `json:"failed_jobs"`
CompleteJobs int64 `json:"complete_jobs"`
CompletedAt time.Time `json:"completed_at"`
TotalCertificates int64 `json:"total_certificates"`
ExpiringCertificates int64 `json:"expiring_certificates"`
ExpiredCertificates int64 `json:"expired_certificates"`
RevokedCertificates int64 `json:"revoked_certificates"`
ActiveAgents int64 `json:"active_agents"`
OfflineAgents int64 `json:"offline_agents"`
TotalAgents int64 `json:"total_agents"`
PendingJobs int64 `json:"pending_jobs"`
FailedJobs int64 `json:"failed_jobs"`
CompleteJobs int64 `json:"complete_jobs"`
// NotificationsDead is the number of notification_events rows currently
// in the terminal "dead" status (I-005 dead-letter queue). Exposed here
// so the metrics handler can derive the Prometheus counter
// certctl_notification_dead_total from the same snapshot used by the
// dashboard. DB-COUNT rather than in-memory — notifications can grow
// without bound, and filter-based List() is PerPage-capped to 50.
NotificationsDead int64 `json:"notifications_dead"`
CompletedAt time.Time `json:"completed_at"`
}
// GetDashboardSummary returns a summary of key metrics.
@@ -106,6 +128,19 @@ func (s *StatsService) GetDashboardSummary(ctx context.Context) (interface{}, er
}
}
// I-005: dead-letter count for certctl_notification_dead_total. nil-safe
// so the nine existing NewStatsService call sites that haven't yet been
// updated to call SetNotifRepo keep working — they'll simply report
// NotificationsDead=0, which is the correct value on a system without a
// notification repository wired in. A CountByStatus error is non-fatal:
// the dashboard summary is best-effort for this field.
if s.notifRepo != nil {
deadCount, err := s.notifRepo.CountByStatus(ctx, string(domain.NotificationStatusDead))
if err == nil {
summary.NotificationsDead = deadCount
}
}
return summary, nil
}
+194 -20
View File
@@ -157,20 +157,20 @@ func (m *mockCertRepo) AddCert(cert *domain.ManagedCertificate) {
// mockJobRepo is a test implementation of JobRepository
type mockJobRepo struct {
mu sync.Mutex
Jobs map[string]*domain.Job
StatusUpdates map[string]domain.JobStatus
CreateErr error
UpdateErr error
UpdateErrorByID map[string]error
UpdateErrorByIDMu sync.Mutex
UpdateStatusErr error
GetErr error
ListErr error
ListByStatusErr error
DeleteErr error
ListTimedOutErr error
Updated []*domain.Job
mu sync.Mutex
Jobs map[string]*domain.Job
StatusUpdates map[string]domain.JobStatus
CreateErr error
UpdateErr error
UpdateErrorByID map[string]error
UpdateErrorByIDMu sync.Mutex
UpdateStatusErr error
GetErr error
ListErr error
ListByStatusErr error
DeleteErr error
ListTimedOutErr error
Updated []*domain.Job
}
func (m *mockJobRepo) List(ctx context.Context) ([]*domain.Job, error) {
@@ -393,13 +393,36 @@ func (m *mockJobRepo) AddJob(job *domain.Job) {
m.Jobs[job.ID] = job
}
// mockNotifRepo is a test implementation of NotificationRepository
// mockNotifRepo is a test implementation of NotificationRepository.
//
// I-005 extensions (ListRetryEligible / RecordFailedAttempt / MarkAsDead /
// Requeue) mutate the seeded *domain.NotificationEvent pointers in place.
// The service tests in notification_test.go assert against those same
// pointers (via notifRepo.Notifications or the local `row` handle), so
// in-place mutation is the contract — not a copy-and-replace pattern.
//
// Error fields are layered:
// - Per-method errors (ListRetryEligibleErr, RecordFailedAttemptErr, etc.)
// for fine-grained failure injection when a test targets exactly one
// method.
// - Shared legacy errors (ListErr for list-shaped reads, UpdateErr for
// update-shaped writes) so the pre-I-005 tests that configure ListErr
// or UpdateErr continue to short-circuit the new methods too. The
// RequeueNotification_RepoError test deliberately relies on this by
// setting UpdateErr rather than RequeueErr.
type mockNotifRepo struct {
mu sync.Mutex
Notifications []*domain.NotificationEvent
CreateErr error
ListErr error
UpdateErr error
// I-005 per-method failure injection.
ListRetryEligibleErr error
RecordFailedAttemptErr error
MarkAsDeadErr error
RequeueErr error
CountByStatusErr error
}
func (m *mockNotifRepo) Create(ctx context.Context, notif *domain.NotificationEvent) error {
@@ -436,12 +459,163 @@ func (m *mockNotifRepo) UpdateStatus(ctx context.Context, id string, status stri
return errNotFound
}
// ListRetryEligible returns failed rows whose NextRetryAt is non-nil, at or
// before beforeTime, AND whose RetryCount is strictly less than maxAttempts,
// ordered oldest-due first, capped at limit. Signature matches the postgres-
// canonical shape pinned by notification_test.go:118 ("repo.ListRetryEligible
// (ctx, now, 5, 100)") and the NotificationRepository interface at
// interfaces.go:308 — a row at retry_count == maxAttempts is NOT returned
// because the service has already exhausted its attempt budget and the row
// must be MarkAsDead'd by whichever tick last touched it, not re-swept here.
// Mirrors the partial-index predicate
// `WHERE status='failed' AND next_retry_at IS NOT NULL AND next_retry_at <= $1`
// that migration 000016's retry-sweep index makes cheap to scan; the
// retry_count filter is an extra Go-side guard so the mock behaves
// identically to the postgres `AND retry_count < $2` clause.
func (m *mockNotifRepo) ListRetryEligible(ctx context.Context, beforeTime time.Time, maxAttempts, limit int) ([]*domain.NotificationEvent, error) {
m.mu.Lock()
defer m.mu.Unlock()
if m.ListRetryEligibleErr != nil {
return nil, m.ListRetryEligibleErr
}
if m.ListErr != nil {
return nil, m.ListErr
}
eligible := make([]*domain.NotificationEvent, 0)
for _, n := range m.Notifications {
if n.Status != string(domain.NotificationStatusFailed) {
continue
}
if n.NextRetryAt == nil {
continue
}
if n.NextRetryAt.After(beforeTime) {
continue
}
if n.RetryCount >= maxAttempts {
continue
}
eligible = append(eligible, n)
}
// Oldest-due first so the service processes the most-overdue row first,
// matching how an ORDER BY next_retry_at ASC query would behave.
sort.Slice(eligible, func(i, j int) bool {
return eligible[i].NextRetryAt.Before(*eligible[j].NextRetryAt)
})
if limit > 0 && len(eligible) > limit {
eligible = eligible[:limit]
}
return eligible, nil
}
// RecordFailedAttempt mutates the matched row in place: increments
// retry_count, pins next_retry_at, stores last_error, and keeps the row in
// 'failed' state so the next retry-sweep tick picks it up again. Service-
// level backoff math happens before the call; the repo is a dumb setter.
// Signature matches the postgres-canonical shape pinned by
// notification_test.go:184 ("repo.RecordFailedAttempt(ctx, 'notif-attempt-1',
// 'connection refused', nextTry)") and the NotificationRepository interface
// at interfaces.go:315 — id, then lastError, then nextRetryAt. The earlier
// (id, nextRetryAt, lastError) ordering from the Phase 1 Red seed was wrong
// and is corrected here in Phase 2 Green.
func (m *mockNotifRepo) RecordFailedAttempt(ctx context.Context, id string, lastError string, nextRetryAt time.Time) error {
m.mu.Lock()
defer m.mu.Unlock()
if m.RecordFailedAttemptErr != nil {
return m.RecordFailedAttemptErr
}
if m.UpdateErr != nil {
return m.UpdateErr
}
for _, n := range m.Notifications {
if n.ID == id {
n.RetryCount++
next := nextRetryAt
n.NextRetryAt = &next
le := lastError
n.LastError = &le
n.Status = string(domain.NotificationStatusFailed)
return nil
}
}
return errNotFound
}
// MarkAsDead flips the row into the terminal DLQ state. next_retry_at is
// cleared so the partial retry-sweep index no longer touches this row —
// otherwise RetryFailedNotifications would loop over it forever without
// making any state change.
func (m *mockNotifRepo) MarkAsDead(ctx context.Context, id string, lastError string) error {
m.mu.Lock()
defer m.mu.Unlock()
if m.MarkAsDeadErr != nil {
return m.MarkAsDeadErr
}
if m.UpdateErr != nil {
return m.UpdateErr
}
for _, n := range m.Notifications {
if n.ID == id {
n.Status = string(domain.NotificationStatusDead)
n.NextRetryAt = nil
le := lastError
n.LastError = &le
return nil
}
}
return errNotFound
}
// Requeue is the operator-driven escape hatch from 'dead' back to 'pending'.
// Clears retry bookkeeping entirely so ProcessPendingNotifications treats
// the requeued row as a fresh attempt — identical on the wire to a freshly-
// created notification.
func (m *mockNotifRepo) Requeue(ctx context.Context, id string) error {
m.mu.Lock()
defer m.mu.Unlock()
if m.RequeueErr != nil {
return m.RequeueErr
}
if m.UpdateErr != nil {
return m.UpdateErr
}
for _, n := range m.Notifications {
if n.ID == id {
n.Status = string(domain.NotificationStatusPending)
n.RetryCount = 0
n.NextRetryAt = nil
n.LastError = nil
return nil
}
}
return errNotFound
}
func (m *mockNotifRepo) AddNotification(notif *domain.NotificationEvent) {
m.mu.Lock()
defer m.mu.Unlock()
m.Notifications = append(m.Notifications, notif)
}
// CountByStatus counts in-memory rows whose Status field matches exactly.
// Dedicated error injection via CountByStatusErr so a test can assert the
// StatsService wrap-path ("failed to count dead notifications: …") without
// also tripping ListErr or other shared fields. I-005 Phase 2 Green.
func (m *mockNotifRepo) CountByStatus(ctx context.Context, status string) (int64, error) {
m.mu.Lock()
defer m.mu.Unlock()
if m.CountByStatusErr != nil {
return 0, m.CountByStatusErr
}
var count int64
for _, n := range m.Notifications {
if n.Status == status {
count++
}
}
return count, nil
}
// mockAuditRepo is a test implementation of AuditRepository
type mockAuditRepo struct {
mu sync.Mutex
@@ -635,10 +809,10 @@ type mockAgentRepo struct {
// or RetireAgentWithCascade failure after preflight passed, so the
// service's error surfacing (wrap+return, skip audit, etc.) can be
// exercised without having to stand up a real PG connection.
SoftRetireErr error
SoftRetireErr error
RetireCascadeErr error
CountErr error
ListRetiredErr error
CountErr error
ListRetiredErr error
}
// List mirrors the production repo contract post-I-004: it returns only
@@ -993,8 +1167,8 @@ func newMockTargetRepository() *mockTargetRepo {
// mockIssuerConnector is a test implementation of IssuerConnector
type mockIssuerConnector struct {
Result *IssuanceResult
Err error
Result *IssuanceResult
Err error
getRenewalInfoResult *RenewalInfoResult
getRenewalInfoErr error
// LastOCSPSignRequest captures the last request passed to SignOCSPResponse.