feat: add configurable notification dispatch retries (#1655)

* feat: add configurable notification dispatch retries

Allow each node to set 0-3 extra in-process delivery attempts with a fixed
one-second delay for routes, agents, and Test dispatch.

* fix: harden notification retry settings load/save and channel tests

Guard Delivery retries against failed and out-of-order same-node settings responses, and cover Slack/webhook retry classification alongside Discord.

* fix: clear Delivery retries saving state and correct screenshot alt

Separate save-request ownership from value-generation invalidation so a successful PATCH cannot leave Save retries stuck on Saving, reset saving on node switch, and align the Channels screenshot alt with the committed image.

* fix: surface invalid notification retry settings instead of false saved clamp

Align Channels GET handling with the backend strict 0-3 parser so stored values like 9 or 1.5 show as error needing repair, matching runtime fallback to 0 instead of displaying a clamped saved policy.

backend/src/__tests__/notification-apprise.test.ts

@@ -11,6 +11,7 @@ const {
   mockGetStackLabelIds,
   mockAddNotificationHistory,
   mockUpdateNotificationDispatchError,
+  mockGetGlobalSettings,
 } = vi.hoisted(() => ({
   mockGetEnabledNotificationRoutes: vi.fn().mockReturnValue([]),
   mockGetEnabledNotificationSuppressionRules: vi.fn().mockReturnValue([]),
@@ -24,6 +25,7 @@ const {
     is_read: 0,
   }),
   mockUpdateNotificationDispatchError: vi.fn(),
+  mockGetGlobalSettings: vi.fn().mockReturnValue({ notification_dispatch_retries: '0' }),
 }));
 
 vi.mock('../services/DatabaseService', () => ({
@@ -35,6 +37,7 @@ vi.mock('../services/DatabaseService', () => ({
       getStackLabelIds: mockGetStackLabelIds,
       addNotificationHistory: mockAddNotificationHistory,
       updateNotificationDispatchError: mockUpdateNotificationDispatchError,
+      getGlobalSettings: mockGetGlobalSettings,
     }),
   },
 }));

backend/src/__tests__/notification-retries.test.ts

@@ -0,0 +1,337 @@
+/**
+ * Configurable in-process notification dispatch retries.
+ */
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+
+const {
+  mockGetEnabledNotificationRoutes,
+  mockGetEnabledNotificationSuppressionRules,
+  mockGetEnabledAgents,
+  mockGetStackLabelIds,
+  mockAddNotificationHistory,
+  mockUpdateNotificationDispatchError,
+  mockGetGlobalSettings,
+} = vi.hoisted(() => ({
+  mockGetEnabledNotificationRoutes: vi.fn().mockReturnValue([]),
+  mockGetEnabledNotificationSuppressionRules: vi.fn().mockReturnValue([]),
+  mockGetEnabledAgents: vi.fn().mockReturnValue([]),
+  mockGetStackLabelIds: vi.fn().mockReturnValue([]),
+  mockAddNotificationHistory: vi.fn().mockReturnValue({
+    id: 42,
+    level: 'error',
+    message: 'test',
+    timestamp: Date.now(),
+    is_read: 0,
+  }),
+  mockUpdateNotificationDispatchError: vi.fn(),
+  mockGetGlobalSettings: vi.fn().mockReturnValue({ notification_dispatch_retries: '0' }),
+}));
+
+vi.mock('../services/DatabaseService', () => ({
+  DatabaseService: {
+    getInstance: () => ({
+      getEnabledNotificationRoutes: mockGetEnabledNotificationRoutes,
+      getEnabledNotificationSuppressionRules: mockGetEnabledNotificationSuppressionRules,
+      getEnabledAgents: mockGetEnabledAgents,
+      getStackLabelIds: mockGetStackLabelIds,
+      addNotificationHistory: mockAddNotificationHistory,
+      updateNotificationDispatchError: mockUpdateNotificationDispatchError,
+      getGlobalSettings: mockGetGlobalSettings,
+    }),
+  },
+}));
+
+vi.mock('../services/NodeRegistry', () => ({
+  NodeRegistry: {
+    getInstance: () => ({
+      getDefaultNodeId: () => 1,
+      getComposeDir: () => '/app/compose',
+    }),
+  },
+}));
+
+vi.mock('../services/StackActivityMetricsService', () => ({
+  StackActivityMetricsService: {
+    getInstance: () => ({ record: vi.fn() }),
+  },
+}));
+
+import { NotificationService } from '../services/NotificationService';
+
+const DISCORD = 'https://discord.com/api/webhooks/1/token';
+
+function makeRoute(overrides: Record<string, unknown> = {}) {
+  return {
+    id: 1,
+    name: 'Prod Discord',
+    node_id: null as number | null,
+    stack_patterns: [] as string[],
+    label_ids: null as number[] | null,
+    categories: null as string[] | null,
+    levels: null as ('info' | 'warning' | 'error')[] | null,
+    channel_type: 'discord' as const,
+    channel_url: DISCORD,
+    priority: 0,
+    enabled: true,
+    created_at: Date.now(),
+    updated_at: Date.now(),
+    ...overrides,
+  };
+}
+
+describe('notification dispatch retries', () => {
+  let svc: NotificationService;
+  let mockFetch: ReturnType<typeof vi.fn>;
+
+  beforeEach(() => {
+    (NotificationService as unknown as { instance?: NotificationService }).instance = undefined;
+    NotificationService.setRetryDelayMsForTests(0);
+    svc = NotificationService.getInstance();
+    mockFetch = vi.fn().mockResolvedValue({ ok: true, status: 200 });
+    vi.stubGlobal('fetch', mockFetch);
+    mockGetEnabledNotificationRoutes.mockReturnValue([]);
+    mockGetEnabledNotificationSuppressionRules.mockReturnValue([]);
+    mockGetEnabledAgents.mockReturnValue([]);
+    mockGetStackLabelIds.mockReturnValue([]);
+    mockUpdateNotificationDispatchError.mockClear();
+    mockAddNotificationHistory.mockClear();
+    mockGetGlobalSettings.mockReset();
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '0' });
+  });
+
+  afterEach(() => {
+    NotificationService.setRetryDelayMsForTests(1000);
+    vi.unstubAllGlobals();
+  });
+
+  it('retries=0 performs a single fetch', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '0' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute()]);
+    mockFetch.mockResolvedValue({ ok: false, status: 500 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockFetch).toHaveBeenCalledTimes(1);
+    expect(mockGetGlobalSettings).toHaveBeenCalledTimes(1);
+  });
+
+  it('retries=2 on persistent 5xx performs three attempts', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '2' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute()]);
+    mockFetch.mockResolvedValue({ ok: false, status: 502 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockFetch).toHaveBeenCalledTimes(3);
+    expect(mockUpdateNotificationDispatchError).toHaveBeenCalledWith(
+      42,
+      expect.stringContaining('HTTP 502'),
+    );
+  });
+
+  it('uses a fixed 1s delay between retryable attempts in production config', async () => {
+    NotificationService.setRetryDelayMsForTests(1000);
+    vi.useFakeTimers();
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '1' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute()]);
+    mockFetch
+      .mockResolvedValueOnce({ ok: false, status: 503 })
+      .mockResolvedValueOnce({ ok: true, status: 200 });
+
+    try {
+      const p = svc.dispatchAlert('error', 'monitor_alert', 'down');
+      // First attempt runs immediately; do not advance AbortSignal.timeout (10s).
+      await vi.advanceTimersByTimeAsync(0);
+      await Promise.resolve();
+      expect(mockFetch).toHaveBeenCalledTimes(1);
+      await vi.advanceTimersByTimeAsync(999);
+      expect(mockFetch).toHaveBeenCalledTimes(1);
+      await vi.advanceTimersByTimeAsync(1);
+      await p;
+      expect(mockFetch).toHaveBeenCalledTimes(2);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it('stops after success on the second attempt', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '2' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute()]);
+    mockFetch
+      .mockResolvedValueOnce({ ok: false, status: 503 })
+      .mockResolvedValueOnce({ ok: true, status: 200 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockFetch).toHaveBeenCalledTimes(2);
+    expect(mockUpdateNotificationDispatchError).not.toHaveBeenCalled();
+    expect(mockAddNotificationHistory).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not retry non-retryable 4xx', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '3' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute()]);
+    mockFetch.mockResolvedValue({ ok: false, status: 404 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockFetch).toHaveBeenCalledTimes(1);
+  });
+
+  it('reads settings once for multi-destination fanout', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '1' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([
+      makeRoute({ id: 1, name: 'A', channel_url: `${DISCORD}-a` }),
+      makeRoute({ id: 2, name: 'B', channel_url: `${DISCORD}-b` }),
+    ]);
+    mockFetch.mockResolvedValue({ ok: true, status: 200 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockGetGlobalSettings).toHaveBeenCalledTimes(1);
+    expect(mockFetch).toHaveBeenCalledTimes(2);
+  });
+
+  it('falls back to zero retries when settings throw and still sends once', async () => {
+    mockGetGlobalSettings.mockImplementation(() => {
+      throw new Error('db down');
+    });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute()]);
+    mockFetch.mockResolvedValue({ ok: false, status: 500 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockFetch).toHaveBeenCalledTimes(1);
+  });
+
+  it('falls back to zero retries for corrupt stored values', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '9' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute()]);
+    mockFetch.mockResolvedValue({ ok: false, status: 500 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockFetch).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not retry unsupported channel types', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '2' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([
+      makeRoute({ channel_type: 'sms' as 'discord', channel_url: 'https://example.com' }),
+    ]);
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    expect(mockFetch).not.toHaveBeenCalled();
+    expect(mockUpdateNotificationDispatchError).toHaveBeenCalledWith(
+      42,
+      expect.stringContaining('Unsupported channel type'),
+    );
+  });
+
+  it('aggregates final errors from multiple failed destinations', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '0' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([
+      makeRoute({ id: 1, name: 'One', channel_url: `${DISCORD}-1` }),
+      makeRoute({ id: 2, name: 'Two', channel_url: `${DISCORD}-2` }),
+    ]);
+    mockFetch
+      .mockResolvedValueOnce({ ok: false, status: 500 })
+      .mockResolvedValueOnce({ ok: false, status: 503 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    const joined = mockUpdateNotificationDispatchError.mock.calls[0][1] as string;
+    expect(joined).toContain('Route "One"');
+    expect(joined).toContain('Route "Two"');
+  });
+
+  it('records the last attempt message for a destination after retries', async () => {
+    mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '1' });
+    mockGetEnabledNotificationRoutes.mockReturnValue([makeRoute({ name: 'Flaky' })]);
+    mockFetch
+      .mockResolvedValueOnce({ ok: false, status: 500 })
+      .mockResolvedValueOnce({ ok: false, status: 503 });
+
+    await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+    const joined = mockUpdateNotificationDispatchError.mock.calls[0][1] as string;
+    expect(joined).toContain('HTTP 503');
+    expect(joined).not.toContain('HTTP 500');
+  });
+
+  describe('channel retry classification matrix', () => {
+    const channels: Array<{ type: 'discord' | 'slack' | 'webhook'; url: string }> = [
+      { type: 'discord', url: DISCORD },
+      { type: 'slack', url: 'https://hooks.slack.com/services/T/B/X' },
+      { type: 'webhook', url: 'https://example.com/hooks/sencho' },
+    ];
+
+    for (const channel of channels) {
+      it(`${channel.type}: does not retry non-retryable 4xx`, async () => {
+        mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '3' });
+        mockGetEnabledNotificationRoutes.mockReturnValue([
+          makeRoute({ channel_type: channel.type, channel_url: channel.url }),
+        ]);
+        mockFetch.mockResolvedValue({ ok: false, status: 404 });
+
+        await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+        expect(mockFetch).toHaveBeenCalledTimes(1);
+      });
+
+      it(`${channel.type}: retries persistent 5xx for configured extras`, async () => {
+        mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '2' });
+        mockGetEnabledNotificationRoutes.mockReturnValue([
+          makeRoute({ channel_type: channel.type, channel_url: channel.url }),
+        ]);
+        mockFetch.mockResolvedValue({ ok: false, status: 502 });
+
+        await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+        expect(mockFetch).toHaveBeenCalledTimes(3);
+      });
+
+      it(`${channel.type}: stops after a successful retry`, async () => {
+        mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '2' });
+        mockGetEnabledNotificationRoutes.mockReturnValue([
+          makeRoute({ channel_type: channel.type, channel_url: channel.url }),
+        ]);
+        mockFetch
+          .mockResolvedValueOnce({ ok: false, status: 503 })
+          .mockResolvedValueOnce({ ok: true, status: 200 });
+
+        await svc.dispatchAlert('error', 'monitor_alert', 'down');
+
+        expect(mockFetch).toHaveBeenCalledTimes(2);
+        expect(mockUpdateNotificationDispatchError).not.toHaveBeenCalled();
+      });
+    }
+  });
+
+
+  describe('testDispatch parity', () => {
+    it('retries a retryable failure then succeeds', async () => {
+      mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '1' });
+      mockFetch
+        .mockResolvedValueOnce({ ok: false, status: 502 })
+        .mockResolvedValueOnce({ ok: true, status: 200 });
+
+      await svc.testDispatch('discord', DISCORD);
+
+      expect(mockGetGlobalSettings).toHaveBeenCalledTimes(1);
+      expect(mockFetch).toHaveBeenCalledTimes(2);
+    });
+
+    it('does not retry a non-retryable test failure', async () => {
+      mockGetGlobalSettings.mockReturnValue({ notification_dispatch_retries: '3' });
+      mockFetch.mockResolvedValue({ ok: false, status: 401 });
+
+      await expect(svc.testDispatch('discord', DISCORD)).rejects.toMatchObject({
+        status: 401,
+        retryable: false,
+      });
+      expect(mockFetch).toHaveBeenCalledTimes(1);
+    });
+  });
+});

backend/src/__tests__/notification-routing.test.ts

@@ -13,6 +13,7 @@ const {
   mockGetStackLabelIds,
   mockAddNotificationHistory,
   mockUpdateNotificationDispatchError,
+  mockGetGlobalSettings,
 } = vi.hoisted(() => ({
   mockGetEnabledNotificationRoutes: vi.fn().mockReturnValue([]),
   mockGetEnabledNotificationSuppressionRules: vi.fn().mockReturnValue([]),
@@ -26,6 +27,7 @@ const {
     is_read: 0,
   }),
   mockUpdateNotificationDispatchError: vi.fn(),
+  mockGetGlobalSettings: vi.fn().mockReturnValue({ notification_dispatch_retries: '0' }),
 }));
 
 vi.mock('../services/DatabaseService', () => ({
@@ -37,6 +39,7 @@ vi.mock('../services/DatabaseService', () => ({
       getStackLabelIds: mockGetStackLabelIds,
       addNotificationHistory: mockAddNotificationHistory,
       updateNotificationDispatchError: mockUpdateNotificationDispatchError,
+      getGlobalSettings: mockGetGlobalSettings,
     }),
   },
 }));
@@ -301,7 +304,7 @@ describe('NotificationService - routing logic', () => {
 
     expect(mockUpdateNotificationDispatchError).toHaveBeenCalledWith(
       1, // notification id from mock
-      expect.stringContaining('Connection refused')
+      expect.stringContaining('Discord webhook request failed')
     );
   });
 
@@ -314,7 +317,7 @@ describe('NotificationService - routing logic', () => {
 
     expect(mockUpdateNotificationDispatchError).toHaveBeenCalledWith(
       1,
-      expect.stringContaining('Timeout')
+      expect.stringContaining('Slack webhook request failed')
     );
   });
 

backend/src/__tests__/notification-suppression.test.ts

@@ -12,6 +12,7 @@ const {
   mockGetEnabledNotificationSuppressionRules,
   mockUpdateNotificationSuppressionMatch,
   mockBroadcast,
+  mockGetGlobalSettings,
 } = vi.hoisted(() => ({
   mockGetEnabledNotificationRoutes: vi.fn().mockReturnValue([]),
   mockGetEnabledAgents: vi.fn().mockReturnValue([]),
@@ -27,6 +28,7 @@ const {
   mockGetEnabledNotificationSuppressionRules: vi.fn().mockReturnValue([]),
   mockUpdateNotificationSuppressionMatch: vi.fn(),
   mockBroadcast: vi.fn(),
+  mockGetGlobalSettings: vi.fn().mockReturnValue({ notification_dispatch_retries: '0' }),
 }));
 
 vi.mock('../services/DatabaseService', () => ({
@@ -39,6 +41,7 @@ vi.mock('../services/DatabaseService', () => ({
       updateNotificationDispatchError: mockUpdateNotificationDispatchError,
       getEnabledNotificationSuppressionRules: mockGetEnabledNotificationSuppressionRules,
       updateNotificationSuppressionMatch: mockUpdateNotificationSuppressionMatch,
+      getGlobalSettings: mockGetGlobalSettings,
     }),
   },
 }));

backend/src/__tests__/settings-routes.test.ts

@@ -338,6 +338,64 @@ describe('health gate settings', () => {
   });
 });
 
+describe('notification_dispatch_retries setting', () => {
+  it('seeds to "0" in a fresh database', () => {
+    expect(DatabaseService.getInstance().getGlobalSettings().notification_dispatch_retries).toBe('0');
+  });
+
+  it('is exposed through the settings GET projection', async () => {
+    const res = await request(app).get('/api/settings').set('Cookie', adminCookie);
+    expect(res.status).toBe(200);
+    expect(res.body.notification_dispatch_retries).toBe('0');
+  });
+
+  it('accepts integer number and digit-string writes via POST and PATCH', async () => {
+    const postNum = await request(app)
+      .post('/api/settings')
+      .set('Cookie', adminCookie)
+      .send({ key: 'notification_dispatch_retries', value: 2 });
+    expect(postNum.status).toBe(200);
+    expect(DatabaseService.getInstance().getGlobalSettings().notification_dispatch_retries).toBe('2');
+
+    const patchStr = await request(app)
+      .patch('/api/settings')
+      .set('Cookie', adminCookie)
+      .send({ notification_dispatch_retries: '3' });
+    expect(patchStr.status).toBe(200);
+    expect(DatabaseService.getInstance().getGlobalSettings().notification_dispatch_retries).toBe('3');
+
+    DatabaseService.getInstance().updateGlobalSetting('notification_dispatch_retries', '0');
+  });
+
+  it('rejects malformed values on POST and PATCH without persisting', async () => {
+    const before = DatabaseService.getInstance().getGlobalSettings().notification_dispatch_retries;
+
+    for (const value of [null, true, false, '', ' ', '1.5', '-1', 4, '4', 'abc']) {
+      const post = await request(app)
+        .post('/api/settings')
+        .set('Cookie', adminCookie)
+        .send({ key: 'notification_dispatch_retries', value });
+      expect(post.status).toBe(400);
+
+      const patch = await request(app)
+        .patch('/api/settings')
+        .set('Cookie', adminCookie)
+        .send({ notification_dispatch_retries: value });
+      expect(patch.status).toBe(400);
+    }
+
+    expect(DatabaseService.getInstance().getGlobalSettings().notification_dispatch_retries).toBe(before);
+  });
+
+  it('rejects a non-admin write with 403', async () => {
+    const res = await request(app)
+      .post('/api/settings')
+      .set('Cookie', viewerCookie)
+      .send({ key: 'notification_dispatch_retries', value: '1' });
+    expect(res.status).toBe(403);
+  });
+});
+
 describe('env_block_deploy_on_missing_required setting', () => {
   it('seeds to "0" (opt-in) in a fresh database', () => {
     expect(DatabaseService.getInstance().getGlobalSettings().env_block_deploy_on_missing_required).toBe('0');