feat(mqtt): persist observer preferences as JSON

2026-09-26 21:07:56 +00:00 · 2026-08-14 09:09:05 -07:00
parent 40635a53c4
commit 75f3e446e0
27 changed files with 2131 additions and 254 deletions
@@ -16,6 +16,7 @@
 #include "AlertReporter.h"  // for alertReporterBannedChannelMatch[Hex]()
 #include "MQTTObserverValidation.h"  // pure input validators (host-testable)
 #include <Utils.h>
+#include <new>
 #ifdef ESP_PLATFORM
 #include <WiFi.h>
 #include <WiFiClientSecure.h>
@@ -109,6 +110,46 @@ static bool valueTooLong(const char* val, size_t bufsize, char* reply, const cha
  return false;
 }

+static bool isObserverPrefsSetCommand(const char* config) {
+  return strncmp(config, "snmp", 4) == 0 ||
+      strncmp(config, "radio.watchdog", 14) == 0 ||
+      strncmp(config, "mqtt", 4) == 0 ||
+      strncmp(config, "wifi.", 5) == 0 ||
+      strncmp(config, "timezone", 8) == 0 ||
+      strncmp(config, "alert", 5) == 0;
+}
+
+// Keep observer setters atomic from the caller's perspective. The live object
+// is restored if its JSON transaction fails, and the snapshot pointer is valid
+// only for this synchronous command dispatch.
+class ObserverPrefsRollbackScope {
+public:
+  ObserverPrefsRollbackScope(const MQTTPrefs** active_snapshot,
+                             const MQTTPrefs& live,
+                             bool capture)
+      : _active_snapshot(active_snapshot) {
+    *_active_snapshot = nullptr;
+    if (capture) {
+      _snapshot = new (std::nothrow) MQTTPrefs;
+      if (_snapshot != nullptr) {
+        memcpy(_snapshot, &live, sizeof(*_snapshot));
+        *_active_snapshot = _snapshot;
+      }
+    }
+  }
+
+  ~ObserverPrefsRollbackScope() {
+    *_active_snapshot = nullptr;
+    delete _snapshot;
+  }
+
+  bool available() const { return _snapshot != nullptr; }
+
+private:
+  const MQTTPrefs** _active_snapshot;
+  MQTTPrefs* _snapshot = nullptr;
+};
+
 static const char* getMQTTPresetNameByIndex(int index) {
  if (index < MQTT_PRESET_COUNT) return MQTT_PRESETS[index].name;
  if (index == MQTT_PRESET_COUNT) return MQTT_PRESET_CUSTOM;
@@ -161,17 +202,38 @@ static void formatMQTTPresetListReply(char* reply, size_t reply_size, int start)
 }
 #endif

+bool CommonCLI::persistObserverPrefs(char* reply) {
+#ifdef WITH_MQTT_BRIDGE
+  if (_callbacks->saveObserverPrefs()) return true;
+  if (_observer_prefs_rollback != nullptr) {
+    memcpy(&_mqtt_prefs, _observer_prefs_rollback, sizeof(_mqtt_prefs));
+  }
+  strcpy(reply, "Error: setting not persisted; change rolled back");
+  return false;
+#else
+  (void)reply;
+  return false;
+#endif
+}
+
 bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* config, char* reply) {
 #ifdef WITH_MQTT_BRIDGE
+  const bool needs_snapshot = isObserverPrefsSetCommand(config);
+  ObserverPrefsRollbackScope rollback_scope(
+      &_observer_prefs_rollback, _mqtt_prefs, needs_snapshot);
+  if (needs_snapshot && !rollback_scope.available()) {
+    strcpy(reply, "Error: insufficient memory to update observer setting");
+    return true;
+  }
  bool handled = true;
  if (memcmp(config, "snmp.community ", 15) == 0) {
    if (valueTooLong(&config[15], sizeof(_mqtt_prefs.snmp_community), reply, "snmp.community")) return true;
    StrHelper::strncpy(_mqtt_prefs.snmp_community, &config[15], sizeof(_mqtt_prefs.snmp_community));
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK - restart to apply");
  } else if (memcmp(config, "snmp ", 5) == 0) {
    _mqtt_prefs.snmp_enabled = memcmp(&config[5], "on", 2) == 0;
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK - restart to apply");
  } else if (memcmp(config, "radio.watchdog ", 15) == 0) {
    const char* val = &config[15];
@@ -189,7 +251,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
        strcpy(reply, "Error: radio.watchdog must be 0-120 minutes");
      } else {
        _mqtt_prefs.radio_watchdog_minutes = (uint8_t)mins;
-        savePrefs();
+        if (!persistObserverPrefs(reply)) return true;
        if (mins == 0) {
          strcpy(reply, "OK - radio watchdog disabled");
        } else {
@@ -200,13 +262,13 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
 #ifdef WITH_MQTT_BRIDGE
  } else if (strcmp(config, "mqtt.origin") == 0) {
    _mqtt_prefs.mqtt_origin[0] = '\0';
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.origin ", 12) == 0) {
    if (valueTooLong(&config[12], sizeof(_mqtt_prefs.mqtt_origin), reply, "origin")) return true;
    StrHelper::strncpy(_mqtt_prefs.mqtt_origin, &config[12], sizeof(_mqtt_prefs.mqtt_origin));
    StrHelper::stripSurroundingQuotes(_mqtt_prefs.mqtt_origin, sizeof(_mqtt_prefs.mqtt_origin));
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.iata ", 10) == 0) {
    const char* iata = &config[10];
@@ -215,7 +277,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      // Empty clears the region code (meshcore-topic publishing stays disabled
      // until one is set). This keeps the pre-existing "clear IATA" capability.
      _mqtt_prefs.mqtt_iata[0] = '\0';
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->restartBridge();
      strcpy(reply, "OK - IATA cleared");
    } else {
@@ -228,22 +290,22 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
        for (int i = 0; _mqtt_prefs.mqtt_iata[i]; i++) {
          _mqtt_prefs.mqtt_iata[i] = toupper(_mqtt_prefs.mqtt_iata[i]);
        }
-        savePrefs();
+        if (!persistObserverPrefs(reply)) return true;
        _callbacks->restartBridge();
        strcpy(reply, "OK");
      }
    }
  } else if (memcmp(config, "mqtt.status ", 12) == 0) {
    _mqtt_prefs.mqtt_status_enabled = memcmp(&config[12], "on", 2) == 0;
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.packets ", 13) == 0) {
    _mqtt_prefs.mqtt_packets_enabled = memcmp(&config[13], "on", 2) == 0;
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.raw ", 9) == 0) {
    _mqtt_prefs.mqtt_raw_enabled = memcmp(&config[9], "on", 2) == 0;
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.tx ", 8) == 0) {
    if (memcmp(&config[8], "advert", 6) == 0) {
@@ -251,17 +313,17 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    } else {
      _mqtt_prefs.mqtt_tx_enabled = memcmp(&config[8], "on", 2) == 0 ? 1 : 0;
    }
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.rx ", 8) == 0) {
    _mqtt_prefs.mqtt_rx_enabled = memcmp(&config[8], "on", 2) == 0 ? 1 : 0;
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.interval ", 14) == 0) {
    uint32_t minutes = _atoi(&config[14]);
    if (minutes >= 1 && minutes <= 60) {
      _mqtt_prefs.mqtt_status_interval = minutes * 60000;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->restartBridge();
      sprintf(reply, "OK - interval set to %u minutes (%lu ms), bridge restarted", minutes, (unsigned long)_mqtt_prefs.mqtt_status_interval);
    } else {
@@ -274,7 +336,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    uint32_t hours = _atoi(&config[24]);
    if (hours >= MQTT_NEIGHBORS_MIN_INTERVAL_HOURS && hours <= MQTT_NEIGHBORS_MAX_INTERVAL_HOURS) {
      _mqtt_prefs.mqtt_neighbors_interval = hours * 3600000UL;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      sprintf(reply, "OK - neighbors interval set to %u hours (%lu ms)", (unsigned)hours,
              (unsigned long)_mqtt_prefs.mqtt_neighbors_interval);
    } else {
@@ -284,7 +346,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    // The mesh loop reads this live, so no bridge restart is needed; enabling it
    // triggers a discovery on the next eligible loop pass.
    _mqtt_prefs.mqtt_neighbors_enabled = memcmp(&config[15], "on", 2) == 0;
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
 #elif defined(WITH_MQTT_BRIDGE)
  } else if (memcmp(config, "mqtt.neighbors.interval ", 24) == 0 ||
@@ -303,7 +365,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      } else {
        StrHelper::strncpy(_mqtt_prefs.mqtt_ntp_server, host, sizeof(_mqtt_prefs.mqtt_ntp_server));
      }
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
 #ifdef ESP_PLATFORM
      // Queue a sync on the MQTT task (Core 0) but do NOT block: this handler
      // runs on the Arduino loop task, shared with mesh/radio processing and the
@@ -325,12 +387,12 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
  } else if (memcmp(config, "wifi.ssid ", 10) == 0) {
    if (valueTooLong(&config[10], sizeof(_mqtt_prefs.wifi_ssid), reply, "wifi.ssid")) return true;
    StrHelper::strncpy(_mqtt_prefs.wifi_ssid, &config[10], sizeof(_mqtt_prefs.wifi_ssid));
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "wifi.pwd ", 9) == 0) {
    if (valueTooLong(&config[9], sizeof(_mqtt_prefs.wifi_password), reply, "wifi.pwd")) return true;
    StrHelper::strncpy(_mqtt_prefs.wifi_password, &config[9], sizeof(_mqtt_prefs.wifi_password));
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "wifi.powersave ", 15) == 0) {
    const char* value = &config[15];
@@ -350,7 +412,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      strcpy(reply, "Error: must be none, min, or max");
    } else {
      _mqtt_prefs.wifi_power_save = ps_value;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
 #ifdef ESP_PLATFORM
      if (WiFi.status() == WL_CONNECTED) {
        wifi_ps_type_t ps_mode = (ps_value == 1) ? WIFI_PS_NONE :
@@ -374,13 +436,13 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
  } else if (memcmp(config, "timezone ", 9) == 0) {
    if (valueTooLong(&config[9], sizeof(_mqtt_prefs.timezone_string), reply, "timezone")) return true;
    StrHelper::strncpy(_mqtt_prefs.timezone_string, &config[9], sizeof(_mqtt_prefs.timezone_string));
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
  } else if (memcmp(config, "timezone.offset ", 16) == 0) {
    int8_t offset = _atoi(&config[16]);
    if (offset >= -12 && offset <= 14) {
      _mqtt_prefs.timezone_offset = offset;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      strcpy(reply, "OK");
    } else {
      strcpy(reply, "Error: timezone offset must be between -12 and +14");
@@ -397,20 +459,14 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
          strcmp(preset_name, MQTT_PRESET_CUSTOM) == 0 ||
          strcmp(preset_name, MQTT_PRESET_NONE) == 0) {
        // Reject duplicate presets (except "none" and "custom")
-        int dup_slot = -1;
-        if (findMQTTPreset(preset_name) != nullptr) {
-          for (int s = 0; s < MAX_MQTT_SLOTS; s++) {
-            if (s != slot && strcmp(_mqtt_prefs.mqtt_slot_preset[s], preset_name) == 0) {
-              dup_slot = s;
-              break;
-            }
-          }
-        }
+        const int dup_slot = findMQTTPreset(preset_name) == nullptr ? -1 :
+            mqttAssignedPresetSlot(_mqtt_prefs.mqtt_slot_preset,
+                                   preset_name, slot);
        if (dup_slot >= 0) {
          sprintf(reply, "Error: preset '%s' is already assigned to slot %d", preset_name, dup_slot + 1);
        } else {
          StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], preset_name, sizeof(_mqtt_prefs.mqtt_slot_preset[slot]));
-          savePrefs();
+          if (!persistObserverPrefs(reply)) return true;
          _callbacks->restartBridgeSlot(slot);
          // Check if the slot has everything it needs to connect
          const MQTTPresetDef* p = findMQTTPreset(preset_name);
@@ -469,7 +525,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    } else if (memcmp(subcmd, "server ", 7) == 0) {
      if (valueTooLong(&subcmd[7], sizeof(_mqtt_prefs.mqtt_slot_host[slot]), reply, "server")) return true;
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_host[slot], &subcmd[7], sizeof(_mqtt_prefs.mqtt_slot_host[slot]));
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      // Reconfigure the slot so the new host reaches the live connection (other
      // custom-slot setters do the same; without it the change only applies on
      // the next reboot/bridge restart).
@@ -479,7 +535,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      int port = atoi(&subcmd[5]);
      if (port > 0 && port <= 65535) {
        _mqtt_prefs.mqtt_slot_port[slot] = port;
-        savePrefs();
+        if (!persistObserverPrefs(reply)) return true;
        _callbacks->restartBridgeSlot(slot);
        strcpy(reply, "OK");
      } else {
@@ -488,19 +544,19 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    } else if (memcmp(subcmd, "username ", 9) == 0) {
      if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_username[slot]), reply, "username")) return true;
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_username[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_username[slot]));
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->restartBridgeSlot(slot);
      strcpy(reply, "OK");
    } else if (memcmp(subcmd, "password ", 9) == 0) {
      if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_password[slot]), reply, "password")) return true;
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_password[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_password[slot]));
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->restartBridgeSlot(slot);
      strcpy(reply, "OK");
    } else if (memcmp(subcmd, "token ", 6) == 0) {
      if (valueTooLong(&subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_token[slot]), reply, "token")) return true;
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_token[slot], &subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_token[slot]));
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->restartBridgeSlot(slot);
      sprintf(reply, "OK - slot %d token set", slot + 1);
    } else if (memcmp(subcmd, "topic ", 6) == 0) {
@@ -510,14 +566,14 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
        return true;
      } else {
        StrHelper::strncpy(_mqtt_prefs.mqtt_slot_topic[slot], &subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_topic[slot]));
-        savePrefs();
+        if (!persistObserverPrefs(reply)) return true;
        _callbacks->restartBridgeSlot(slot);
        sprintf(reply, "OK - slot %d topic: %s", slot + 1, _mqtt_prefs.mqtt_slot_topic[slot]);
      }
    } else if (memcmp(subcmd, "audience ", 9) == 0) {
      if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_audience[slot]), reply, "audience")) return true;
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_audience[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_audience[slot]));
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->restartBridgeSlot(slot);
      if (_mqtt_prefs.mqtt_slot_audience[slot][0] != '\0') {
        sprintf(reply, "OK - slot %d JWT audience: %s", slot + 1, _mqtt_prefs.mqtt_slot_audience[slot]);
@@ -527,7 +583,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    } else if (memcmp(subcmd, "audience", 8) == 0 && subcmd[8] == '\0') {
      // "set mqttN.audience" with no value — clear the audience
      _mqtt_prefs.mqtt_slot_audience[slot][0] = '\0';
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->restartBridgeSlot(slot);
      sprintf(reply, "OK - slot %d JWT audience cleared (using username/password auth)", slot + 1);
    } else if (strcmp(subcmd, "filter") == 0 ||
@@ -539,20 +595,18 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
        strcpy(reply, "Error: filter must be all, none, or a CSV of types 0-15 / names (advert,txt_msg,...)");
      } else {
        _mqtt_prefs.mqtt_slot_packet_filter[slot] = filter_mask;
-        savePrefs();
+        if (!persistObserverPrefs(reply)) return true;
        char filter_text[MQTTPacketFilter::kFilterTextSize];
        MQTTPacketFilter::format(filter_mask, filter_text, sizeof(filter_text));
        snprintf(reply, 160, "OK - slot %d packet types: %s", slot + 1, filter_text);
-        // A non-default filter extends /mqtt_prefs past what pre-filter
-        // firmware can read (see MQTTPrefsCodec::payloadLenFor), so say when
-        // that cost buys nothing: slots beyond the runtime array are never
-        // published to on this board, the same warning `preset` gives.
+        // Slots beyond the runtime array are never published to on this board,
+        // so retain the same inactive-hardware warning `preset` gives.
        if (slot >= RUNTIME_MQTT_SLOTS &&
            filter_mask != MQTTPacketFilter::kAllPacketTypes) {
          size_t used = strlen(reply);
          if (used < 158) {
            snprintf(reply + used, 160 - used,
-                     " (slot inactive on this hardware; blocks firmware rollback)");
+                     " (slot inactive on this hardware)");
          }
        }
      }
@@ -562,21 +616,35 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
  } else if (memcmp(config, "mqtt.analyzer.us ", 17) == 0) {
    const int slot = 0;
    if (memcmp(&config[17], "on", 2) == 0) {
+      const int dup_slot = mqttAssignedPresetSlot(
+          _mqtt_prefs.mqtt_slot_preset, "analyzer-us", slot);
+      if (dup_slot >= 0) {
+        sprintf(reply, "Error: preset 'analyzer-us' is already assigned to slot %d",
+                dup_slot + 1);
+        return true;
+      }
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], "analyzer-us", sizeof(_mqtt_prefs.mqtt_slot_preset[slot]));
    } else {
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], MQTT_PRESET_NONE, sizeof(_mqtt_prefs.mqtt_slot_preset[slot]));
    }
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    _callbacks->restartBridgeSlot(slot);
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.analyzer.eu ", 17) == 0) {
    const int slot = 1;
    if (memcmp(&config[17], "on", 2) == 0) {
+      const int dup_slot = mqttAssignedPresetSlot(
+          _mqtt_prefs.mqtt_slot_preset, "analyzer-eu", slot);
+      if (dup_slot >= 0) {
+        sprintf(reply, "Error: preset 'analyzer-eu' is already assigned to slot %d",
+                dup_slot + 1);
+        return true;
+      }
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], "analyzer-eu", sizeof(_mqtt_prefs.mqtt_slot_preset[slot]));
    } else {
      StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], MQTT_PRESET_NONE, sizeof(_mqtt_prefs.mqtt_slot_preset[slot]));
    }
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    _callbacks->restartBridgeSlot(slot);
    strcpy(reply, "OK");
  } else if (memcmp(config, "mqtt.owner ", 11) == 0) {
@@ -585,11 +653,11 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      // Owner key is optional — empty clears it (previously this errored, so a
      // set key could never be removed via the portal/CLI).
      _mqtt_prefs.mqtt_owner_public_key[0] = '\0';
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      strcpy(reply, "OK - owner key cleared");
    } else if (mqttOwnerKeyValid(owner_key)) {
      StrHelper::strncpy(_mqtt_prefs.mqtt_owner_public_key, owner_key, sizeof(_mqtt_prefs.mqtt_owner_public_key));
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      strcpy(reply, "OK");
    } else {
      strcpy(reply, "Error: public key must be 64 hex characters (32 bytes)");
@@ -597,7 +665,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
  } else if (memcmp(config, "mqtt.email ", 11) == 0) {
    if (valueTooLong(&config[11], sizeof(_mqtt_prefs.mqtt_email), reply, "email")) return true;
    StrHelper::strncpy(_mqtt_prefs.mqtt_email, &config[11], sizeof(_mqtt_prefs.mqtt_email));
-    savePrefs();
+    if (!persistObserverPrefs(reply)) return true;
    strcpy(reply, "OK");
 #endif
  } else if (memcmp(config, "alert ", 6) == 0) {
@@ -605,12 +673,12 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    const char* val = &config[6];
    if (memcmp(val, "on", 2) == 0 && (val[2] == 0 || val[2] == ' ')) {
      _mqtt_prefs.alert_enabled = 1;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->onAlertConfigChanged();
      strcpy(reply, "OK - alerts on");
    } else if (memcmp(val, "off", 3) == 0 && (val[3] == 0 || val[3] == ' ')) {
      _mqtt_prefs.alert_enabled = 0;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->onAlertConfigChanged();
      strcpy(reply, "OK - alerts off");
    } else {
@@ -625,7 +693,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    if (len == 0) {
      _mqtt_prefs.alert_psk_hex[0] = '\0';
      _mqtt_prefs.alert_hashtag[0] = '\0';
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->onAlertConfigChanged();
      strcpy(reply, "OK - alert.psk cleared (alerts disabled until configured)");
    } else if (val[0] == '#') {
@@ -659,7 +727,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
          // The new PSK is operator-supplied, so any previously-derived
          // hashtag name is no longer accurate provenance — drop it.
          _mqtt_prefs.alert_hashtag[0] = '\0';
-          savePrefs();
+          if (!persistObserverPrefs(reply)) return true;
          _callbacks->onAlertConfigChanged();
          strcpy(reply, "OK - alert.psk updated");
        }
@@ -672,7 +740,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    if (in_len == 0) {
      _mqtt_prefs.alert_psk_hex[0] = '\0';
      _mqtt_prefs.alert_hashtag[0] = '\0';
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->onAlertConfigChanged();
      strcpy(reply, "OK - alert.hashtag cleared (alerts disabled until configured)");
    } else {
@@ -708,7 +776,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
          mesh::Utils::toHex(hex, digest, 16);
          StrHelper::strncpy(_mqtt_prefs.alert_hashtag, hashtag, sizeof(_mqtt_prefs.alert_hashtag));
          StrHelper::strncpy(_mqtt_prefs.alert_psk_hex, hex, sizeof(_mqtt_prefs.alert_psk_hex));
-          savePrefs();
+          if (!persistObserverPrefs(reply)) return true;
          _callbacks->onAlertConfigChanged();
          sprintf(reply, "OK - alert.hashtag: %s", _mqtt_prefs.alert_hashtag);
        }
@@ -726,7 +794,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    size_t len = strlen(val);
    if (len == 0) {
      _mqtt_prefs.alert_region[0] = '\0';
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->onAlertConfigChanged();
      strcpy(reply, "OK - alert.region cleared (using default scope)");
    } else if (len >= sizeof(_mqtt_prefs.alert_region)) {
@@ -734,7 +802,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
    } else {
      StrHelper::strncpy(_mqtt_prefs.alert_region, val, sizeof(_mqtt_prefs.alert_region));
      StrHelper::stripSurroundingQuotes(_mqtt_prefs.alert_region, sizeof(_mqtt_prefs.alert_region));
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      _callbacks->onAlertConfigChanged();
      sprintf(reply, "OK - alert.region: %s", _mqtt_prefs.alert_region);
    }
@@ -744,7 +812,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      strcpy(reply, "Error: alert.wifi must be 0-1440 minutes (0=off)");
    } else {
      _mqtt_prefs.alert_wifi_minutes = (uint16_t)mins;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      sprintf(reply, "OK - alert.wifi %d min%s", mins, mins == 0 ? " (disabled)" : "");
    }
  } else if (memcmp(config, "alert.mqtt ", 11) == 0) {
@@ -753,7 +821,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      strcpy(reply, "Error: alert.mqtt must be 0-10080 minutes (0=off)");
    } else {
      _mqtt_prefs.alert_mqtt_minutes = (uint16_t)mins;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      sprintf(reply, "OK - alert.mqtt %d min%s", mins, mins == 0 ? " (disabled)" : "");
    }
  } else if (memcmp(config, "alert.interval ", 15) == 0) {
@@ -764,7 +832,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
      strcpy(reply, "Error: alert.interval must be 60-10080 minutes");
    } else {
      _mqtt_prefs.alert_min_interval_min = (uint16_t)mins;
-      savePrefs();
+      if (!persistObserverPrefs(reply)) return true;
      sprintf(reply, "OK - alert.interval %d min", mins);
    }
  } else {