diff --git a/backend/src/__tests__/notification-apprise.test.ts b/backend/src/__tests__/notification-apprise.test.ts index 503846dd..c6e27da1 100644 --- a/backend/src/__tests__/notification-apprise.test.ts +++ b/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, }), }, })); diff --git a/backend/src/__tests__/notification-retries.test.ts b/backend/src/__tests__/notification-retries.test.ts new file mode 100644 index 00000000..9eebbbaf --- /dev/null +++ b/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 = {}) { + 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; + + 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); + }); + }); +}); diff --git a/backend/src/__tests__/notification-routing.test.ts b/backend/src/__tests__/notification-routing.test.ts index 275fbc6d..1bb7dc6a 100644 --- a/backend/src/__tests__/notification-routing.test.ts +++ b/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') ); }); diff --git a/backend/src/__tests__/notification-suppression.test.ts b/backend/src/__tests__/notification-suppression.test.ts index fade91e1..5c03d0da 100644 --- a/backend/src/__tests__/notification-suppression.test.ts +++ b/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, }), }, })); diff --git a/backend/src/__tests__/settings-routes.test.ts b/backend/src/__tests__/settings-routes.test.ts index 2afd30ac..83c11dc6 100644 --- a/backend/src/__tests__/settings-routes.test.ts +++ b/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'); diff --git a/backend/src/helpers/notificationDispatchRetries.ts b/backend/src/helpers/notificationDispatchRetries.ts new file mode 100644 index 00000000..550fc5cd --- /dev/null +++ b/backend/src/helpers/notificationDispatchRetries.ts @@ -0,0 +1,16 @@ +/** + * Strict parser for notification_dispatch_retries (extra attempts, 0..3). + * Accepts JSON number integers or single-digit strings "0".."3" only. + * Rejects null, booleans, empty/whitespace, decimals, and out-of-range values. + */ +export function parseNotificationDispatchRetries(raw: unknown): number | null { + if (typeof raw === 'number') { + if (!Number.isInteger(raw) || raw < 0 || raw > 3) return null; + return raw; + } + if (typeof raw === 'string') { + if (!/^[0-3]$/.test(raw)) return null; + return Number(raw); + } + return null; +} diff --git a/backend/src/routes/settings.ts b/backend/src/routes/settings.ts index 7919ebfc..782b13c2 100644 --- a/backend/src/routes/settings.ts +++ b/backend/src/routes/settings.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { DatabaseService } from '../services/DatabaseService'; import { authMiddleware } from '../middleware/auth'; import { requireAdmin, requirePaid } from '../middleware/tierGates'; +import { parseNotificationDispatchRetries } from '../helpers/notificationDispatchRetries'; // Strict allowlist of keys readable and writable via the generic settings // API. This is the single source of truth for what the endpoint exposes: @@ -34,6 +35,7 @@ const ALLOWED_SETTING_KEYS = new Set([ 'env_block_deploy_on_missing_required', 'auto_create_missing_external_networks', 'image_update_sidebar_indicators', + 'notification_dispatch_retries', ]); // Keys whose write requires a paid license, not just an admin role. @@ -66,6 +68,15 @@ const SettingsPatchSchema = z.object({ env_block_deploy_on_missing_required: z.enum(['0', '1']), auto_create_missing_external_networks: z.enum(['0', '1']), image_update_sidebar_indicators: z.enum(['0', '1']), + // Strict: do not use bare z.coerce.number() (null/false/'' become 0; true becomes 1). + notification_dispatch_retries: z.unknown().superRefine((v, ctx) => { + if (parseNotificationDispatchRetries(v) === null) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Must be an integer from 0 to 3', + }); + } + }).transform((v) => String(parseNotificationDispatchRetries(v)!)), }).partial(); export const settingsRouter = Router(); diff --git a/backend/src/services/DatabaseService.ts b/backend/src/services/DatabaseService.ts index 973c032d..5f7cce45 100644 --- a/backend/src/services/DatabaseService.ts +++ b/backend/src/services/DatabaseService.ts @@ -1826,6 +1826,7 @@ export class DatabaseService { stmt.run('image_update_check_mode', 'interval'); stmt.run('image_update_check_cron', ''); stmt.run('image_update_sidebar_indicators', '1'); + stmt.run('notification_dispatch_retries', '0'); stmt.run('env_block_deploy_on_missing_required', '0'); stmt.run('auto_create_missing_external_networks', '0'); diff --git a/backend/src/services/NotificationService.ts b/backend/src/services/NotificationService.ts index d1ea2542..b1f139ac 100644 --- a/backend/src/services/NotificationService.ts +++ b/backend/src/services/NotificationService.ts @@ -19,6 +19,7 @@ import { parseStoredAppriseConfig, validateNotificationChannel, } from '../helpers/notificationChannels'; +import { parseNotificationDispatchRetries } from '../helpers/notificationDispatchRetries'; export type NotificationCategory = | 'deploy_success' @@ -71,6 +72,13 @@ export const ALL_SUPPRESSIBLE_CATEGORIES: readonly NotificationCategory[] = [ /** Webhook timeout: 10 seconds per external dispatch call. */ const WEBHOOK_TIMEOUT_MS = 10_000; +/** Fixed delay between retryable delivery attempts (extra attempts only). */ +const RETRY_DELAY_MS_DEFAULT = 1_000; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + /** Valid notification channel types for defense-in-depth validation. */ const ALLOWED_CHANNEL_TYPES = new Set(['discord', 'slack', 'webhook', 'apprise']); @@ -84,6 +92,8 @@ export class NotificationService { private static instance: NotificationService; private dbService: DatabaseService; private readonly subscribers = new Set(); + /** Overridable in tests so retry loops need not wait a real second. */ + private static retryDelayMs = RETRY_DELAY_MS_DEFAULT; private constructor() { this.dbService = DatabaseService.getInstance(); @@ -96,6 +106,11 @@ export class NotificationService { return NotificationService.instance; } + /** @internal Test-only: set the inter-attempt delay (production uses 1000). */ + public static setRetryDelayMsForTests(ms: number): void { + NotificationService.retryDelayMs = ms; + } + /** * Register a WebSocket as a live-notification subscriber. Returns an * unsubscribe function the caller should invoke on `'close'` / `'error'` @@ -256,6 +271,9 @@ export class NotificationService { return; } + // Resolve retry extras once for this dispatch (shared by all destinations). + const retries = this.resolveDispatchRetries(); + // 3. Check notification routing rules โ€” always evaluated, matchers compose AND const errors: string[] = []; @@ -264,7 +282,7 @@ export class NotificationService { if (isDebugEnabled()) console.log(`[Notify:diag] Matched ${matched.length} route(s) for stack "${sanitizeForLog(stackName ?? '(none)')}", category="${sanitizeForLog(category)}"`); await Promise.allSettled( matched.map(route => - this.sendToChannel(route.channel_type, route.channel_url, level, sanitized, route.config) + this.sendWithRetries(route.channel_type, route.channel_url, level, sanitized, route.config, retries) .then(() => { if (isDebugEnabled()) console.log(`[Notify:diag] Dispatched ${level} via route "${sanitizeForLog(route.name)}" (${route.channel_type})`); }) @@ -288,7 +306,7 @@ export class NotificationService { if (isDebugEnabled()) console.log(`[Notify:diag] Falling back to ${agents.length} global agent(s)`); await Promise.allSettled( agents.map(agent => - this.sendToChannel(agent.type, agent.url, level, sanitized, agent.config) + this.sendWithRetries(agent.type, agent.url, level, sanitized, agent.config, retries) .then(() => { if (isDebugEnabled()) console.log(`[Notify:diag] Dispatched ${level} via global agent (${agent.type})`); }) @@ -304,6 +322,25 @@ export class NotificationService { } } + /** + * Read notification_dispatch_retries once. Missing, malformed, out-of-range, + * or thrown settings reads fall back to 0 so the initial send still happens. + */ + private resolveDispatchRetries(): number { + try { + const raw = this.dbService.getGlobalSettings().notification_dispatch_retries; + const parsed = parseNotificationDispatchRetries(raw); + if (parsed === null) { + console.warn('[Notify] Invalid notification_dispatch_retries; using 0'); + return 0; + } + return parsed; + } catch (err) { + console.warn('[Notify] Failed to read notification_dispatch_retries; using 0:', err); + return 0; + } + } + /** Persist dispatch errors to the notification record for user visibility. */ private recordDispatchErrors(notificationId: number, errors: string[]) { if (errors.length > 0) { @@ -315,6 +352,43 @@ export class NotificationService { } } + /** + * Deliver with up to `retries` extra attempts after the first try. + * Waits a fixed 1s between attempts only when a retryable failure leaves attempts remaining. + */ + private async sendWithRetries( + type: string, + url: string, + level: 'info' | 'warning' | 'error', + message: string, + config: string | null | undefined, + retries: number, + ): Promise { + const totalAttempts = 1 + retries; + let lastError: NotificationDeliveryError | undefined; + for (let attempt = 0; attempt < totalAttempts; attempt++) { + try { + await this.sendToChannel(type, url, level, message, config); + return; + } catch (error) { + const deliveryError = error instanceof NotificationDeliveryError + ? error + : new NotificationDeliveryError( + getErrorMessage(error, 'Notification delivery failed'), + null, + false, + ); + lastError = deliveryError; + const attemptsRemain = attempt < totalAttempts - 1; + if (!deliveryError.retryable || !attemptsRemain) { + throw deliveryError; + } + await sleep(NotificationService.retryDelayMs); + } + } + throw lastError ?? new NotificationDeliveryError('Notification delivery failed', null, false); + } + private async sendToChannel(type: string, url: string, level: 'info' | 'warning' | 'error', message: string, config?: string | null): Promise { if (type === 'discord') { await this.sendDiscordWebhook(url, level, message); @@ -329,7 +403,7 @@ export class NotificationService { } await this.sendAppriseNotify(url, level, message, parsed); } else { - throw new Error(`Unsupported channel type: ${type}`); + throw new NotificationDeliveryError(`Unsupported channel type: ${type}`, null, false); } } @@ -338,7 +412,8 @@ export class NotificationService { const validation = validateNotificationChannel(type, url, config); if (validation) throw new Error(`URL ${validation}`); const stored = type === 'apprise' ? normalizeAppriseStoredJson(url, config) : (config == null ? null : JSON.stringify(config)); - await this.sendToChannel(type, url, 'info', '๐Ÿ”Œ Test Notification from Sencho!', stored); + const retries = this.resolveDispatchRetries(); + await this.sendWithRetries(type, url, 'info', '๐Ÿ”Œ Test Notification from Sencho!', stored, retries); } private async sendAppriseNotify( @@ -393,15 +468,28 @@ export class NotificationService { }] }; - const response = await fetch(url, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload), - signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS), - }); + try { + const response = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS), + }); - if (!response.ok) { - throw new Error(`Discord Webhook responded with ${response.status}`); + if (response.status >= 400 && response.status < 500) { + throw new NotificationDeliveryError(`Discord webhook responded with HTTP ${response.status}`, response.status, false); + } + if (!response.ok) { + throw new NotificationDeliveryError(`Discord webhook responded with HTTP ${response.status}`, response.status, true); + } + } catch (error) { + if (error instanceof NotificationDeliveryError) throw error; + const aborted = error instanceof Error && (error.name === 'AbortError' || error.name === 'TimeoutError'); + throw new NotificationDeliveryError( + aborted ? 'Discord webhook request timed out' : 'Discord webhook request failed', + null, + true, + ); } } @@ -416,15 +504,28 @@ export class NotificationService { text: `${emojiMap[level]} *Sencho Alert [${level.toUpperCase()}]*\n${message}` }; - const response = await fetch(url, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload), - signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS), - }); + try { + const response = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS), + }); - if (!response.ok) { - throw new Error(`Slack Webhook responded with ${response.status}`); + if (response.status >= 400 && response.status < 500) { + throw new NotificationDeliveryError(`Slack webhook responded with HTTP ${response.status}`, response.status, false); + } + if (!response.ok) { + throw new NotificationDeliveryError(`Slack webhook responded with HTTP ${response.status}`, response.status, true); + } + } catch (error) { + if (error instanceof NotificationDeliveryError) throw error; + const aborted = error instanceof Error && (error.name === 'AbortError' || error.name === 'TimeoutError'); + throw new NotificationDeliveryError( + aborted ? 'Slack webhook request timed out' : 'Slack webhook request failed', + null, + true, + ); } } @@ -436,15 +537,28 @@ export class NotificationService { source: 'sencho' }; - const response = await fetch(url, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload), - signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS), - }); + try { + const response = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS), + }); - if (!response.ok) { - throw new Error(`Custom Webhook responded with ${response.status}`); + if (response.status >= 400 && response.status < 500) { + throw new NotificationDeliveryError(`Custom webhook responded with HTTP ${response.status}`, response.status, false); + } + if (!response.ok) { + throw new NotificationDeliveryError(`Custom webhook responded with HTTP ${response.status}`, response.status, true); + } + } catch (error) { + if (error instanceof NotificationDeliveryError) throw error; + const aborted = error instanceof Error && (error.name === 'AbortError' || error.name === 'TimeoutError'); + throw new NotificationDeliveryError( + aborted ? 'Custom webhook request timed out' : 'Custom webhook request failed', + null, + true, + ); } } } diff --git a/docs/features/alerts-notifications.mdx b/docs/features/alerts-notifications.mdx index 4987b5a9..ff612474 100644 --- a/docs/features/alerts-notifications.mdx +++ b/docs/features/alerts-notifications.mdx @@ -7,13 +7,15 @@ description: Threshold and event alerts for your fleet, dispatched to Discord, S Sencho watches each node it manages for container crashes, host pressure, scheduled-task results, and update availability, then surfaces every signal in two places: the in-app notification bell at the top of the shell and an external channel you configure. This page covers everything from configuring channels to writing per-stack threshold rules, routing alerts to dedicated channels with routing rules, and tuning retention. - Settings ยท Notifications ยท Channels panel with Discord, Slack, Webhook, and Apprise tabs, CHANNELS 1/4 in the masthead, the Apprise tab active with Enabled on, a redacted Apprise endpoint, an empty Tags field, and Test disabled beside Save. + Settings ยท Notifications ยท Channels panel with NODE Local in the header, a Delivery retries row showing Extra attempts 0 and a Save retries button, Discord Slack Webhook and Apprise tabs with Apprise selected, Enabled off, an empty Apprise endpoint placeholder, and Test beside Save. ## Notification channels Open **Settings ยท Notifications ยท Channels** to configure Discord, Slack, custom webhook, and Apprise channels. Each channel is per-node, so switching the active node via the node picker reloads the panel against that node's stored settings. The masthead carries a `CHANNELS` stat showing how many of the four slots are enabled. +Above the channel tabs, **Delivery retries** sets how many extra in-process attempts (0 to 3) Sencho makes after a transient delivery failure on that node. The default is `0` (single-shot). Extra attempts wait a fixed one second between tries. Admin role is required to change the value. + Each Discord, Slack, and Webhook tab carries an **Enabled** toggle, a **Webhook URL** input (HTTPS only), and **Test** / **Save**. The Apprise tab uses an **Apprise endpoint** instead: keyed `/notify/{key}` shows optional **Tags**; stateless `/notify` shows **Destination URLs**. The kicker on each tab toggles between `enabled` and `off` so you can see at a glance which slots are wired up. @@ -51,9 +53,9 @@ Sencho sends the alert title, body, severity, and either tags or destination URL ### Test sends and delivery semantics -The **Test** button on each tab dispatches the literal message `๐Ÿ”Œ Test Notification from Sencho!` at level `info` through the same path a real alert would take. Test sends require the admin role; the server returns 403 if an operator or viewer submits one. +The **Test** button on each tab dispatches the literal message `๐Ÿ”Œ Test Notification from Sencho!` at level `info` through the same path a real alert would take, including the node's delivery-retries setting. Test sends require the admin role; the server returns 403 if an operator or viewer submits one. -Each dispatch is a single-shot HTTP POST with a 10-second `AbortSignal.timeout`. There is no retry queue. If your endpoint is down at the moment of the dispatch, the alert is recorded in the bell with an internal `dispatch_error` field set and is not redelivered. +Each delivery attempt is an HTTP POST with a 10-second `AbortSignal.timeout`. By default (`Delivery retries` = 0) Sencho makes one attempt. You can allow up to three extra in-process attempts with a fixed one-second delay between them. Retries apply only to classified transient failures (for example HTTP 5xx or network timeouts). Client errors such as HTTP 4xx and Apprise HTTP 204 are not retried. There is no durable retry queue: if the process exits mid-dispatch, remaining attempts are not persisted. Delivery is at-least-once under ambiguous timeouts or connection resets, so a receiver that accepted a request whose response was lost can receive a duplicate. If every attempt fails, the alert remains in the bell with `dispatch_error` set. ## Notification Routing @@ -421,7 +423,7 @@ The **Host Alerts** panel carries the **Host thresholds** rows (CPU limit, RAM l | Per-stack alert rule evaluation | 30 seconds | | Image update poll | Configurable (default every 2 hours), with a 2-minute startup delay and a 2-minute cooldown on manual refresh | | Sencho version check | Monitor evaluation every 30 seconds; shared version cache refreshes every 30 minutes when published or every 3 minutes while publish is pending | -| Notification fanout to channels | Single shot per dispatch, 10-second timeout, no retries | +| Notification fanout to channels | One attempt by default; optional 0-3 extra in-process attempts with a fixed 1s delay; 10-second timeout per attempt; no durable queue | | Bell live updates | Pushed live over the notifications WebSocket per node | | Bell safety-net reconcile | 60 seconds | | Crash dedup window per container | 60 minutes | diff --git a/docs/images/alerts-notifications/notifications-settings.png b/docs/images/alerts-notifications/notifications-settings.png index 85004f3a..cc44884b 100644 Binary files a/docs/images/alerts-notifications/notifications-settings.png and b/docs/images/alerts-notifications/notifications-settings.png differ diff --git a/docs/reference/settings.mdx b/docs/reference/settings.mdx index f95d1641..5e7e460e 100644 --- a/docs/reference/settings.mdx +++ b/docs/reference/settings.mdx @@ -380,6 +380,8 @@ Quick reference: Configure external destinations for alert notifications. Four agent types are available on separate tabs: **Discord**, **Slack**, **Webhook**, and **Apprise**. The masthead publishes a **CHANNELS** pill showing how many agents are enabled (for example, `2/4`). +**Delivery retries** (admin-only) sets how many extra in-process attempts (0 to 3, default 0) this node makes after a transient channel failure, with a fixed one-second delay between attempts. There is no durable queue; ambiguous network failures can produce duplicate notifications. + For each agent: | Field | Description | diff --git a/frontend/src/components/settings/NotificationsSection.tsx b/frontend/src/components/settings/NotificationsSection.tsx index 652ddbd2..edcc3c2f 100644 --- a/frontend/src/components/settings/NotificationsSection.tsx +++ b/frontend/src/components/settings/NotificationsSection.tsx @@ -7,13 +7,17 @@ import { TogglePill } from '@/components/ui/toggle-pill'; import { toast } from '@/components/ui/toast-store'; import { apiFetch } from '@/lib/api'; import { useNodes } from '@/context/NodeContext'; +import { useAuth } from '@/context/AuthContext'; import { RefreshCw } from 'lucide-react'; import type { Agent } from './types'; +import { DEFAULT_SETTINGS } from './types'; import { SettingsSection } from './SettingsSection'; import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; +import { NumberChip } from './SystemControls'; import { classifyAppriseEndpoint, isKeyedAppriseEndpoint, isStatelessAppriseEndpoint } from '@/lib/appriseEndpoint'; +import { parseNotificationDispatchRetries } from '@/lib/notificationDispatchRetries'; type ChannelType = 'discord' | 'slack' | 'webhook' | 'apprise'; @@ -38,8 +42,20 @@ function hasStoredAppriseAgent(agent: Agent): boolean { return Boolean(agent.config?.mode) || agent.url.includes(''); } -export function NotificationsSection() { +function clampRetryExtras(raw: string): string { + const n = Math.trunc(Number(raw)); + if (!Number.isFinite(n)) return DEFAULT_SETTINGS.notification_dispatch_retries!; + return String(Math.max(0, Math.min(3, n))); +} + +interface NotificationsSectionProps { + onDirtyChange?: (dirty: boolean) => void; +} + +export function NotificationsSection({ onDirtyChange }: NotificationsSectionProps) { const { activeNode } = useNodes(); + const { isAdmin } = useAuth(); + const readOnly = !isAdmin; const activeNodeIdRef = useRef(activeNode?.id); useEffect(() => { activeNodeIdRef.current = activeNode?.id; }, [activeNode?.id]); @@ -50,6 +66,22 @@ export function NotificationsSection() { const [appriseUrlDirty, setAppriseUrlDirty] = useState(false); const [appriseConfigDirty, setAppriseConfigDirty] = useState(false); + const [retries, setRetries] = useState(DEFAULT_SETTINGS.notification_dispatch_retries!); + const [savedRetries, setSavedRetries] = useState(DEFAULT_SETTINGS.notification_dispatch_retries!); + const [isSavingRetries, setIsSavingRetries] = useState(false); + // idle: node-switch reset before first successful load; ready: trusted saved value; error: GET failed. + const [retriesLoadState, setRetriesLoadState] = useState<'idle' | 'loading' | 'ready' | 'error'>('idle'); + const [hasLoadedRetries, setHasLoadedRetries] = useState(false); + const [retriesNeedsRepair, setRetriesNeedsRepair] = useState(false); + const retriesFetchGenRef = useRef(0); + const retriesMutationGenRef = useRef(0); + const retriesSaveGenRef = useRef(0); + const retriesDirty = hasLoadedRetries && (retries !== savedRetries || retriesNeedsRepair); + + useEffect(() => { + onDirtyChange?.(retriesDirty); + }, [retriesDirty, onDirtyChange]); + const fetchAgents = async () => { const requestNodeId = activeNode?.id; try { @@ -73,11 +105,98 @@ export function NotificationsSection() { } }; + const fetchRetries = async () => { + const requestNodeId = activeNode?.id; + const fetchGen = ++retriesFetchGenRef.current; + const mutationAtStart = retriesMutationGenRef.current; + setRetriesLoadState('loading'); + const isCurrent = () => ( + activeNodeIdRef.current === requestNodeId + && fetchGen === retriesFetchGenRef.current + && retriesMutationGenRef.current === mutationAtStart + ); + const restoreAfterStale = () => { + if (activeNodeIdRef.current !== requestNodeId) return; + // A newer fetch owns loading; do not fight it. + if (fetchGen !== retriesFetchGenRef.current) return; + // A newer edit/save owns the value; leave ready so edited UI stays interactive. + if (retriesMutationGenRef.current !== mutationAtStart) { + setRetriesLoadState('ready'); + } + }; + try { + const res = await apiFetch('/settings', { + nodeId: typeof requestNodeId === 'number' ? requestNodeId : undefined, + }); + if (!isCurrent()) { + restoreAfterStale(); + return; + } + if (!res.ok) { + setRetriesLoadState('error'); + return; + } + const data: Record = await res.json(); + if (!isCurrent()) { + restoreAfterStale(); + return; + } + // Missing key: same as seed/runtime default. Malformed stored values must NOT + // clamp into a false "saved" policy (backend falls back to 0 without accepting 1.5/9). + const raw = data.notification_dispatch_retries; + if (raw === undefined || raw === null || raw === '') { + const next = DEFAULT_SETTINGS.notification_dispatch_retries!; + setRetries(next); + setSavedRetries(next); + setRetriesNeedsRepair(false); + setRetriesLoadState('ready'); + setHasLoadedRetries(true); + return; + } + const parsed = parseNotificationDispatchRetries(raw); + if (parsed === null) { + const next = DEFAULT_SETTINGS.notification_dispatch_retries!; + setRetries(next); + setSavedRetries(next); + setRetriesNeedsRepair(true); + setRetriesLoadState('error'); + setHasLoadedRetries(true); + return; + } + const next = String(parsed); + setRetries(next); + setSavedRetries(next); + setRetriesNeedsRepair(false); + setRetriesLoadState('ready'); + setHasLoadedRetries(true); + } catch (e) { + console.error('Failed to fetch notification retry setting', e); + if (!isCurrent()) { + restoreAfterStale(); + return; + } + setRetriesLoadState('error'); + } + }; + useEffect(() => { + // Reset local channel/retry state when the active node changes so a prior + // node's values cannot flash while the replacement fetches settle. + retriesFetchGenRef.current += 1; + retriesMutationGenRef.current += 1; + retriesSaveGenRef.current += 1; + // eslint-disable-next-line react-hooks/set-state-in-effect -- intentional node-switch reset setAgents(emptyAgents()); setAppriseUrlDirty(false); setAppriseConfigDirty(false); + setRetries(DEFAULT_SETTINGS.notification_dispatch_retries!); + setSavedRetries(DEFAULT_SETTINGS.notification_dispatch_retries!); + setRetriesLoadState('idle'); + setHasLoadedRetries(false); + setRetriesNeedsRepair(false); + setIsSavingRetries(false); void fetchAgents(); + void fetchRetries(); }, [activeNode?.id]); // eslint-disable-line react-hooks/exhaustive-deps const enabledCount = Object.values(agents).filter(a => a.enabled).length; @@ -87,8 +206,63 @@ export function NotificationsSection() { value: `${enabledCount}/4`, tone: enabledCount > 0 ? 'value' : 'subtitle', }, + ...(retriesDirty + ? [{ label: 'EDITED', value: 'retries', tone: 'warn' as const }] + : []), ]); + const saveRetries = async () => { + const requestNodeId = activeNode?.id; + const submitted = clampRetryExtras(retries); + const mutationAtStart = retriesMutationGenRef.current; + const saveGen = ++retriesSaveGenRef.current; + setIsSavingRetries(true); + try { + const res = await apiFetch('/settings', { + method: 'PATCH', + nodeId: typeof requestNodeId === 'number' ? requestNodeId : undefined, + body: JSON.stringify({ notification_dispatch_retries: submitted }), + }); + if (activeNodeIdRef.current !== requestNodeId) return; + if (retriesMutationGenRef.current !== mutationAtStart) return; + if (!res.ok) { + const err = await res.json().catch(() => ({})); + toast.error(err?.error || err?.message || 'Something went wrong.'); + return; + } + // Invalidate in-flight GETs so a late load cannot overwrite this save. + retriesFetchGenRef.current += 1; + retriesMutationGenRef.current += 1; + setRetries(submitted); + setSavedRetries(submitted); + setRetriesLoadState('ready'); + setHasLoadedRetries(true); + setRetriesNeedsRepair(false); + toast.success('Delivery retries saved.'); + } catch (e: unknown) { + if (activeNodeIdRef.current !== requestNodeId) return; + if (retriesMutationGenRef.current !== mutationAtStart) return; + toast.error((e as Error)?.message || 'Network error.'); + } finally { + // Own the spinner by save generation, not value mutation (success bumps mutation). + if (activeNodeIdRef.current === requestNodeId && saveGen === retriesSaveGenRef.current) { + setIsSavingRetries(false); + } + } + }; + + const retriesKicker = + retriesNeedsRepair && retries === savedRetries + ? 'error' + : retriesDirty + ? 'edited' + : retriesLoadState === 'error' + ? 'error' + : retriesLoadState === 'loading' || retriesLoadState === 'idle' + ? 'loading' + : 'saved'; + const retriesControlsDisabled = readOnly || !hasLoadedRetries; + const handleAgentChange = (type: string, field: keyof Agent, value: Agent[keyof Agent]) => { setAgents(prev => ({ ...prev, @@ -283,6 +457,57 @@ export function NotificationsSection() { return (
+
+ + + { + retriesMutationGenRef.current += 1; + setRetries(clampRetryExtras(v)); + }} + suffix="extra" + min={0} + max={3} + step={1} + disabled={retriesControlsDisabled} + /> + + + {(hasLoadedRetries || retriesLoadState === 'error') && ( + + )} + void saveRetries()} + disabled={retriesControlsDisabled || !retriesDirty || isSavingRetries} + > + {isSavingRetries ? ( + <> + + Saving + + ) : ( + 'Save retries' + )} + + + +
setNotifTab(v as ChannelType)} className="w-full"> diff --git a/frontend/src/components/settings/SettingsSectionContent.tsx b/frontend/src/components/settings/SettingsSectionContent.tsx index fc769562..5f8f22a3 100644 --- a/frontend/src/components/settings/SettingsSectionContent.tsx +++ b/frontend/src/components/settings/SettingsSectionContent.tsx @@ -96,7 +96,7 @@ function renderSection( case 'docker-storage': return onDirtyChange('docker-storage', d)} />; case 'image-updates': return ; case 'fleet-mesh': return onDirtyChange('fleet-mesh', d)} />; - case 'notifications': return ; + case 'notifications': return onDirtyChange('notifications', d)} />; case 'notification-routing': return ; case 'notification-suppression': return ( ({ vi.mock('@/context/NodeContext', () => ({ useNodes: () => ({ activeNode: nodeState.activeNode }), })); +const authState = { isAdmin: true }; +vi.mock('@/context/AuthContext', () => ({ + useAuth: () => authState, +})); vi.mock('../MastheadStatsContext', () => ({ useMastheadStats: (stats: MastheadMetadataItem[] | null) => { masthead.last = stats; @@ -64,11 +68,18 @@ describe('NotificationsSection', () => { mockedFetch.mockReset(); masthead.last = null; nodeState.activeNode = { id: 1 }; - mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => { + authState.isAdmin = true; + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string; nodeId?: number }) => { if (url === '/agents' && !opts?.method) return agentsResponse(); if (url === '/agents' && opts?.method === 'POST') { return { ok: true, json: async () => ({}) }; } + if (url === '/settings' && !opts?.method) { + return { ok: true, json: async () => ({ notification_dispatch_retries: '0' }) }; + } + if (url === '/settings' && opts?.method === 'PATCH') { + return { ok: true, json: async () => ({ success: true }) }; + } return { ok: true, json: async () => ([]) }; }); }); @@ -321,6 +332,9 @@ describe('NotificationsSection', () => { if (nodeState.activeNode.id === 1) return agentsResponse([REDACTED_APPRISE]); return agentsResponse([]); } + if (url === '/settings' && !opts?.method) { + return { ok: true, json: async () => ({ notification_dispatch_retries: '0' }) }; + } return { ok: true, json: async () => ({}) }; }); @@ -355,6 +369,9 @@ describe('NotificationsSection', () => { } return agentsResponse([]); } + if (url === '/settings' && !opts?.method) { + return { ok: true, json: async () => ({ notification_dispatch_retries: '0' }) }; + } return { ok: true, json: async () => ({}) }; }); @@ -376,4 +393,310 @@ describe('NotificationsSection', () => { expect(screen.getByLabelText(/Apprise endpoint/i)).toHaveValue(''); }); + it('preserves CHANNELS masthead and loads retries with explicit nodeId', async () => { + render(); + await waitFor(() => expect(masthead.last?.[0]).toMatchObject({ label: 'CHANNELS', value: '1/4' })); + await waitFor(() => + expect(mockedFetch.mock.calls.some( + ([url, opts]) => url === '/settings' && (opts as { nodeId?: number })?.nodeId === 1, + )).toBe(true), + ); + expect(screen.getByText('Delivery retries')).toBeInTheDocument(); + expect(screen.getByText('0')).toBeInTheDocument(); + }); + + it('PATCHes only notification_dispatch_retries when saving retries', async () => { + render(); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + const chipButton = screen.getByRole('button', { name: /0\s*extra/i }); + await userEvent.click(chipButton); + const input = screen.getByRole('spinbutton'); + await userEvent.clear(input); + await userEvent.type(input, '2'); + await userEvent.keyboard('{Enter}'); + await userEvent.click(screen.getByRole('button', { name: 'Save retries' })); + await waitFor(() => { + const patch = mockedFetch.mock.calls.find( + ([url, opts]) => url === '/settings' && (opts as { method?: string })?.method === 'PATCH', + ); + expect(patch).toBeTruthy(); + expect(JSON.parse((patch![1] as { body: string }).body)).toEqual({ notification_dispatch_retries: '2' }); + expect((patch![1] as { nodeId?: number }).nodeId).toBe(1); + }); + await waitFor(() => expect(screen.getByRole('button', { name: 'Save retries' })).toBeInTheDocument()); + expect(screen.queryByRole('button', { name: /Saving/i })).toBeNull(); + expect(findAgentsPost()).toBeUndefined(); + }); + + it('disables delivery retries controls for non-admins', async () => { + authState.isAdmin = false; + render(); + await waitFor(() => expect(screen.getByText('Delivery retries')).toBeInTheDocument()); + expect(screen.getByRole('button', { name: 'Save retries' })).toBeDisabled(); + }); + + it('ignores a stale settings body after a node switch', async () => { + let releaseNode1Settings: (() => void) | undefined; + const gate = new Promise((resolve) => { releaseNode1Settings = resolve; }); + + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string; nodeId?: number | null }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + const targetId = opts?.nodeId ?? nodeState.activeNode.id; + if (targetId === 1) { + return { + ok: true, + json: async () => { + await gate; + return { notification_dispatch_retries: '3' }; + }, + }; + } + return { ok: true, json: async () => ({ notification_dispatch_retries: '0' }) }; + } + return { ok: true, json: async () => ({}) }; + }); + + const { rerender } = render(); + nodeState.activeNode = { id: 2 }; + rerender(); + await waitFor(() => + expect(mockedFetch.mock.calls.some( + ([url, opts]) => url === '/settings' && (opts as { nodeId?: number })?.nodeId === 2, + )).toBe(true), + ); + releaseNode1Settings?.(); + await new Promise((r) => setTimeout(r, 40)); + expect(screen.getByRole('button', { name: /0\s*extra/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /3\s*extra/i })).toBeNull(); + }); + + it('does not present default 0 as saved when settings GET fails', async () => { + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + return { ok: false, status: 500, json: async () => ({ error: 'boom' }) }; + } + return { ok: true, json: async () => ({}) }; + }); + + render(); + await waitFor(() => expect(screen.getByText('error')).toBeInTheDocument()); + expect(screen.queryByText('saved')).toBeNull(); + expect(screen.getByRole('button', { name: 'Save retries' })).toBeDisabled(); + expect(screen.getByRole('button', { name: /0\s*extra/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Retry load' })).toBeInTheDocument(); + }); + + it('disables retry controls until the settings GET succeeds', async () => { + let releaseGet: (() => void) | undefined; + const gate = new Promise((resolve) => { releaseGet = resolve; }); + + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + return { + ok: true, + json: async () => { + await gate; + return { notification_dispatch_retries: '3' }; + }, + }; + } + return { ok: true, json: async () => ({}) }; + }); + + render(); + await waitFor(() => expect(screen.getByText('loading')).toBeInTheDocument()); + expect(screen.queryByText('saved')).toBeNull(); + expect(screen.getByRole('button', { name: /0\s*extra/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Save retries' })).toBeDisabled(); + + releaseGet?.(); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + expect(screen.getByRole('button', { name: /3\s*extra/i })).not.toBeDisabled(); + }); + + it('keeps PATCH result when a deferred Reload GET returns stale data', async () => { + let releaseStale: (() => void) | undefined; + const staleGate = new Promise((resolve) => { releaseStale = resolve; }); + let settingsGetCount = 0; + + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + settingsGetCount += 1; + if (settingsGetCount === 1) { + return { ok: true, json: async () => ({ notification_dispatch_retries: '0' }) }; + } + return { + ok: true, + json: async () => { + await staleGate; + return { notification_dispatch_retries: '0' }; + }, + }; + } + if (url === '/settings' && opts?.method === 'PATCH') { + return { ok: true, json: async () => ({ success: true }) }; + } + return { ok: true, json: async () => ({}) }; + }); + + render(); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + + // Start a soft reload, then edit+save while that GET is still in flight. + await userEvent.click(screen.getByRole('button', { name: 'Reload' })); + await waitFor(() => expect(mockedFetch.mock.calls.filter( + ([url, opts]) => url === '/settings' && !(opts as { method?: string })?.method, + ).length).toBeGreaterThan(1)); + + await userEvent.click(screen.getByRole('button', { name: /0\s*extra/i })); + const input = screen.getByRole('spinbutton'); + await userEvent.clear(input); + await userEvent.type(input, '2'); + await userEvent.keyboard('{Enter}'); + await userEvent.click(screen.getByRole('button', { name: 'Save retries' })); + await waitFor(() => expect(screen.getByRole('button', { name: /2\s*extra/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + + releaseStale?.(); + await new Promise((r) => setTimeout(r, 40)); + expect(screen.getByRole('button', { name: /2\s*extra/i })).toBeInTheDocument(); + expect(screen.getByText('saved')).toBeInTheDocument(); + }); + + + it('clears Saving after edit-during-save supersedes the PATCH apply', async () => { + let releasePatch: (() => void) | undefined; + const patchGate = new Promise((resolve) => { releasePatch = resolve; }); + + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + return { ok: true, json: async () => ({ notification_dispatch_retries: '0' }) }; + } + if (url === '/settings' && opts?.method === 'PATCH') { + await patchGate; + return { ok: true, json: async () => ({ success: true }) }; + } + return { ok: true, json: async () => ({}) }; + }); + + render(); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + + await userEvent.click(screen.getByRole('button', { name: /0\s*extra/i })); + let input = screen.getByRole('spinbutton'); + await userEvent.clear(input); + await userEvent.type(input, '2'); + await userEvent.keyboard('{Enter}'); + await userEvent.click(screen.getByRole('button', { name: 'Save retries' })); + await waitFor(() => expect(screen.getByRole('button', { name: /Saving/i })).toBeInTheDocument()); + + // Edit while the PATCH is in flight (bumps mutation gen). + await userEvent.click(screen.getByRole('button', { name: /2\s*extra/i })); + input = screen.getByRole('spinbutton'); + await userEvent.clear(input); + await userEvent.type(input, '3'); + await userEvent.keyboard('{Enter}'); + + releasePatch?.(); + await waitFor(() => expect(screen.getByRole('button', { name: 'Save retries' })).toBeInTheDocument()); + expect(screen.queryByRole('button', { name: /Saving/i })).toBeNull(); + expect(screen.getByRole('button', { name: /3\s*extra/i })).toBeInTheDocument(); + expect(screen.getByText('edited')).toBeInTheDocument(); + }); + + it('clears Saving when the active node changes during an in-flight PATCH', async () => { + let releasePatch: (() => void) | undefined; + const patchGate = new Promise((resolve) => { releasePatch = resolve; }); + + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string; nodeId?: number }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + const id = opts?.nodeId ?? nodeState.activeNode.id; + return { ok: true, json: async () => ({ notification_dispatch_retries: id === 1 ? '0' : '1' }) }; + } + if (url === '/settings' && opts?.method === 'PATCH') { + await patchGate; + return { ok: true, json: async () => ({ success: true }) }; + } + return { ok: true, json: async () => ({}) }; + }); + + const { rerender } = render(); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + + await userEvent.click(screen.getByRole('button', { name: /0\s*extra/i })); + const input = screen.getByRole('spinbutton'); + await userEvent.clear(input); + await userEvent.type(input, '2'); + await userEvent.keyboard('{Enter}'); + await userEvent.click(screen.getByRole('button', { name: 'Save retries' })); + await waitFor(() => expect(screen.getByRole('button', { name: /Saving/i })).toBeInTheDocument()); + + nodeState.activeNode = { id: 2 }; + rerender(); + await waitFor(() => expect(screen.queryByRole('button', { name: /Saving/i })).toBeNull()); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + expect(screen.getByRole('button', { name: 'Save retries' })).toBeInTheDocument(); + + releasePatch?.(); + await new Promise((r) => setTimeout(r, 40)); + expect(screen.queryByRole('button', { name: /Saving/i })).toBeNull(); + expect(screen.getByRole('button', { name: /1\s*extra/i })).toBeInTheDocument(); + }); + + + + it('treats invalid stored notification_dispatch_retries as error, not clamped saved', async () => { + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + return { ok: true, json: async () => ({ notification_dispatch_retries: '9' }) }; + } + if (url === '/settings' && opts?.method === 'PATCH') { + return { ok: true, json: async () => ({ success: true }) }; + } + return { ok: true, json: async () => ({}) }; + }); + + render(); + await waitFor(() => expect(screen.getByText('error')).toBeInTheDocument()); + expect(screen.queryByText('saved')).toBeNull(); + expect(screen.getByText(/Stored delivery retries value is invalid/i)).toBeInTheDocument(); + // Chip may show 0 as a draft, but Save must be enabled so the operator can repair. + expect(screen.getByRole('button', { name: 'Save retries' })).not.toBeDisabled(); + + await userEvent.click(screen.getByRole('button', { name: 'Save retries' })); + await waitFor(() => { + const patch = mockedFetch.mock.calls.find( + ([url, opts]) => url === '/settings' && (opts as { method?: string })?.method === 'PATCH', + ); + expect(patch).toBeTruthy(); + expect(JSON.parse((patch![1] as { body: string }).body)).toEqual({ notification_dispatch_retries: '0' }); + }); + await waitFor(() => expect(screen.getByText('saved')).toBeInTheDocument()); + expect(screen.getByRole('button', { name: /0\s*extra/i })).toBeInTheDocument(); + }); + + it('treats decimal stored notification_dispatch_retries as invalid, not truncated saved', async () => { + mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => { + if (url === '/agents' && !opts?.method) return agentsResponse([]); + if (url === '/settings' && !opts?.method) { + return { ok: true, json: async () => ({ notification_dispatch_retries: '1.5' }) }; + } + return { ok: true, json: async () => ({}) }; + }); + + render(); + await waitFor(() => expect(screen.getByText('error')).toBeInTheDocument()); + expect(screen.queryByText('saved')).toBeNull(); + expect(screen.queryByRole('button', { name: /1\s*extra/i })).toBeNull(); + expect(screen.getByText(/Stored delivery retries value is invalid/i)).toBeInTheDocument(); + }); + + }); diff --git a/frontend/src/components/settings/registry.ts b/frontend/src/components/settings/registry.ts index 557a9b3c..96fa7cb6 100644 --- a/frontend/src/components/settings/registry.ts +++ b/frontend/src/components/settings/registry.ts @@ -205,7 +205,7 @@ export const SETTINGS_ITEMS: readonly SettingsItemMeta[] = [ group: 'notifications', label: 'Channels', description: 'Discord, Slack, Apprise, and custom webhook destinations for Sencho alerts.', - keywords: ['discord', 'slack', 'apprise', 'webhook', 'channels', 'destinations', 'alerts'], + keywords: ['discord', 'slack', 'apprise', 'webhook', 'channels', 'destinations', 'alerts', 'retry', 'retries'], tier: null, scope: 'node', }, diff --git a/frontend/src/components/settings/types.ts b/frontend/src/components/settings/types.ts index d03eb8f9..891facb3 100644 --- a/frontend/src/components/settings/types.ts +++ b/frontend/src/components/settings/types.ts @@ -22,6 +22,7 @@ export interface PatchableSettings { env_block_deploy_on_missing_required?: '0' | '1'; auto_create_missing_external_networks?: '0' | '1'; image_update_sidebar_indicators?: '0' | '1'; + notification_dispatch_retries?: string; } export const DEFAULT_SETTINGS: PatchableSettings = { @@ -48,6 +49,7 @@ export const DEFAULT_SETTINGS: PatchableSettings = { env_block_deploy_on_missing_required: '0', auto_create_missing_external_networks: '0', image_update_sidebar_indicators: '1', + notification_dispatch_retries: '0', }; export type SectionId = diff --git a/frontend/src/lib/notificationDispatchRetries.test.ts b/frontend/src/lib/notificationDispatchRetries.test.ts new file mode 100644 index 00000000..c6506703 --- /dev/null +++ b/frontend/src/lib/notificationDispatchRetries.test.ts @@ -0,0 +1,21 @@ + +import { describe, it, expect } from 'vitest'; +import { parseNotificationDispatchRetries } from './notificationDispatchRetries'; + +describe('parseNotificationDispatchRetries', () => { + it('accepts integers and digit strings 0-3', () => { + expect(parseNotificationDispatchRetries(0)).toBe(0); + expect(parseNotificationDispatchRetries(3)).toBe(3); + expect(parseNotificationDispatchRetries('2')).toBe(2); + }); + + it('rejects out-of-range, decimals, and non-canonical strings', () => { + expect(parseNotificationDispatchRetries(9)).toBeNull(); + expect(parseNotificationDispatchRetries('9')).toBeNull(); + expect(parseNotificationDispatchRetries(1.5)).toBeNull(); + expect(parseNotificationDispatchRetries('1.5')).toBeNull(); + expect(parseNotificationDispatchRetries(' 1')).toBeNull(); + expect(parseNotificationDispatchRetries(null)).toBeNull(); + expect(parseNotificationDispatchRetries(true)).toBeNull(); + }); +}); diff --git a/frontend/src/lib/notificationDispatchRetries.ts b/frontend/src/lib/notificationDispatchRetries.ts new file mode 100644 index 00000000..32097fa0 --- /dev/null +++ b/frontend/src/lib/notificationDispatchRetries.ts @@ -0,0 +1,16 @@ +/** + * Strict parser for notification_dispatch_retries (extra attempts, 0..3). + * Must stay aligned with backend/src/helpers/notificationDispatchRetries.ts: + * accepts JSON number integers or single-digit strings "0".."3" only. + */ +export function parseNotificationDispatchRetries(raw: unknown): number | null { + if (typeof raw === 'number') { + if (!Number.isInteger(raw) || raw < 0 || raw > 3) return null; + return raw; + } + if (typeof raw === 'string') { + if (!/^[0-3]$/.test(raw)) return null; + return Number(raw); + } + return null; +}