Stop swallowing alert config persistence errors

Refs #1341

UpdateAlertConfig used to log SaveAlertConfig failures and still tell
the client "saved successfully", leaving the in-memory state with the
new override but the on-disk file untouched. On the next config reload
or process restart, the override silently vanished and the user saw
their threshold "revert" with no surfaced error. Return HTTP 500 with
the persistence error so the frontend can show a real save-failed
toast instead of false confidence.

internal/api/alerts.go

@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -199,10 +200,15 @@ func (h *AlertHandlers) UpdateAlertConfig(w http.ResponseWriter, r *http.Request
 	)
 	notificationMgr.SetNotifyOnResolve(updatedConfig.Schedule.NotifyOnResolve)
 
-	// Save to persistent storage
+	// Save to persistent storage. Failure here used to be swallowed (logged
+	// and reported "success"), which led to the in-memory state diverging
+	// from what was actually persisted. On the next restart or config
+	// reload, the override silently vanished. Surface the error so the
+	// client can show a real save-failed signal.
 	if err := h.getMonitor(r.Context()).GetConfigPersistence().SaveAlertConfig(updatedConfig); err != nil {
-		// Log error but don't fail the request
 		log.Error().Err(err).Msg("Failed to save alert configuration")
+		http.Error(w, fmt.Sprintf("Failed to save alert configuration: %v", err), http.StatusInternalServerError)
+		return
 	}
 
 	if err := utils.WriteJSONResponse(w, map[string]interface{}{

internal/api/alerts_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
@@ -167,6 +168,43 @@ func TestUpdateAlertConfig(t *testing.T) {
 	assert.False(t, notificationMgr.IsEnabled())
 }
 
+// Regression for #1341: persistence failures used to be silently swallowed
+// (logged but reported as "saved successfully"), so a user setting a 50%
+// per-pool override would see the value stick in memory and revert on
+// reload, with no clue why. Surface the error so the frontend can show a
+// real save-failed signal.
+func TestUpdateAlertConfig_PersistenceFailureSurfacesAsError(t *testing.T) {
+	mockMonitor := new(MockAlertMonitor)
+	mockManager := new(MockAlertManager)
+	mockPersist := new(MockConfigPersistence)
+	notificationMgr := notifications.NewNotificationManager("")
+	defer notificationMgr.Stop()
+
+	mockMonitor.On("GetAlertManager").Return(mockManager)
+	mockMonitor.On("GetConfigPersistence").Return(mockPersist)
+	mockMonitor.On("GetNotificationManager").Return(notificationMgr)
+
+	h := NewAlertHandlers(nil, mockMonitor, nil)
+
+	cfg := alerts.AlertConfig{Enabled: true, ActivationState: alerts.ActivationPending}
+	mockManager.On("UpdateConfig", testifymock.Anything).Return()
+	mockManager.On("GetConfig").Return(cfg)
+	mockPersist.On("SaveAlertConfig", testifymock.Anything).Return(errors.New("permission denied"))
+
+	body, _ := json.Marshal(cfg)
+	req := httptest.NewRequest("POST", "/api/alerts/config", bytes.NewReader(body))
+	w := httptest.NewRecorder()
+
+	h.UpdateAlertConfig(w, req)
+
+	if w.Code != http.StatusInternalServerError {
+		t.Fatalf("status = %d, want 500 on persistence failure", w.Code)
+	}
+	if !strings.Contains(w.Body.String(), "permission denied") {
+		t.Fatalf("response body should expose persistence error, got %q", w.Body.String())
+	}
+}
+
 func TestActivateAlerts_EnablesNotificationManager(t *testing.T) {
 	mockMonitor := new(MockAlertMonitor)
 	mockManager := new(MockAlertManager)