From 80f8fcff6b0cc06398d4f280cde35595577f46ae Mon Sep 17 00:00:00 2001 From: Anki Date: Sun, 17 Aug 2025 18:09:13 +0530 Subject: [PATCH] refactor: enhance notification system with granular controls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Migrate legacy ntfyUrl configuration to structured ntfy object - Add individual enable/disable toggles for ntfy and web push notifications - Implement automatic config migration with backward compatibility - Add timeout and improved error handling for ntfy requests - Update UI with separate toggle switches for each notification type - Enhance error logging with non-fatal ntfy failures - Improve web push error handling and subscription cleanup 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/services/config-service.ts | 30 +++++++++- src/services/notification-service.ts | 57 +++++++++++++++---- src/services/web-push-service.ts | 16 ++++-- src/types/config.ts | 6 +- .../PreferencesModal/NotificationTab.tsx | 51 +++++++++++++++-- src/web/chat/types/index.ts | 8 ++- 6 files changed, 140 insertions(+), 28 deletions(-) diff --git a/src/services/config-service.ts b/src/services/config-service.ts index 6fca1158..e21ef984 100644 --- a/src/services/config-service.ts +++ b/src/services/config-service.ts @@ -135,11 +135,26 @@ export class ConfigService { throw new Error('Invalid JSON in configuration file'); } + // Migrate legacy ntfyUrl to new structure + let migrated = false; + if (fileConfig.interface?.notifications && 'ntfyUrl' in fileConfig.interface.notifications) { + const notifications = fileConfig.interface.notifications as any; + if (!notifications.ntfy) { + notifications.ntfy = { + enabled: false, // Default to false as requested + url: notifications.ntfyUrl + }; + } + delete notifications.ntfyUrl; + migrated = true; + this.logger.info('Migrated legacy ntfyUrl to new ntfy configuration structure'); + } + // Validate provided fields (strict for provided keys, allow missing) this.validateProvidedFields(fileConfig); // Merge with defaults for missing sections while preserving all existing fields (e.g., router) - let updated = false; + let updated = migrated; const merged: CUIConfig = { // Start with defaults ...DEFAULT_CONFIG, @@ -318,8 +333,17 @@ export class ConfigService { if (n && typeof n.enabled !== 'boolean') { throw new Error('Invalid config: interface.notifications.enabled must be a boolean'); } - if (n && n.ntfyUrl !== undefined && typeof n.ntfyUrl !== 'string') { - throw new Error('Invalid config: interface.notifications.ntfyUrl must be a string'); + // Validate ntfy settings + if (n && n.ntfy !== undefined) { + if (typeof n.ntfy !== 'object' || n.ntfy === null) { + throw new Error('Invalid config: interface.notifications.ntfy must be an object'); + } + if (n.ntfy.enabled !== undefined && typeof n.ntfy.enabled !== 'boolean') { + throw new Error('Invalid config: interface.notifications.ntfy.enabled must be a boolean'); + } + if (n.ntfy.url !== undefined && typeof n.ntfy.url !== 'string') { + throw new Error('Invalid config: interface.notifications.ntfy.url must be a string'); + } } } } diff --git a/src/services/notification-service.ts b/src/services/notification-service.ts index 298ee2d3..0ed98d5e 100644 --- a/src/services/notification-service.ts +++ b/src/services/notification-service.ts @@ -57,7 +57,12 @@ export class NotificationService { */ private async getNtfyUrl(): Promise { const config = this.configService.getConfig(); - return config.interface.notifications?.ntfyUrl || 'https://ntfy.sh'; + return config.interface.notifications?.ntfy?.url || 'https://ntfy.sh'; + } + + private async isNtfyEnabled(): Promise { + const config = this.configService.getConfig(); + return config.interface.notifications?.ntfy?.enabled ?? false; } /** @@ -90,8 +95,18 @@ export class NotificationService { permissionRequestId: request.id }; - // Send via ntfy - await this.sendNotification(ntfyUrl, topic, notification); + // Send via ntfy if enabled (best-effort, don't fail the whole operation) + if (await this.isNtfyEnabled()) { + try { + await this.sendNotification(ntfyUrl, topic, notification); + } catch (ntfyError) { + this.logger.warn('Ntfy notification failed (non-fatal)', { + error: (ntfyError as Error)?.message, + url: ntfyUrl, + topic + }); + } + } // Also broadcast via native web push (best-effort) try { @@ -152,8 +167,18 @@ export class NotificationService { streamingId }; - // Send via ntfy - await this.sendNotification(ntfyUrl, topic, notification); + // Send via ntfy if enabled (best-effort, don't fail the whole operation) + if (await this.isNtfyEnabled()) { + try { + await this.sendNotification(ntfyUrl, topic, notification); + } catch (ntfyError) { + this.logger.warn('Ntfy notification failed (non-fatal)', { + error: (ntfyError as Error)?.message, + url: ntfyUrl, + topic + }); + } + } // Also broadcast via native web push (best-effort) try { @@ -210,14 +235,22 @@ export class NotificationService { headers['X-CUI-PermissionRequestId'] = notification.permissionRequestId; } - const response = await fetch(url, { - method: 'POST', - headers, - body: notification.message - }); + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 5000); // 5 second timeout + + try { + const response = await fetch(url, { + method: 'POST', + headers, + body: notification.message, + signal: controller.signal + }); - if (!response.ok) { - throw new Error(`Ntfy returned ${response.status}: ${await response.text()}`); + if (!response.ok) { + throw new Error(`Ntfy returned ${response.status}: ${await response.text()}`); + } + } finally { + clearTimeout(timeout); } } } \ No newline at end of file diff --git a/src/services/web-push-service.ts b/src/services/web-push-service.ts index 51d74b02..8c9a6bd6 100644 --- a/src/services/web-push-service.ts +++ b/src/services/web-push-service.ts @@ -160,8 +160,10 @@ export class WebPushService { } getEnabled(): boolean { - const enabled = this.configService.getConfig().interface.notifications?.enabled ?? false; - return enabled; + const notifications = this.configService.getConfig().interface.notifications; + const globalEnabled = notifications?.enabled ?? false; + const webPushEnabled = notifications?.webPush?.enabled ?? true; // Default to true for backward compatibility + return globalEnabled && webPushEnabled; } getSubscriptionCount(): number { @@ -203,15 +205,19 @@ export class WebPushService { await webpush.sendNotification(sub, JSON.stringify(payload), { TTL: 60 }); this.upsertSeenStmt.run(new Date().toISOString(), 0, row.endpoint); sent += 1; - } catch (_err: unknown) { + } catch (err: unknown) { failed += 1; // 410 Gone or 404 Not Found => expire subscription - const status = undefined; + const status = (err as any)?.statusCode || (err as any)?.response?.statusCode; if (status === 404 || status === 410) { this.upsertSeenStmt.run(new Date().toISOString(), 1, row.endpoint); this.logger.info('Expired web push subscription removed', { endpoint: row.endpoint, status }); } else { - this.logger.error('Failed sending web push notification', { endpoint: row.endpoint, statusCode: status }); + this.logger.error('Failed sending web push notification', { + endpoint: row.endpoint, + statusCode: status, + error: err instanceof Error ? err.message : String(err) + }); } } }) diff --git a/src/types/config.ts b/src/types/config.ts index 66614f4b..c1886657 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -27,8 +27,12 @@ export interface InterfaceConfig { language: string; notifications?: { enabled: boolean; - ntfyUrl?: string; + ntfy?: { + enabled: boolean; + url?: string; + }; webPush?: { + enabled?: boolean; subject?: string; // e.g. mailto:you@example.com vapidPublicKey?: string; vapidPrivateKey?: string; diff --git a/src/web/chat/components/PreferencesModal/NotificationTab.tsx b/src/web/chat/components/PreferencesModal/NotificationTab.tsx index 45660f13..2cfddcf4 100644 --- a/src/web/chat/components/PreferencesModal/NotificationTab.tsx +++ b/src/web/chat/components/PreferencesModal/NotificationTab.tsx @@ -150,7 +150,24 @@ export function NotificationTab({ prefs, machineId, onUpdate }: Props) { {/* Ntfy Section */}
-

Ntfy

+
+

Ntfy

+ onUpdate({ + notifications: { + ...prefs.notifications, + enabled: prefs.notifications?.enabled || false, + ntfy: { + ...prefs.notifications?.ntfy, + enabled: checked + } + } + })} + disabled={!prefs.notifications?.enabled} + aria-label="Toggle ntfy notifications" + /> +

Ntfy works out of the box but opens notifications in the ntfy app. To receive push notifications, subscribe to the following ntfy topic:

@@ -171,15 +188,20 @@ export function NotificationTab({ prefs, machineId, onUpdate }: Props) { id="ntfy-url" type="url" className="bg-white dark:bg-neutral-900 border-neutral-200 dark:border-neutral-700 hover:border-neutral-300 dark:hover:border-neutral-600 focus:border-blue-500 dark:focus:border-blue-400" - value={prefs.notifications?.ntfyUrl || ''} + value={prefs.notifications?.ntfy?.url || ''} placeholder="https://ntfy.sh" onChange={(e) => onUpdate({ notifications: { ...prefs.notifications, enabled: prefs.notifications?.enabled || false, - ntfyUrl: e.target.value || undefined + ntfy: { + ...prefs.notifications?.ntfy, + enabled: prefs.notifications?.ntfy?.enabled || false, + url: e.target.value || undefined + } } })} + disabled={!prefs.notifications?.enabled || !prefs.notifications?.ntfy?.enabled} aria-label="Ntfy server URL" />
@@ -188,7 +210,24 @@ export function NotificationTab({ prefs, machineId, onUpdate }: Props) { {/* Web Push Section */}
-

Web Push

+
+

Web Push

+ onUpdate({ + notifications: { + ...prefs.notifications, + enabled: prefs.notifications?.enabled || false, + webPush: { + ...prefs.notifications?.webPush, + enabled: checked + } + } + })} + disabled={!prefs.notifications?.enabled} + aria-label="Toggle web push notifications" + /> +

Web Push requires HTTPS hosting. See setup instructions at{' '}