Merge pull request #52 from AnsoCode/fix/alerts-notifications-overhaul

fix(alerts): overhaul alerts & notifications system for local and remote nodes

CHANGELOG.md

@@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- **Fixed:** `StackAlertSheet` now fetches notification agent status from the active node on open and displays a contextual banner: green checkmark with active channel names when agents are enabled, amber warning with a link to Settings → Notifications when none are configured, and a blue info callout on remote nodes explaining that alerts are evaluated and dispatched by that remote Sencho instance.
+- **Fixed:** `SettingsModal` Notifications tab is no longer hidden when a remote node is active — users can now configure Discord/Slack/Webhook channels directly on any remote node. The section header shows the remote node name and a "Remote" badge with a tooltip explaining that channels are saved on the remote instance.
+- **Fixed:** `StackAlertSheet` form error handling now surfaces the actual server error message (`err.error`) instead of a generic "Failed to add alert rule." string; console error logging added for all failure paths to aid debugging.
+- **Fixed:** `POST /api/alerts` now validates the request body with a Zod schema — rejects unknown metric/operator values, negative thresholds, and missing required fields with a structured 400 response instead of passing raw input to SQLite.
+- **Fixed:** Remote node HTTP proxy body forwarding — replaced the manual `proxyReq.write(JSON.stringify(req.body))` approach (which raced against `http-proxy`'s `process.nextTick(proxyReq.end)` and lost, causing "write after end" and the request to hang) with a conditional JSON body parser that skips `express.json()` entirely for remote-targeted `/api/` requests. The raw `IncomingMessage` stream is left unconsumed so `http-proxy`'s `req.pipe(proxyReq)` can forward it intact. This fixes the "Add Rule spins forever on remote nodes" bug.
+- **Fixed:** WebSocket notification reconnect in `EditorLayout` upgraded to exponential backoff (1 s → 2 s → 4 s → 8 s → 16 s → 30 s max) instead of a flat 5-second retry; `ws.onerror` now logs the event rather than silently calling `close()`; cleanup correctly guards against closing a WebSocket that is already in CLOSING/CLOSED state, eliminating the "WebSocket is closed before the connection is established" console error on React StrictMode double-mount.
 - **Security:** Host Console and container exec WebSocket endpoints now reject `node_proxy` scoped JWT tokens with HTTP 403 — machine-to-machine proxy credentials can no longer be used to open interactive terminals.
 - **Security:** `stackParam` query parameter on `/api/system/host-console` is now validated against `path.resolve` + `startsWith(baseDir)` to prevent directory traversal when setting the PTY working directory.
 - **Security:** `HostTerminalService` no longer forwards the full `process.env` to spawned PTY shells; `JWT_SECRET`, `AUTH_PASSWORD`, `AUTH_PASSWORD_HASH`, and `DATABASE_URL` are stripped before the shell is spawned.

backend/src/index.ts

@@ -68,7 +68,32 @@ app.use(cors({
   origin: true,
   credentials: true,
 }));
-app.use(express.json());
+// Conditionally parse JSON bodies. Remote proxy requests must NOT have their body
+// consumed here: express.json() drains the IncomingMessage stream into req.body
+// and http-proxy then pipes an already-ended stream to the remote server.
+// When Node.js pipes an ended readable it calls process.nextTick(dest.end()),
+// which fires *before* the proxyReq socket event, so any attempt to write the
+// body inside the proxyReq handler results in "write after end" and the request
+// hangs. Solution: skip JSON parsing for remote-targeted /api/ requests so the
+// raw stream flows through the proxy intact.
+app.use((req: Request, res: Response, next: NextFunction): void => {
+  const nodeIdHeader = req.headers['x-node-id'];
+  if (nodeIdHeader) {
+    const nodeId = parseInt(nodeIdHeader as string, 10);
+    const node = NodeRegistry.getInstance().getNode(nodeId);
+    if (
+      node?.type === 'remote' &&
+      req.path.startsWith('/api/') &&
+      !req.path.startsWith('/api/auth/') &&
+      !req.path.startsWith('/api/nodes')
+    ) {
+      // Preserve body stream for proxy piping
+      next();
+      return;
+    }
+  }
+  express.json()(req, res, next);
+});
 app.use(cookieParser());
 
 // Node Context Middleware
@@ -370,6 +395,10 @@ const remoteNodeProxy = createProxyMiddleware<Request, Response>({
         const newQs = params.toString();
         proxyReq.path = pathname + (newQs ? `?${newQs}` : '');
       }
+      // Body forwarding: the conditional json parser (see top of file) skips
+      // parsing for remote requests, so req's raw stream is intact and
+      // http-proxy's req.pipe(proxyReq) forwards the body automatically.
+      // No manual body rewriting needed here.
     },
     error: (err, _req, proxyRes) => {
       console.error('[Proxy] Remote node error:', (err as Error).message);
@@ -1392,12 +1421,26 @@ app.get('/api/alerts', async (req: Request, res: Response) => {
   }
 });
 
+const AlertCreateSchema = z.object({
+  stack_name: z.string().min(1).max(255),
+  metric: z.enum(['cpu_percent', 'memory_percent', 'memory_mb', 'net_rx', 'net_tx', 'restart_count']),
+  operator: z.enum(['>', '>=', '<', '<=', '==']),
+  threshold: z.number().min(0),
+  duration_mins: z.coerce.number().int().min(0).max(1440),
+  cooldown_mins: z.coerce.number().int().min(0).max(10080),
+});
+
 app.post('/api/alerts', async (req: Request, res: Response) => {
+  const parsed = AlertCreateSchema.safeParse(req.body);
+  if (!parsed.success) {
+    res.status(400).json({ error: 'Invalid alert data', details: parsed.error.flatten().fieldErrors });
+    return;
+  }
   try {
-    const alert = req.body;
+    DatabaseService.getInstance().addStackAlert(parsed.data);
-    DatabaseService.getInstance().addStackAlert(alert);
     res.json({ success: true });
   } catch (error) {
+    console.error('Failed to add alert:', error);
     res.status(500).json({ error: 'Failed to add alert' });
   }
 });

frontend/src/components/EditorLayout.tsx

@@ -194,10 +194,22 @@ export default function EditorLayout() {
     let ws: WebSocket | null = null;
     let reconnectTimer: ReturnType<typeof setTimeout> | null = null;
     let isMounted = true;
+    let retryCount = 0;
+    const MAX_RETRY_DELAY_MS = 30000;
 
     const connect = () => {
+      if (!isMounted) return;
       ws = new WebSocket(`${wsBase}/ws/notifications`);
 
+      ws.onopen = () => {
+        if (!isMounted) {
+          // Component unmounted while the handshake was in-flight (React StrictMode double-mount)
+          ws?.close();
+          return;
+        }
+        retryCount = 0; // Reset backoff on successful connect
+      };
+
       ws.onmessage = (event) => {
         try {
           const msg = JSON.parse(event.data);
@@ -209,14 +221,18 @@ export default function EditorLayout() {
         }
       };
 
-      ws.onclose = () => {
+      ws.onclose = (event) => {
-        if (isMounted) {
+        if (!isMounted) return;
-          reconnectTimer = setTimeout(connect, 5000);
+        // Exponential backoff: 1s, 2s, 4s, 8s, 16s, 30s max
-        }
+        const delay = Math.min(1000 * Math.pow(2, retryCount), MAX_RETRY_DELAY_MS);
+        retryCount++;
+        console.debug(`[WS notifications] closed (code=${event.code}), reconnecting in ${delay}ms (attempt ${retryCount})`);
+        reconnectTimer = setTimeout(connect, delay);
       };
 
-      ws.onerror = () => {
+      ws.onerror = (event) => {
-        ws?.close();
+        // onerror always fires before onclose - log it and let onclose handle reconnect
+        console.warn('[WS notifications] error event', event);
       };
     };
 
@@ -225,7 +241,12 @@ export default function EditorLayout() {
     return () => {
       isMounted = false;
       if (reconnectTimer) clearTimeout(reconnectTimer);
-      ws?.close();
+      // Only close an already-open connection. If still CONNECTING, let onopen
+      // detect isMounted=false and close then — avoids the browser warning
+      // "WebSocket is closed before the connection is established".
+      if (ws && ws.readyState === WebSocket.OPEN) {
+        ws.close();
+      }
     };
   }, []); // eslint-disable-line react-hooks/exhaustive-deps
 

frontend/src/components/SettingsModal.tsx

@@ -3,7 +3,10 @@ import { motion } from 'motion/react';
 import {
     Dialog,
     DialogContent,
+    DialogTitle,
+    DialogDescription,
 } from '@/components/ui/dialog';
+import { VisuallyHidden } from '@radix-ui/react-visually-hidden';
 import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs';
 import { Input } from '@/components/ui/input';
 import { Switch } from '@/components/ui/switch';
@@ -123,7 +126,7 @@ export function SettingsModal({ isOpen, onClose, theme, setTheme }: SettingsModa
             fetchAgents();
             fetchSettings();
         }
-    }, [isOpen]); // eslint-disable-line react-hooks/exhaustive-deps
+    }, [isOpen, activeNode?.id]); // eslint-disable-line react-hooks/exhaustive-deps
 
     const fetchAgents = async () => {
         try {
@@ -392,6 +395,8 @@ export function SettingsModal({ isOpen, onClose, theme, setTheme }: SettingsModa
     return (
         <Dialog open={isOpen} onOpenChange={(open) => !open && onClose()}>
             <DialogContent className="sm:max-w-[900px] h-[650px] flex p-0 font-sans shadow-lg bg-background border-border overflow-hidden gap-0">
+                <VisuallyHidden><DialogTitle>Settings Hub</DialogTitle></VisuallyHidden>
+                <VisuallyHidden><DialogDescription>Configure Sencho settings</DialogDescription></VisuallyHidden>
                 {/* Sidebar */}
                 <div className="w-[200px] bg-muted/20 border-r border-border flex flex-col p-4 shrink-0">
                     <div className="font-semibold text-lg mb-1 text-foreground tracking-tight">Settings Hub</div>
@@ -410,9 +415,7 @@ export function SettingsModal({ isOpen, onClose, theme, setTheme }: SettingsModa
                             label="System Limits"
                             showDot={hasSystemChanges}
                         />
-                        {!isRemote && (
+                        <NavButton section="notifications" icon={<Bell className="w-4 h-4 mr-2" />} label="Notifications" />
-                            <NavButton section="notifications" icon={<Bell className="w-4 h-4 mr-2" />} label="Notifications" />
-                        )}
                         {!isRemote && (
                             <NavButton section="appearance" icon={<Palette className="w-4 h-4 mr-2" />} label="Appearance" />
                         )}
@@ -573,9 +576,31 @@ export function SettingsModal({ isOpen, onClose, theme, setTheme }: SettingsModa
 
                     {activeSection === 'notifications' && (
                         <div className="space-y-6">
-                            <div>
+                            <div className="flex items-start justify-between pr-8">
-                                <h3 className="text-lg font-semibold tracking-tight">Notifications & Alerts</h3>
+                                <div>
-                                <p className="text-sm text-muted-foreground">Configure external integrations for crash alerts.</p>
+                                    <h3 className="text-lg font-semibold tracking-tight">Notifications & Alerts</h3>
+                                    <p className="text-sm text-muted-foreground">
+                                        {isRemote
+                                            ? <>Configuring notification channels on <span className="font-semibold text-foreground">{activeNode!.name}</span>. Alerts from this remote node will dispatch via these channels.</>
+                                            : 'Configure external integrations for crash alerts.'
+                                        }
+                                    </p>
+                                </div>
+                                {isRemote && (
+                                    <TooltipProvider>
+                                        <Tooltip>
+                                            <TooltipTrigger asChild>
+                                                <Badge variant="secondary" className="text-xs shrink-0 ml-2 mt-0.5 cursor-help">
+                                                    <Info className="w-3 h-3 mr-1" />
+                                                    Remote
+                                                </Badge>
+                                            </TooltipTrigger>
+                                            <TooltipContent side="bottom" className="max-w-[240px] text-center">
+                                                These channels are saved on the remote Sencho instance and used when it dispatches alerts.
+                                            </TooltipContent>
+                                        </Tooltip>
+                                    </TooltipProvider>
+                                )}
                             </div>
                             <Tabs value={notifTab} onValueChange={(v) => setNotifTab(v as 'discord' | 'slack' | 'webhook')} className="w-full">
                                 <TabsList className="w-full mb-4 grid grid-cols-3">

frontend/src/components/StackAlertSheet.tsx

@@ -11,9 +11,10 @@ import { Input } from '@/components/ui/input';
 import { Label } from '@/components/ui/label';
 import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select';
 import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip';
-import { Trash2, HelpCircle } from 'lucide-react';
+import { Trash2, HelpCircle, AlertTriangle, Info, CheckCircle2, Loader2 } from 'lucide-react';
 import { toast } from 'sonner';
 import { apiFetch } from '@/lib/api';
+import { useNodes } from '@/context/NodeContext';
 
 interface StackAlert {
     id?: number;
@@ -31,9 +32,23 @@ interface StackAlertSheetProps {
     stackName: string;
 }
 
+interface AgentStatus {
+    loading: boolean;
+    hasEnabled: boolean;
+    enabledTypes: string[];
+}
+
 export function StackAlertSheet({ isOpen, onClose, stackName }: StackAlertSheetProps) {
+    const { activeNode } = useNodes();
+    const isRemote = activeNode?.type === 'remote';
+
     const [alerts, setAlerts] = useState<StackAlert[]>([]);
     const [isLoading, setIsLoading] = useState(false);
+    const [agentStatus, setAgentStatus] = useState<AgentStatus>({
+        loading: false,
+        hasEnabled: false,
+        enabledTypes: [],
+    });
 
     // New Alert Form State
     const [metric, setMetric] = useState('cpu_percent');
@@ -45,18 +60,41 @@ export function StackAlertSheet({ isOpen, onClose, stackName }: StackAlertSheetP
     useEffect(() => {
         if (isOpen && stackName) {
             fetchAlerts();
+            fetchAgentStatus();
         }
-    }, [isOpen, stackName]);
+    }, [isOpen, stackName]); // eslint-disable-line react-hooks/exhaustive-deps
 
     const fetchAlerts = async () => {
         try {
-            const res = await apiFetch(`/alerts?stackName=${stackName}`);
+            const res = await apiFetch(`/alerts?stackName=${encodeURIComponent(stackName)}`);
             if (res.ok) {
                 const data = await res.json();
                 setAlerts(data);
             }
         } catch (e) {
-            console.error('Failed to fetch alerts', e);
+            console.error('[StackAlertSheet] Failed to fetch alerts', e);
+        }
+    };
+
+    const fetchAgentStatus = async () => {
+        setAgentStatus(prev => ({ ...prev, loading: true }));
+        try {
+            // Always fetch agents from the active node (proxied via x-node-id for remote)
+            const res = await apiFetch('/agents');
+            if (res.ok) {
+                const agents: Array<{ type: string; enabled: boolean }> = await res.json();
+                const enabled = agents.filter(a => a.enabled);
+                setAgentStatus({
+                    loading: false,
+                    hasEnabled: enabled.length > 0,
+                    enabledTypes: enabled.map(a => a.type),
+                });
+            } else {
+                setAgentStatus({ loading: false, hasEnabled: false, enabledTypes: [] });
+            }
+        } catch (e) {
+            console.error('[StackAlertSheet] Failed to fetch agent status', e);
+            setAgentStatus({ loading: false, hasEnabled: false, enabledTypes: [] });
         }
     };
 
@@ -73,23 +111,26 @@ export function StackAlertSheet({ isOpen, onClose, stackName }: StackAlertSheetP
             operator,
             threshold: parseFloat(threshold),
             duration_mins: parseInt(duration, 10),
-            cooldown_mins: parseInt(cooldown, 10)
+            cooldown_mins: parseInt(cooldown, 10),
         };
 
         try {
             const res = await apiFetch('/alerts', {
                 method: 'POST',
-                body: JSON.stringify(newAlert)
+                body: JSON.stringify(newAlert),
             });
             if (res.ok) {
                 toast.success('Alert rule added.');
                 setThreshold('');
                 fetchAlerts();
             } else {
-                toast.error('Failed to add alert rule.');
+                const err = await res.json().catch(() => ({}));
+                toast.error(err?.error || err?.message || 'Failed to add alert rule.');
+                console.error('[StackAlertSheet] addAlert failed:', err);
             }
         } catch (e) {
-            toast.error('Network error.');
+            console.error('[StackAlertSheet] addAlert threw:', e);
+            toast.error('Network error. Could not reach the node.');
         } finally {
             setIsLoading(false);
         }
@@ -103,10 +144,11 @@ export function StackAlertSheet({ isOpen, onClose, stackName }: StackAlertSheetP
                 toast.success('Alert rule deleted.');
                 fetchAlerts();
             } else {
-                toast.error('Failed to delete alert rule.');
+                const err = await res.json().catch(() => ({}));
+                toast.error(err?.error || 'Failed to delete alert rule.');
             }
         } catch (e) {
-            toast.error('Network error.');
+            toast.error('Network error. Could not reach the node.');
         } finally {
             setIsLoading(false);
         }
@@ -118,12 +160,81 @@ export function StackAlertSheet({ isOpen, onClose, stackName }: StackAlertSheetP
         memory_mb: 'Memory Usage (MB)',
         net_rx: 'Network In (MB)',
         net_tx: 'Network Out (MB)',
-        restart_count: 'Restart Count'
+        restart_count: 'Restart Count',
+    };
+
+    const agentTypeLabels: Record<string, string> = {
+        discord: 'Discord',
+        slack: 'Slack',
+        webhook: 'Webhook',
+    };
+
+    const renderAgentStatusBanner = () => {
+        if (agentStatus.loading) {
+            return (
+                <div className="flex items-center gap-2 p-3 rounded-lg bg-muted/50 border text-sm text-muted-foreground">
+                    <Loader2 className="h-4 w-4 animate-spin shrink-0" />
+                    <span>Checking notification channels…</span>
+                </div>
+            );
+        }
+
+        if (isRemote) {
+            return (
+                <div className="flex items-start gap-2 p-3 rounded-lg bg-blue-500/10 border border-blue-500/20 text-sm">
+                    <Info className="h-4 w-4 text-blue-500 shrink-0 mt-0.5" />
+                    <div className="space-y-0.5">
+                        <p className="font-medium text-blue-700 dark:text-blue-400">
+                            Remote node: <span className="font-semibold">{activeNode?.name}</span>
+                        </p>
+                        <p className="text-muted-foreground">
+                            Alert rules are stored and evaluated on this remote instance. Notifications are dispatched using that node's configured channels.
+                        </p>
+                        {!agentStatus.hasEnabled && (
+                            <p className="text-amber-600 dark:text-amber-400 font-medium mt-1">
+                                No notification channels are configured on this remote node. Open Settings → Notifications to configure them.
+                            </p>
+                        )}
+                        {agentStatus.hasEnabled && (
+                            <p className="text-green-600 dark:text-green-400 font-medium mt-1">
+                                Active channels: {agentStatus.enabledTypes.map(t => agentTypeLabels[t] ?? t).join(', ')}
+                            </p>
+                        )}
+                    </div>
+                </div>
+            );
+        }
+
+        if (!agentStatus.hasEnabled) {
+            return (
+                <div className="flex items-start gap-2 p-3 rounded-lg bg-amber-500/10 border border-amber-500/20 text-sm">
+                    <AlertTriangle className="h-4 w-4 text-amber-500 shrink-0 mt-0.5" />
+                    <div>
+                        <p className="font-medium text-amber-700 dark:text-amber-400">No notification channels configured</p>
+                        <p className="text-muted-foreground mt-0.5">
+                            Alert rules will be saved and evaluated, but no notifications will be dispatched. Configure Discord, Slack, or a webhook in{' '}
+                            <span className="font-medium">Settings → Notifications</span>.
+                        </p>
+                    </div>
+                </div>
+            );
+        }
+
+        return (
+            <div className="flex items-center gap-2 p-3 rounded-lg bg-green-500/10 border border-green-500/20 text-sm">
+                <CheckCircle2 className="h-4 w-4 text-green-500 shrink-0" />
+                <div>
+                    <p className="font-medium text-green-700 dark:text-green-400">
+                        Notifications active via {agentStatus.enabledTypes.map(t => agentTypeLabels[t] ?? t).join(', ')}
+                    </p>
+                </div>
+            </div>
+        );
     };
 
     return (
         <Sheet open={isOpen} onOpenChange={(open) => !open && onClose()}>
-            <SheetContent className="overflow-y-auto sm:max-w-[400px]">
+            <SheetContent className="overflow-y-auto sm:max-w-[420px]">
                 <SheetHeader>
                     <SheetTitle>Stack Alerts: {stackName}</SheetTitle>
                     <SheetDescription>
@@ -132,7 +243,10 @@ export function StackAlertSheet({ isOpen, onClose, stackName }: StackAlertSheetP
                 </SheetHeader>
 
                 <TooltipProvider>
-                    <div className="mt-6 space-y-6">
+                    <div className="mt-4 space-y-5">
+                        {/* Notification agent status banner */}
+                        {renderAgentStatusBanner()}
+
                         {/* List Existing Alerts */}
                         <div className="space-y-3">
                             <h4 className="text-sm font-semibold">Existing Rules</h4>
@@ -299,7 +413,11 @@ export function StackAlertSheet({ isOpen, onClose, stackName }: StackAlertSheetP
                             </div>
 
                             <Button className="w-full mt-2" onClick={addAlert} disabled={isLoading}>
-                                Add Rule
+                                {isLoading ? (
+                                    <><Loader2 className="h-4 w-4 mr-2 animate-spin" />Saving…</>
+                                ) : (
+                                    'Add Rule'
+                                )}
                             </Button>
                         </div>
                     </div>