Merge pull request #54 from agessaman/fix/mqtt-review-2026-09-09

MQTT client ownership and review fixes (F01–F04, F06–F08, F11–F14)

MQTT_IMPLEMENTATION.md

@@ -524,14 +524,14 @@ These settings apply across all MQTT slots:
 - `get mqtt.neighbors` - Get periodic neighbors publishing setting (on/off; neighbors-enabled builds)
 - `get mqtt.neighbors.interval` - Get neighbors publish interval in hours (neighbors-enabled builds)
 - `get mqtt.ntp` - Get effective NTP server hostname
-- `get mqtt.ntp.diag` - Probe every configured NTP server for connectivity (does not change the clock; serial console shows each server's reported time, LoRa shows a compact `<server> ok|fail` list)
+- `get mqtt.ntp.diag` - Probe every configured NTP server for connectivity (does not change the clock; serial console shows each server's reported time, or why a probe was rejected — `DNS failed`, `unsolicited reply`, `server unsynced`, ... — and LoRa shows a compact `<server> ok|fail` list)
 - `get mqtt.owner` - Get owner public key (serial console only)
 - `get mqtt.email` - Get owner email address (serial console only)
 
 #### Set Commands
 - `set mqtt.origin <name>` - Set device origin name
 - `set mqtt.iata <code>` - Set IATA code (auto-uppercased)
-- `set mqtt.status on|off` - Enable/disable status messages
+- `set mqtt.status on|off` - Enable/disable status messages (periodic *and* the one sent on each broker connect)
 - `set mqtt.packets on|off` - Enable/disable packet messages
 - `set mqtt.raw on|off` - Enable/disable raw messages
 - `set mqtt.rx on|off` - Enable/disable RX (received) packet uplinking
@@ -883,7 +883,8 @@ the radio actually performs in that case.
 ### NTP Time Synchronization
 - Automatic time synchronization with NTP servers (required for JWT authentication)
 - Default primary: `pool.ntp.org`; built-in fallbacks (tried sequentially on failure): `time.google.com`, `time.cloudflare.com`, `time.aws.com`, `time.nist.gov`
-- Periodic time updates (every hour) on the effective primary only; system time is kept in UTC
+- Periodic time updates (every hour) through the same validated probe, one attempt per server in list order; system time is kept in UTC
+- Replies are validated before they are trusted: the datagram must be a full-length NTPv3/v4 server reply from the queried address and port, from a synchronised server (stratum 1-15, no leap alarm), echoing the random transmit timestamp of the request, with a plausible epoch. Anything else is discarded and the clock, the RTC and JWT issuance are left alone
 - Configure and diagnose with `set mqtt.ntp` / `get mqtt.ntp` / `get mqtt.ntp.diag` — see [MQTT Shared Commands](#mqtt-shared-commands)
 
 ### Authentication

examples/simple_repeater/MyMesh.cpp

@@ -1699,6 +1699,19 @@ void MyMesh::loop() {
   // MQTT processing runs in a separate FreeRTOS task on Core 0, so we don't call bridge.loop() here
   mesh::Mesh::loop();
 
+#ifdef WITH_MQTT_BRIDGE
+  // A timed-out stop keeps the bridge down until the MQTT task acknowledges it.
+  // Release the withheld resources whenever that late ack lands, and restart
+  // only a bridge that is still meant to be running.
+  if (bridge && bridge->stopAcknowledgedLate()) {
+    bridge->pollLateStopAck();
+    if (_bridge_resume_pending && _prefs.bridge_enabled) {
+      Serial.println("MQTT: stop acknowledged late - resuming bridge");
+      setBridgeState(true);
+    }
+  }
+#endif
+
 #ifdef WITH_BRIDGE
   // bridge.loop() is now handled by FreeRTOS task on Core 0 - no need to call it here
 #endif
@@ -1752,8 +1765,9 @@ void MyMesh::loop() {
     // resume the bridge instead of flashing under uncertain ownership.
     if (bridge && !bridge->canFlashAfterStop()) {
       Serial.println("OTA: aborted, MQTT stop did not complete cleanly - resuming bridge");
-      otaAlert("OTA aborted: MQTT stop unclean, bridge resumed");
       setBridgeState(true);
+      otaAlert(bridge->isRunning() ? "OTA aborted: MQTT stop unclean, bridge resumed"
+                                   : "OTA aborted: MQTT stop unproven, bridge resumes when it completes");
     } else if (!_cli.getBoard()->otaFromManifest(getFirmwareVer(), false, ota_reply)) {
       Serial.print("OTA: aborted, resuming bridge - "); Serial.println(ota_reply);
       char ota_alert_msg[160];

examples/simple_repeater/MyMesh.h

@@ -137,6 +137,8 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks
   ESPNowBridge bridge;
 #elif defined(WITH_MQTT_BRIDGE)
   MQTTBridge* bridge;
+  // begin() was refused after an unproven stop; restart once the task acknowledges.
+  bool _bridge_resume_pending = false;
 #endif
 #ifdef WITH_SNMP
   MeshSNMPAgent _snmp_agent;
@@ -351,6 +353,9 @@ public:
 #endif
       if (!bridge) return;
     }
+#ifdef WITH_MQTT_BRIDGE
+    if (!enable) _bridge_resume_pending = false;   // an explicit stop cancels a pending resume
+#endif
     if (enable == bridge->isRunning()) return;
     if (enable)
     {
@@ -368,6 +373,7 @@ public:
       bridge->begin();
 #ifdef WITH_MQTT_BRIDGE
       _alerter.setBridge(bridge);
+      _bridge_resume_pending = !bridge->isRunning() && bridge->isStopUnproven();
 #endif
     }
     else
@@ -375,6 +381,7 @@ public:
       bridge->end();
 #ifdef WITH_MQTT_BRIDGE
       _alerter.setBridge(nullptr);
+      _bridge_resume_pending = false;
 #endif
     }
   }
@@ -400,6 +407,9 @@ public:
     bridge->setStatsSources(this, _radio, _cli.getBoard(), _ms);
 #endif
     bridge->begin();
+#ifdef WITH_MQTT_BRIDGE
+    _bridge_resume_pending = !bridge->isRunning() && bridge->isStopUnproven();
+#endif
   }
 
   void restartBridgeSlot(int slot) override {

examples/simple_room_server/MyMesh.cpp

@@ -1514,6 +1514,19 @@ void MyMesh::loop() {
   // Check radio FIRST to ensure we don't miss incoming packets
   // MQTT processing can take time, so we prioritize radio reception
   mesh::Mesh::loop();
+
+#ifdef WITH_MQTT_BRIDGE
+  // A timed-out stop keeps the bridge down until the MQTT task acknowledges it.
+  // Release the withheld resources whenever that late ack lands, and restart
+  // only a bridge that is still meant to be running.
+  if (bridge && bridge->stopAcknowledgedLate()) {
+    bridge->pollLateStopAck();
+    if (_bridge_resume_pending && _prefs.bridge_enabled) {
+      Serial.println("MQTT: stop acknowledged late - resuming bridge");
+      setBridgeState(true);
+    }
+  }
+#endif
 #ifdef WITH_MQTT_BRIDGE
   // bridge.loop() is now handled by FreeRTOS task on Core 0 - no need to call it here
 #endif
@@ -1609,14 +1622,18 @@ void MyMesh::loop() {
     drainOutbound(OTA_TX_DRAIN_TIMEOUT_MS);
 
     bool may_flash = true;
+    if (bridge) bridge->pollLateStopAck();
     if (bridge_was_running) {
       setBridgeState(false);
       // OTA must not write after a forced/timed-out MQTT shutdown: its TLS/heap
       // ownership is uncertain until a subsequent clean start/stop cycle.
       may_flash = bridge && bridge->canFlashAfterStop();
-      if (!may_flash) {
-        Serial.println("OTA: aborted, MQTT stop did not complete cleanly");
-      }
+    } else if (bridge && bridge->isStopUnproven()) {
+      // Reads as stopped, but an unacknowledged MQTT task may still own TLS/client state.
+      may_flash = false;
+    }
+    if (!may_flash) {
+      Serial.println("OTA: aborted, MQTT stop did not complete cleanly");
     }
 
     char ota_reply[160];

examples/simple_room_server/MyMesh.h

@@ -205,6 +205,8 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks
 #endif
 #ifdef WITH_MQTT_BRIDGE
   MQTTBridge* bridge;
+  // begin() was refused after an unproven stop; restart once the task acknowledges.
+  bool _bridge_resume_pending = false;
 #endif
 #ifdef WITH_SNMP
   MeshSNMPAgent _snmp_agent;
@@ -362,6 +364,9 @@ public:
 #endif
       if (!bridge) return;
     }
+#ifdef WITH_MQTT_BRIDGE
+    if (!enable) _bridge_resume_pending = false;   // an explicit stop cancels a pending resume
+#endif
     if (enable == bridge->isRunning()) return;
     if (enable)
     {
@@ -385,6 +390,7 @@ public:
       bridge->begin();
 #ifdef WITH_MQTT_BRIDGE
       _alerter.setBridge(bridge);
+      _bridge_resume_pending = !bridge->isRunning() && bridge->isStopUnproven();
 #endif
     }
     else
@@ -392,6 +398,7 @@ public:
       bridge->end();
 #ifdef WITH_MQTT_BRIDGE
       _alerter.setBridge(nullptr);
+      _bridge_resume_pending = false;
 #endif
     }
   }
@@ -416,6 +423,9 @@ public:
     bridge->setStatsSources(this, _radio, _cli.getBoard(), _ms);
 #endif
     bridge->begin();
+#ifdef WITH_MQTT_BRIDGE
+    _bridge_resume_pending = !bridge->isRunning() && bridge->isStopUnproven();
+#endif
   }
 
   void restartBridgeSlot(int slot) override {

lib/PsychicMqttClient/src/PsychicMqttClient.cpp

@@ -20,7 +20,8 @@ PsychicMqttClient::PsychicMqttClient() : _mqtt_cfg()
 
 PsychicMqttClient::~PsychicMqttClient()
 {
-    disconnect();
+    // Owners stop the client before deleting it; stopping it again only logs ESP_FAIL.
+    if (_started) disconnect();
     if (_client != nullptr)
     {
         esp_mqtt_client_destroy(_client);
@@ -371,21 +372,59 @@ bool PsychicMqttClient::connected()
     return _connected;
 }
 
-void PsychicMqttClient::connect()
+esp_err_t PsychicMqttClient::applyConfig()
 {
 #if ESP_IDF_VERSION_MAJOR == 5
     if (_mqtt_cfg.broker.address.uri == nullptr)
-    {
-        ESP_LOGE(TAG, "MQTT URI not set.");
-        return;
-    }
-    int desired_buffer = _mqtt_cfg.buffer.size > 0 ? _mqtt_cfg.buffer.size : 1024;
 #else
     if (_mqtt_cfg.uri == nullptr)
+#endif
     {
         ESP_LOGE(TAG, "MQTT URI not set.");
-        return;
+        return ESP_ERR_INVALID_STATE;
     }
+
+    if (_client == nullptr)
+    {
+        // esp_mqtt_client_init() takes the whole configuration, including the
+        // buffer sizes it allocates once and never resizes.
+        _client = esp_mqtt_client_init(&_mqtt_cfg);
+        if (_client == nullptr)
+        {
+            ESP_LOGE(TAG, "esp_mqtt_client_init failed");
+            return ESP_ERR_NO_MEM;
+        }
+        // Register event handler only once when client is first created
+        // to avoid memory leak from repeated registrations
+        esp_mqtt_client_register_event(_client, MQTT_EVENT_ANY, _onMqttEventStatic, this);
+        _config_dirty = false;
+        return ESP_OK;
+    }
+
+    if (!_config_dirty)
+    {
+        ESP_LOGD(TAG, "applyConfig(): mqtt config unchanged, skipping set_config");
+        return ESP_OK;
+    }
+
+    esp_err_t cfg_result = esp_mqtt_set_config(_client, &_mqtt_cfg);
+    if (cfg_result != ESP_OK)
+    {
+        // Leave _config_dirty set so the next attempt retries the whole
+        // transaction rather than starting on a partly-updated configuration.
+        ESP_LOGE(TAG, "applyConfig(): failed to apply mqtt config: %s", esp_err_to_name(cfg_result));
+        return cfg_result;
+    }
+    _config_dirty = false;
+    ESP_LOGD(TAG, "applyConfig(): applied mqtt config update");
+    return ESP_OK;
+}
+
+esp_err_t PsychicMqttClient::connect()
+{
+#if ESP_IDF_VERSION_MAJOR == 5
+    int desired_buffer = _mqtt_cfg.buffer.size > 0 ? _mqtt_cfg.buffer.size : 1024;
+#else
     int desired_buffer = _mqtt_cfg.buffer_size > 0 ? _mqtt_cfg.buffer_size : 1024;
 #endif
 
@@ -400,41 +439,17 @@ void PsychicMqttClient::connect()
         {
             ESP_LOGE(TAG, "Failed to allocate reassembly buffer (%u bytes)", (unsigned)_buffer_capacity);
             _buffer_capacity = 0;
+            return ESP_ERR_NO_MEM;
         }
     }
 
-    if (_client == nullptr)
-    {
-        _client = esp_mqtt_client_init(&_mqtt_cfg);
-        // Register event handler only once when client is first created
-        // to avoid memory leak from repeated registrations
-        esp_mqtt_client_register_event(_client, MQTT_EVENT_ANY, _onMqttEventStatic, this);
-        _config_dirty = false;
-    }
-    else
-    {
-        if (_config_dirty)
-        {
-            esp_err_t cfg_result = esp_mqtt_set_config(_client, &_mqtt_cfg);
-            ESP_ERROR_CHECK_WITHOUT_ABORT(cfg_result);
-            if (cfg_result == ESP_OK)
-            {
-                _config_dirty = false;
-                ESP_LOGD(TAG, "connect(): applied mqtt config update");
-            }
-            else
-            {
-                ESP_LOGW(TAG, "connect(): failed to apply mqtt config, will retry");
-            }
-        }
-        else
-        {
-            ESP_LOGD(TAG, "connect(): mqtt config unchanged, skipping set_config");
-        }
-    }
+    // A failed config transaction must not be followed by a start: the client
+    // would come up on a partly-updated configuration, which is how a slot
+    // connects with the previous broker's credentials.
+    esp_err_t cfg_result = applyConfig();
+    if (cfg_result != ESP_OK) return cfg_result;
 
     esp_err_t start_result = esp_mqtt_client_start(_client);
-    ESP_ERROR_CHECK_WITHOUT_ABORT(start_result);
     if (start_result == ESP_OK)
     {
         _started = true;
@@ -445,76 +460,90 @@ void PsychicMqttClient::connect()
         // Reporting success here hides the one state reconnect() cannot recover from.
         ESP_LOGE(TAG, "MQTT client failed to start: %s", esp_err_to_name(start_result));
     }
+    return start_result;
 }
 
-void PsychicMqttClient::reconnect()
+esp_err_t PsychicMqttClient::reconnect()
 {
     if (_client == nullptr)
     {
         ESP_LOGW(TAG, "MQTT client not initialized, cannot reconnect.");
-        return;
+        return ESP_ERR_INVALID_STATE;
     }
-    if (_config_dirty)
+    // A failed config update aborts the reconnect: reconnecting on the previous
+    // config was how a renewed token silently failed to reach the connection.
+    esp_err_t cfg_result = applyConfig();
+    if (cfg_result != ESP_OK) return cfg_result;
+
+    esp_err_t r = esp_mqtt_client_reconnect(_client);
+    if (r != ESP_OK)
     {
-        // Apply config only when mutating setters changed _mqtt_cfg.
-        esp_err_t cfg_result = esp_mqtt_set_config(_client, &_mqtt_cfg);
-        ESP_ERROR_CHECK_WITHOUT_ABORT(cfg_result);
-        if (cfg_result == ESP_OK)
-        {
-            _config_dirty = false;
-            ESP_LOGD(TAG, "reconnect(): applied mqtt config update");
-        }
-        else
-        {
-            ESP_LOGW(TAG, "reconnect(): failed to apply mqtt config, reconnecting with previous config");
-        }
+        ESP_LOGE(TAG, "MQTT reconnect request failed: %s", esp_err_to_name(r));
+        return r;
     }
-    else
-    {
-        ESP_LOGD(TAG, "reconnect(): mqtt config unchanged, skipping set_config");
-    }
-    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_mqtt_client_reconnect(_client));
     ESP_LOGI(TAG, "MQTT client reconnect requested.");
+    return ESP_OK;
 }
 
-void PsychicMqttClient::disconnect()
+esp_err_t PsychicMqttClient::disconnect(unsigned long timeout_ms)
 {
     if (_client == nullptr)
     {
         ESP_LOGW(TAG, "MQTT client not started.");
-        return;
+        return ESP_ERR_INVALID_STATE;
     }
 
+    bool clean = true;
     if (_connected)
     {
         ESP_LOGI(TAG, "Disconnecting MQTT client.");
         _stopMqttClient = false;
         esp_mqtt_client_disconnect(_client);
 
-        // Wait for all disconnect events to be processed
-        while (!_stopMqttClient)
+        // Bounded, unlike the original: this runs on the MQTT task whose stop
+        // acknowledgement the bridge's shutdown waits for, so a lost
+        // DISCONNECTED event used to wedge the whole teardown.
+        unsigned long waited = 0;
+        while (!_stopMqttClient && waited < timeout_ms)
         {
             vTaskDelay(10 / portTICK_PERIOD_MS);
+            waited += 10;
+        }
+        if (!_stopMqttClient)
+        {
+            ESP_LOGW(TAG, "disconnect: no DISCONNECTED event in %lums; stopping anyway", timeout_ms);
+            clean = false;
         }
     }
 
-    esp_mqtt_client_stop(_client);
-    _started = false;
-    ESP_LOGI(TAG, "MQTT client stopped.");
+    esp_err_t stop_result = esp_mqtt_client_stop(_client);
+    if (stop_result == ESP_OK)
+    {
+        _started = false;
+        ESP_LOGI(TAG, "MQTT client stopped.");
+    }
+    else
+    {
+        // The SDK task was not joined. Saying otherwise is what let the caller
+        // destroy the client from under a live task.
+        ESP_LOGE(TAG, "esp_mqtt_client_stop failed: %s", esp_err_to_name(stop_result));
+        return stop_result;
+    }
+    return clean ? ESP_OK : ESP_ERR_TIMEOUT;
 }
 
-void PsychicMqttClient::softDisconnect(unsigned long timeout_ms)
+esp_err_t PsychicMqttClient::softDisconnect(unsigned long timeout_ms)
 {
     if (_client == nullptr)
     {
         ESP_LOGW(TAG, "MQTT client not started.");
-        return;
+        return ESP_ERR_INVALID_STATE;
     }
 
     if (!_connected)
     {
         // Nothing to close; leaving the task alone is the whole point.
-        return;
+        return ESP_OK;
     }
 
     ESP_LOGI(TAG, "Disconnecting MQTT transport (client task retained).");
@@ -530,25 +559,34 @@ void PsychicMqttClient::softDisconnect(unsigned long timeout_ms)
     if (!_stopMqttClient)
     {
         ESP_LOGW(TAG, "softDisconnect: no DISCONNECTED event in %lums", timeout_ms);
+        return ESP_ERR_TIMEOUT;
     }
+    return ESP_OK;
 }
 
-void PsychicMqttClient::forceStop()
+esp_err_t PsychicMqttClient::forceStop()
 {
     if (_client == nullptr)
     {
         ESP_LOGW(TAG, "MQTT client not started.");
-        return;
+        return ESP_ERR_INVALID_STATE;
     }
 
     if (_connected)
     {
         ESP_LOGI(TAG, "Forced stop MQTT client.");
     }
-    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_mqtt_client_stop(_client));
+    esp_err_t r = esp_mqtt_client_stop(_client);
+    if (r != ESP_OK)
+    {
+        // The SDK task was not joined: the caller must not destroy this client.
+        ESP_LOGE(TAG, "forceStop: esp_mqtt_client_stop failed: %s", esp_err_to_name(r));
+        return r;
+    }
     _connected = false;
     _started = false;
     ESP_LOGI(TAG, "MQTT client forcefully stopped.");
+    return ESP_OK;
 }
 
 int PsychicMqttClient::subscribe(const char *topic, int qos)

lib/PsychicMqttClient/src/PsychicMqttClient.h

@@ -345,12 +345,37 @@ public:
      */
     bool connected();
 
+    /**
+     * @brief Commits the pending configuration to the SDK client, creating it
+     *        if it does not exist yet, WITHOUT starting it.
+     *
+     * Exists so a caller can tell "the configuration is now what I asked for"
+     * from "the client started": esp_mqtt_client_start() can fail after the
+     * configuration has been committed, and a caller that cannot distinguish
+     * those records a stale view of what the SDK actually holds.
+     *
+     * @return ESP_OK when the SDK client holds this configuration.
+     *         ESP_ERR_INVALID_STATE if no URI is set, ESP_ERR_NO_MEM if the
+     *         client could not be allocated, or the error from
+     *         esp_mqtt_set_config() (which leaves the update pending).
+     */
+    esp_err_t applyConfig();
+
     /**
      * @brief Connects the MQTT client to the server.
      *
      * @note All parameters must be set before calling this method.
+     *
+     * @return ESP_OK when esp_mqtt_client_start() accepted the client.
+     *         ESP_ERR_INVALID_STATE if no URI is set, ESP_ERR_NO_MEM if the
+     *         client or its reassembly buffer could not be allocated, or the
+     *         error from esp_mqtt_set_config()/esp_mqtt_client_start().
+     *
+     * A failed configuration update does NOT start the client: starting with a
+     * half-updated config is how a slot ends up connected with the previous
+     * broker's credentials.
      */
-    void connect();
+    esp_err_t connect();
 
     /**
      * @brief Reconnects a previously started MQTT client.
@@ -359,14 +384,28 @@ public:
      * re-initiating a connection on an already-started client, especially
      * when auto-reconnect is disabled. Updates config before reconnecting
      * so credential changes (e.g., refreshed JWT tokens) take effect.
+     *
+     * @return ESP_OK when the reconnect was requested, ESP_ERR_INVALID_STATE on
+     *         an uninitialised client, or the error from esp_mqtt_set_config().
+     *         A failed config update aborts the reconnect rather than
+     *         reconnecting with the previous configuration.
      */
-    void reconnect();
+    esp_err_t reconnect();
 
     /**
-     * @brief Disconnects the MQTT client from the server.
-     * This call might be blocking until the client is stopped cleanly
+     * @brief Disconnects the MQTT client and stops its task.
+     *
+     * Blocks until the DISCONNECTED event arrives or timeout_ms elapses, then
+     * stops the client either way. The wait used to be unbounded, which turned
+     * a lost event into a wedged caller — and that caller is the MQTT task
+     * whose acknowledgement the bridge's shutdown waits on.
+     *
+     * @return ESP_OK when the client stopped after a clean disconnect,
+     *         ESP_ERR_TIMEOUT when the DISCONNECTED event never arrived (the
+     *         stop still ran), ESP_ERR_INVALID_STATE on an uninitialised
+     *         client, or the error from esp_mqtt_client_stop().
      */
-    void disconnect();
+    esp_err_t disconnect(unsigned long timeout_ms = 10000);
 
     /**
      * @brief Closes the transport but leaves the client task running.
@@ -380,8 +419,12 @@ public:
      * @param timeout_ms how long to wait for the DISCONNECTED event before
      *                   giving up. Bounded on purpose: disconnect()'s wait is
      *                   unbounded and a lost event would wedge the caller.
+     *
+     * @return ESP_OK when the DISCONNECTED event arrived, ESP_ERR_TIMEOUT if it
+     *         did not (the transport may still be open), ESP_ERR_INVALID_STATE
+     *         on an uninitialised client, ESP_OK when already disconnected.
      */
-    void softDisconnect(unsigned long timeout_ms = 5000);
+    esp_err_t softDisconnect(unsigned long timeout_ms = 5000);
 
     /**
      * @brief True once esp_mqtt_client_start() has succeeded and no stop has run.
@@ -392,10 +435,14 @@ public:
     bool isStarted() const { return _started; }
 
     /**
-     * @brief Forcefully stops the MQTT client and disconnects from the server.
+     * @brief Stops the MQTT client without waiting for a DISCONNECTED event.
      * This does not trigger the onDisconnect callbacks.
+     *
+     * @return the result of esp_mqtt_client_stop(), or ESP_ERR_INVALID_STATE on
+     *         an uninitialised client. A non-OK result means the SDK task was
+     *         NOT joined: the caller must not destroy the client after it.
      */
-    void forceStop();
+    esp_err_t forceStop();
 
     /**
      * @brief Subscribes to a topic. Server must be connected

src/Dispatcher.cpp

@@ -29,6 +29,9 @@ void Dispatcher::begin() {
 
   _radio->begin();
   prev_isrecv_mode = _radio->isInRecvMode();
+#ifdef WITH_MQTT_BRIDGE
+  radio_watchdog.reset((uint32_t)_ms->getMillis());
+#endif
 }
 
 float Dispatcher::getAirtimeBudgetFactor() const {
@@ -94,23 +97,20 @@ void Dispatcher::loop() {
   // MQTTPrefs radio_watchdog_minutes setting.
 #ifdef WITH_MQTT_BRIDGE
   {
-    const uint32_t watchdog_ms = getRadioWatchdogMillis();
-    if (watchdog_ms > 0) {
-      unsigned long last_recv = _radio->getLastRecvMillis();
-      unsigned long last_irq  = _radio->getLastRadioInterruptMillis();
-      unsigned long last_active = (last_recv > last_irq ? last_recv : last_irq);
-      if (last_radio_active_ms > last_active) last_active = last_radio_active_ms;
-      if (is_recv && last_active > 0) {
-        unsigned long silent_ms = _ms->getMillis() - last_active;
-        unsigned long since_recovery = _ms->getMillis() - last_watchdog_recovery;
-        if (silent_ms > watchdog_ms && since_recovery > watchdog_ms) {
-          _err_flags |= ERR_EVENT_RADIO_WATCHDOG;
-          MESH_DEBUG_PRINTLN("Radio watchdog: silent %lu ms, state=%d, recovering", silent_ms, _radio->getRadioState());
-          _radio->idle();
-          _radio->startRecv();
-          last_watchdog_recovery = _ms->getMillis();
-        }
-      }
+    // Selecting the most recent event by age rather than by largest timestamp is
+    // what keeps this correct across the millis() wrap — see RadioWatchdog.
+    RadioWatchdogDecision wd = radio_watchdog.update(
+        (uint32_t)_ms->getMillis(), is_recv, getRadioWatchdogMillis(),
+        (uint32_t)_radio->getLastRecvMillis(),
+        (uint32_t)_radio->getLastRadioInterruptMillis(),
+        (uint32_t)last_radio_active_ms);
+    if (wd.recover) {
+      _err_flags |= ERR_EVENT_RADIO_WATCHDOG;
+      MESH_DEBUG_PRINTLN("Radio watchdog: silent %lu ms, state=%d, recovering",
+                         (unsigned long)wd.silent_ms, _radio->getRadioState());
+      _radio->idle();
+      _radio->startRecv();
+      radio_watchdog.noteRecovery();
     }
   }
 #endif // WITH_MQTT_BRIDGE (radio watchdog)

src/Dispatcher.h

@@ -5,6 +5,9 @@
 #include <Packet.h>
 #include <Utils.h>
 #include <string.h>
+#ifdef WITH_MQTT_BRIDGE
+  #include "helpers/RadioWatchdog.h"
+#endif
 
 namespace mesh {
 
@@ -135,8 +138,10 @@ typedef uint32_t  DispatcherAction;
 class Dispatcher {
   Packet* outbound;  // current outbound packet
   unsigned long outbound_expiry, outbound_start, total_air_time, rx_air_time;
-  unsigned long last_watchdog_recovery;
   unsigned long last_radio_active_ms;   // updated on any TX or RX event; used by watchdog
+#ifdef WITH_MQTT_BRIDGE
+  RadioWatchdog radio_watchdog;
+#endif
   unsigned long next_tx_time;
   unsigned long cad_busy_start;
   unsigned long radio_nonrx_start;
@@ -171,7 +176,6 @@ protected:
     tx_budget_ms = 0;
     last_budget_update = 0;
     duty_cycle_window_ms = 3600000;
-    last_watchdog_recovery = 0;
     last_radio_active_ms = 0;
   }
 

src/helpers/CommonCLI_Observer.cpp

@@ -15,6 +15,7 @@
 #include "TxtDataHelpers.h"
 #include "AlertReporter.h"  // for alertReporterBannedChannelMatch[Hex]()
 #include "MQTTObserverValidation.h"  // pure input validators (host-testable)
+#include "WifiPowerSavePolicy.h"     // one powersave value->mode/name mapping
 #include <Utils.h>
 #include <new>
 #ifdef ESP_PLATFORM
@@ -438,41 +439,28 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
     if (!persistObserverPrefs(reply)) return true;
     strcpy(reply, "OK");
   } else if (memcmp(config, "wifi.powersave ", 15) == 0) {
-    const char* value = &config[15];
     uint8_t ps_value;
-    bool valid = false;
-    if (memcmp(value, "min", 3) == 0 && (value[3] == 0 || value[3] == ' ')) {
-      ps_value = 0;
-      valid = true;
-    } else if (memcmp(value, "none", 4) == 0 && (value[4] == 0 || value[4] == ' ')) {
-      ps_value = 1;
-      valid = true;
-    } else if (memcmp(value, "max", 3) == 0 && (value[3] == 0 || value[3] == ' ')) {
-      ps_value = 2;
-      valid = true;
-    }
-    if (!valid) {
+    if (!WifiPowerSavePolicy::parseName(&config[15], &ps_value)) {
       strcpy(reply, "Error: must be none, min, or max");
     } else {
       _mqtt_prefs.wifi_power_save = ps_value;
       if (!persistObserverPrefs(reply)) return true;
+      const char* ps_name = WifiPowerSavePolicy::nameFor(ps_value);
 #ifdef ESP_PLATFORM
       if (WiFi.status() == WL_CONNECTED) {
-        wifi_ps_type_t ps_mode = (ps_value == 1) ? WIFI_PS_NONE :
-                                (ps_value == 2) ? WIFI_PS_MAX_MODEM : WIFI_PS_MIN_MODEM;
-        esp_err_t ps_result = esp_wifi_set_ps(ps_mode);
+        // Same mapping the bridge applies on every association, so this cannot
+        // drift back apart (see WifiPowerSavePolicy).
+        esp_err_t ps_result =
+            esp_wifi_set_ps((wifi_ps_type_t)WifiPowerSavePolicy::modeFor(ps_value));
         if (ps_result == ESP_OK) {
-          const char* ps_name = (ps_value == 1) ? "none" : (ps_value == 2) ? "max" : "min";
           sprintf(reply, "OK - power save set to %s", ps_name);
         } else {
           sprintf(reply, "OK - saved, but failed to apply: %d", ps_result);
         }
       } else {
-        const char* ps_name = (ps_value == 1) ? "none" : (ps_value == 2) ? "max" : "min";
         sprintf(reply, "OK - saved as %s (will apply on next WiFi connection)", ps_name);
       }
 #else
-      const char* ps_name = (ps_value == 1) ? "none" : (ps_value == 2) ? "max" : "min";
       sprintf(reply, "OK - saved as %s", ps_name);
 #endif
     }
@@ -1090,9 +1078,7 @@ bool CommonCLI::handleObserverGetCmd(uint32_t sender_timestamp, const char* conf
 #endif
     }
   } else if (memcmp(config, "wifi.powersave", 14) == 0) {
-    uint8_t ps = _mqtt_prefs.wifi_power_save;
-    const char* ps_name = (ps == 1) ? "none" : (ps == 2) ? "max" : "min";
-    sprintf(reply, "> %s", ps_name);
+    sprintf(reply, "> %s", WifiPowerSavePolicy::nameFor(_mqtt_prefs.wifi_power_save));
   } else if (memcmp(config, "timezone.offset", 15) == 0) {
     // Must precede the "timezone" (8-byte) check below — that prefix-matches
     // "timezone.offset" too, so the more-specific key has to come first or

src/helpers/MQTTClientState.h

@@ -0,0 +1,82 @@
+#pragma once
+
+#include <stdint.h>
+
+// What a slot's MQTT client is doing, as opposed to whether its network is up,
+// and the two decisions that hang off it.
+//
+// Pure logic: no Arduino, esp-mqtt or bridge dependency, so the shutdown
+// contract below is a host test rather than a hardware run.
+//
+// `client->connected()` answers "is the network up" and was being used for "is
+// there anything to stop": a client resolving DNS, negotiating TLS or waiting
+// after a failed CONNECT reports not-connected, so teardown skipped it and left
+// its task running (F04).
+
+enum class MqttClientState : uint8_t {
+  Absent = 0,    // no client object
+  Configured,    // client allocated, never started
+  Starting,      // start/reconnect requested, awaiting CONNECTED
+  Connected,     // CONNECTED received
+  Disconnected,  // started, no session (our reconnect ladder governs it)
+  Stopped,       // stop completed; the SDK task is joined and gone
+  Quarantined,   // stop failed: the SDK task was NOT joined. Never destroy,
+                 // never reuse, never free anything it still points at.
+};
+
+static inline const char* mqttClientStateName(MqttClientState s) {
+  switch (s) {
+    case MqttClientState::Absent:       return "absent";
+    case MqttClientState::Configured:   return "configured";
+    case MqttClientState::Starting:     return "starting";
+    case MqttClientState::Connected:    return "connected";
+    case MqttClientState::Disconnected: return "disconnected";
+    case MqttClientState::Stopped:      return "stopped";
+    case MqttClientState::Quarantined:  return "quarantined";
+  }
+  return "?";
+}
+
+// The client has been started and not proven stopped: it may still own a task,
+// a socket and a TLS context. Quarantined is deliberately NOT live — it is
+// worse: it may own them and we can never find out.
+static inline bool mqttClientStateIsLive(MqttClientState s) {
+  return s == MqttClientState::Starting || s == MqttClientState::Connected ||
+         s == MqttClientState::Disconnected;
+}
+
+// A connection attempt is in flight, so an event for it can still arrive.
+// Closing the transport is not enough here: PsychicMqttClient::softDisconnect()
+// returns immediately when the client is not yet connected, which means an
+// in-progress DNS/TLS/CONNECT is left to complete on its own. Cancelling one
+// requires a real stop.
+static inline bool mqttClientStateHasAttemptInFlight(MqttClientState s) {
+  return s == MqttClientState::Starting;
+}
+
+// The SDK task is known to be gone (or never existed), so the object may be
+// destroyed and anything it pointed at may be freed.
+static inline bool mqttClientStateIsProvenStopped(MqttClientState s) {
+  return s == MqttClientState::Absent || s == MqttClientState::Configured ||
+         s == MqttClientState::Stopped;
+}
+
+// The bridge's cooperative stop may only be acknowledged when EVERY client is
+// proven stopped.
+//
+// This is the invariant the whole shutdown contract rests on: the owner treats
+// the acknowledgement as proof that the task destroyed its clients, and only
+// then frees the queue and buffers and allows a restart. One client whose
+// esp_mqtt_client_stop() never completed makes that false — its SDK task may
+// still be running — so the acknowledgement must be withheld and the bridge
+// left in its unproven state, however many other slots stopped cleanly.
+static inline bool mqttStopMayBeAcknowledged(const MqttClientState* states, int count) {
+  // An empty set is trivially proven (no slots, nothing to stop); a null array
+  // with a nonzero count is a caller bug and must not read as proof.
+  if (count <= 0) return true;
+  if (states == nullptr) return false;
+  for (int i = 0; i < count; i++) {
+    if (!mqttClientStateIsProvenStopped(states[i])) return false;
+  }
+  return true;
+}

src/helpers/MQTTEffectiveConfig.h

@@ -0,0 +1,213 @@
+#pragma once
+
+#include <stdint.h>
+#include <string.h>
+
+// The complete, owned description of what a slot's MQTT client should be
+// configured with, plus the decision of whether the existing SDK client can be
+// reconfigured in place or has to be recreated.
+//
+// Pure logic: no Arduino, esp-mqtt or PsychicMqttClient dependency, so every
+// transition in the table below is exercised on the host
+// (test/test_mqtt_effective_config).
+//
+// Why this exists (F02/F03). The bridge used to apply configuration field by
+// field, skipping the cleanup of stale fields whenever
+// `slot.initial_connect_done` was false — which `teardownSlot()` had just
+// cleared, so normal live reconfiguration always skipped it. On hardware that
+// sent the previous broker's JWT username in the CONNECT to a newly configured
+// anonymous endpoint. Two rules fix it:
+//
+//   1. Every field is written on every apply. "Absent" is a value that gets
+//      written (the empty string), never an omission — IDF's
+//      `esp_mqtt_set_if_config()` treats NULL as "leave unchanged", so a
+//      cleared wrapper pointer cannot erase an SDK-held credential, while an
+//      empty string both overwrites it and leaves the CONNECT's username flag
+//      clear (verified on an ESP32-S3 against a local broker, IDF 4.4).
+//   2. Where a field cannot be overwritten safely, the client is recreated.
+//      That is the exception, not the rule: esp-mqtt's stop/start cycle is the
+//      fork's documented internal-heap fragmentation driver, so recreation must
+//      never happen on the reconnect or token-renewal path.
+
+// Matches MQTTSlot::broker_uri so an effective config can hold its own copy.
+#define MQTT_EFFECTIVE_URI_MAX 128
+
+enum class MqttTransport : uint8_t {
+  Unknown = 0,
+  Tcp,   // mqtt://
+  Tls,   // mqtts://
+  Ws,    // ws://
+  Wss,   // wss://
+};
+
+enum class MqttTrust : uint8_t {
+  Plaintext = 0,  // unencrypted transport: no server verification exists
+  Bundle,         // shared CA bundle attach callback
+  PemCert,        // one specific CA certificate
+};
+
+enum class MqttAuth : uint8_t {
+  None = 0,
+  UserPass,
+  Jwt,
+};
+
+struct MqttEffectiveConfig {
+  char          uri[MQTT_EFFECTIVE_URI_MAX] = {0};
+  MqttTransport transport = MqttTransport::Unknown;
+  MqttTrust     trust = MqttTrust::Plaintext;
+  MqttAuth      auth = MqttAuth::None;
+  // Borrowed pointers with a lifetime at least as long as the client's: preset
+  // certificates live in flash, credentials in the slot/bridge storage.
+  const char*   pem = nullptr;
+  const char*   username = nullptr;   // nullptr == absent, written as ""
+  const char*   password = nullptr;
+  uint16_t      buffer_size = 0;
+  uint16_t      keepalive = 0;
+  bool          valid = false;        // false until successfully built
+};
+
+// Client certificates are deliberately absent: `setClientCertificate()` has no
+// caller in the firmware (no broker preset uses mutual TLS), so modelling it
+// here would be untested configuration surface. Adding mutual TLS means adding
+// it to this struct and to the recreate decision below.
+
+static inline MqttTransport mqttTransportFromUri(const char* uri) {
+  if (uri == nullptr) return MqttTransport::Unknown;
+  if (strncmp(uri, "mqtts://", 8) == 0) return MqttTransport::Tls;
+  if (strncmp(uri, "mqtt://", 7) == 0)  return MqttTransport::Tcp;
+  if (strncmp(uri, "wss://", 6) == 0)   return MqttTransport::Wss;
+  if (strncmp(uri, "ws://", 5) == 0)    return MqttTransport::Ws;
+  return MqttTransport::Unknown;
+}
+
+static inline bool mqttTransportIsEncrypted(MqttTransport t) {
+  return t == MqttTransport::Tls || t == MqttTransport::Wss;
+}
+
+static inline const char* mqttTransportName(MqttTransport t) {
+  switch (t) {
+    case MqttTransport::Tcp: return "mqtt";
+    case MqttTransport::Tls: return "mqtts";
+    case MqttTransport::Ws:  return "ws";
+    case MqttTransport::Wss: return "wss";
+    default:                 return "?";
+  }
+}
+
+// A credential the SDK must be told to forget is written as "" rather than
+// left NULL. See rule 1 above.
+static inline const char* mqttFieldOrEmpty(const char* v) { return v ? v : ""; }
+
+// Builds an effective config, deriving the transport from the URI and
+// normalising the trust policy against it: an unencrypted transport verifies
+// nothing, whatever certificate material was requested, so recording anything
+// else would make a plaintext endpoint look like a trust change away from a
+// verified one (and vice versa). Returns false for an empty or unrecognised
+// URI, leaving *out invalid.
+static inline bool mqttBuildEffectiveConfig(const char* uri,
+                                            MqttAuth auth,
+                                            const char* username,
+                                            const char* password,
+                                            MqttTrust requested_trust,
+                                            const char* pem,
+                                            uint16_t buffer_size,
+                                            uint16_t keepalive,
+                                            MqttEffectiveConfig* out) {
+  if (out == nullptr) return false;
+  *out = MqttEffectiveConfig();
+  if (uri == nullptr || uri[0] == '\0') return false;
+  if (strlen(uri) >= MQTT_EFFECTIVE_URI_MAX) return false;
+
+  const MqttTransport transport = mqttTransportFromUri(uri);
+  if (transport == MqttTransport::Unknown) return false;
+
+  strncpy(out->uri, uri, MQTT_EFFECTIVE_URI_MAX - 1);
+  out->uri[MQTT_EFFECTIVE_URI_MAX - 1] = '\0';
+  out->transport = transport;
+  out->auth = auth;
+  out->username = username;
+  out->password = password;
+  out->buffer_size = buffer_size;
+  out->keepalive = keepalive;
+
+  if (!mqttTransportIsEncrypted(transport)) {
+    out->trust = MqttTrust::Plaintext;
+    out->pem = nullptr;
+  } else if (requested_trust == MqttTrust::PemCert && pem != nullptr) {
+    out->trust = MqttTrust::PemCert;
+    out->pem = pem;
+  } else if (requested_trust == MqttTrust::Bundle) {
+    out->trust = MqttTrust::Bundle;
+  } else {
+    // Encrypted transport with no usable trust material. Recorded as Plaintext
+    // trust so it is never confused with a verified policy; whether to connect
+    // at all is the caller's decision.
+    out->trust = MqttTrust::Plaintext;
+  }
+
+  out->valid = true;
+  return true;
+}
+
+struct MqttRecreateDecision {
+  bool        recreate = false;
+  const char* reason = "reuse";   // static literal, for logs and tests
+};
+
+// Whether the SDK client that `applied` was written to can be reconfigured in
+// place to reach `desired`.
+//
+//   transport (scheme) change   -> recreate. One `wss`->`mqtt` transition was
+//       observed working by reuse on hardware, but that proves one direction
+//       once, not that no websocket transport state survives the switch; the
+//       retained-`frame_state` bug class is real and unfixed upstream even in
+//       IDF 5.3, and this transition only happens when an operator changes the
+//       endpoint.
+//   trust change                -> recreate. `cert_pem` cannot be cleared by
+//       writing "" (an empty PEM is a parse failure, not "no certificate"), and
+//       the bundle attach is a function pointer whose set_config semantics are
+//       not established on this build. Verification policy is the one field
+//       where a stale value silently weakens security.
+//   capacity growth             -> recreate. IDF 4.4 allocates the MQTT buffers
+//       in `esp_mqtt_client_init()` and `esp_mqtt_set_config()` does not resize
+//       them; the wrapper's own reassembly buffer is likewise allocated once.
+//   everything else             -> reuse, including every credential and
+//       auth-mode change, because those are rewritten unconditionally.
+static inline MqttRecreateDecision mqttConfigRecreateDecision(
+    const MqttEffectiveConfig& applied,
+    const MqttEffectiveConfig& desired,
+    uint16_t allocated_buffer_size) {
+  MqttRecreateDecision d;
+  if (!desired.valid) {
+    d.recreate = false;
+    d.reason = "invalid-desired";
+    return d;
+  }
+  if (!applied.valid) {
+    // Nothing applied yet: a fresh client needs configuring, not recreating.
+    d.reason = "first-apply";
+    return d;
+  }
+  if (applied.transport != desired.transport) {
+    d.recreate = true;
+    d.reason = "transport-change";
+    return d;
+  }
+  if (applied.trust != desired.trust) {
+    d.recreate = true;
+    d.reason = "trust-change";
+    return d;
+  }
+  if (desired.trust == MqttTrust::PemCert && applied.pem != desired.pem) {
+    d.recreate = true;
+    d.reason = "ca-cert-change";
+    return d;
+  }
+  if (desired.buffer_size > allocated_buffer_size) {
+    d.recreate = true;
+    d.reason = "buffer-growth";
+    return d;
+  }
+  return d;
+}

src/helpers/MQTTLifecycle.h

@@ -40,6 +40,13 @@ enum class State : uint8_t {
   Running,
   StopRequested,
   Stopping,
+  // A requested stop passed its deadline without an acknowledgement. The MQTT
+  // task may still be alive inside mbedTLS or holding the SDK's API mutex, so
+  // NOTHING it can reach may be released and the bridge may not restart. This
+  // is deliberately not Stopped: releasing on a stop we cannot prove is the
+  // reviewed teardown-heap-panic path (F01). Appended so the values above keep
+  // their numbering.
+  StopUnproven,
 };
 
 // Events driven either by the owner (loop task) or by the MQTT task reporting
@@ -135,12 +142,17 @@ inline Result apply(State s, Event e) {
           r.accepted = true;
           break;
         case Event::StopAcknowledged:
-        case Event::StopTimedOut:
           r.next = State::Stopped;
           r.effects.release_resources = true;
           r.effects.ota_release = true;
           r.accepted = true;
           break;
+        case Event::StopTimedOut:
+          // Unblock the OTA barrier (so it aborts) but release nothing.
+          r.next = State::StopUnproven;
+          r.effects.ota_release = true;
+          r.accepted = true;
+          break;
         default:
           // Duplicate StopRequested is a no-op (idempotent stop).
           break;
@@ -150,16 +162,33 @@ inline Result apply(State s, Event e) {
     case State::Stopping:
       switch (e) {
         case Event::StopAcknowledged:
-        case Event::StopTimedOut:
           r.next = State::Stopped;
           r.effects.release_resources = true;
           r.effects.ota_release = true;
           r.accepted = true;
           break;
+        case Event::StopTimedOut:
+          r.next = State::StopUnproven;
+          r.effects.ota_release = true;
+          r.accepted = true;
+          break;
         default:
           break;
       }
       break;
+
+    case State::StopUnproven:
+      // A late acknowledgement proves exactly what a timely one proves: the
+      // task published it only after tearing its clients down. The deadline was
+      // an availability policy, not a statement about what the ack means — so
+      // honour it, release, and let the bridge be restartable again. A start
+      // before that proof is refused (see mayRestart).
+      if (e == Event::StopAcknowledged) {
+        r.next = State::Stopped;
+        r.effects.release_resources = true;
+        r.accepted = true;
+      }
+      break;
   }
   return r;
 }
@@ -182,6 +211,11 @@ inline bool mayTouchOwnedState(State s) { return s != State::Stopped; }
 // A restart (begin()) is safe only from a completed stop.
 inline bool mayRestart(State s) { return s == State::Stopped; }
 
+// True while a stop has passed its deadline unproven: the owner must keep every
+// resource the MQTT task can reach (queue, buffers, clients, its own task) and
+// must not restart. Cleared only by a late StopAcknowledged.
+inline bool isStopUnproven(State s) { return s == State::StopUnproven; }
+
 inline bool isStopInProgress(State s) {
   return s == State::StopRequested || s == State::Stopping;
 }
@@ -193,6 +227,7 @@ inline const char* stateName(State s) {
     case State::Running:       return "Running";
     case State::StopRequested: return "StopRequested";
     case State::Stopping:      return "Stopping";
+    case State::StopUnproven:  return "StopUnproven";
   }
   return "?";
 }
@@ -264,10 +299,16 @@ class Coordinator {
   bool isStopInProgress() const {
     return MQTTLifecycle::isStopInProgress(_state);
   }
-  // A restart is safe from a completed stop. A stop that reached Stopped via
-  // the timeout fallback still allows restart (the bridge is down); only OTA
-  // flashing is withheld after a dirty stop.
+  // A restart is safe only from a PROVEN stop. An unproven stop keeps the
+  // bridge down until the task acknowledges late (or the node reboots): a new
+  // start is not proof that the previous clients stopped, and starting on top
+  // of them is what the review refused to approve. Because a start is
+  // impossible while unproven, clearing the dirty latch on start (below) can no
+  // longer erase an unproven stop's OTA block.
   bool mayRestart() const { return MQTTLifecycle::mayRestart(_state); }
+  // The owner polls this to decide whether it may release/restart, and calls
+  // onTaskStopped() again if the task acknowledges late.
+  bool isStopUnproven() const { return MQTTLifecycle::isStopUnproven(_state); }
   // OTA erase/write is permitted only after a CLEAN stop. A timed-out stop
   // leaves ownership uncertain, so flashing stays blocked until a clean
   // start/stop cycle clears the latch.

src/helpers/MQTTPrefsSerializer.h

@@ -4,6 +4,7 @@
 #include <helpers/MQTTObserverValidation.h>
 #include <helpers/MQTTPresets.h>
 #include <helpers/MQTTPrefsStorage.h>
+#include <helpers/WifiPowerSavePolicy.h>
 
 #ifdef WITH_MQTT_BRIDGE
 
@@ -50,7 +51,7 @@ class MQTTPrefsSerializer : public ConfigSerializer {
   public:
     explicit WifiPrefs(MQTTPrefs* prefs) : _prefs(prefs), _power_save(prefs->wifi_power_save) {}
     bool apply(bool* repaired) {
-      if (_power_save < 0 || _power_save > 2) {
+      if (_power_save < 0 || _power_save > WifiPowerSavePolicy::kMaxStoredValue) {
         _power_save = 1;
         *repaired = true;
       }

src/helpers/NtpValidation.h

@@ -0,0 +1,141 @@
+#pragma once
+
+#include <stdint.h>
+#include <string.h>
+
+// Client-side NTPv4 request building and response validation.
+//
+// Pure logic: no Arduino, WiFi or UDP headers, so every rejection case is
+// host-testable. The caller owns the socket and supplies the datagram plus
+// whether it arrived from the server it asked (see MQTTBridge::probeNtpServer).
+//
+// This exists because the NTPClient library accepted *any* non-empty datagram
+// on its fixed local port as time: it ignored the read length, the mode, the
+// stratum, the leap indicator and the request/response correlation, so a
+// one-byte packet from anywhere set the clock — which then feeds JWT issuance,
+// certificate validity and the RTC. Everything below is the validation that
+// path never had.
+namespace NtpValidation {
+
+static const size_t kPacketSize = 48;
+
+// Seconds between the NTP epoch (1900-01-01) and the Unix epoch (1970-01-01).
+static const uint32_t kUnixEpochOffset = 2208988800UL;
+// Wrap distance for NTP era 1, which starts 2036-02-07: era-1 seconds count
+// from there, so a small NTP seconds field is a *future* time, not a past one.
+static const uint32_t kEra1UnixOffset = 2085978496UL;
+
+enum Reject : uint8_t {
+  kAccepted = 0,
+  kShortPacket,       // fewer than 48 bytes: not an NTP response at all
+  kWrongSource,       // not from the address/port we queried
+  kBadVersion,        // NTP version outside 3..4
+  kBadMode,           // not mode 4 (server)
+  kLeapAlarm,         // LI=3: the server itself is unsynchronised
+  kBadStratum,        // 0 (kiss-of-death) or >= 16 (unsynchronised)
+  kOriginMismatch,    // did not echo the transmit timestamp we sent
+  kZeroTransmit,      // no transmit timestamp
+  kEpochTooEarly,     // before the caller's floor
+  kEpochTooLate,      // implausibly far in the future
+};
+
+// The transmit timestamp we send and the server must echo back in its originate
+// field. Random, so an off-path sender cannot answer a query it never saw.
+struct Nonce {
+  uint32_t seconds;
+  uint32_t fraction;
+};
+
+struct Result {
+  Reject   reject;
+  uint32_t epoch;     // Unix seconds; only meaningful when reject == kAccepted
+};
+
+static inline uint32_t readU32(const uint8_t* p) {
+  return ((uint32_t)p[0] << 24) | ((uint32_t)p[1] << 16) |
+         ((uint32_t)p[2] << 8) | (uint32_t)p[3];
+}
+
+static inline void writeU32(uint8_t* p, uint32_t v) {
+  p[0] = (uint8_t)(v >> 24);
+  p[1] = (uint8_t)(v >> 16);
+  p[2] = (uint8_t)(v >> 8);
+  p[3] = (uint8_t)v;
+}
+
+// LI=0 (no warning), VN=4, Mode=3 (client). The NTPClient request claimed LI=3,
+// advertising an alarm condition it had no business asserting.
+static inline void buildRequest(uint8_t out[kPacketSize], Nonce nonce) {
+  memset(out, 0, kPacketSize);
+  out[0] = 0x23;
+  out[1] = 0;      // stratum: unspecified
+  out[2] = 6;      // poll interval
+  out[3] = 0xEC;   // precision
+  writeU32(out + 40, nonce.seconds);     // transmit timestamp = our nonce
+  writeU32(out + 44, nonce.fraction);
+}
+
+// Unix time for an NTP seconds field, handling the 2036 era rollover instead of
+// wrapping into 1900 the way an unchecked subtraction does.
+static inline uint32_t unixFromNtpSeconds(uint32_t ntp_seconds) {
+  return ntp_seconds >= kUnixEpochOffset ? (ntp_seconds - kUnixEpochOffset)
+                                         : (ntp_seconds + kEra1UnixOffset);
+}
+
+// `source_matches` is the caller's check that the datagram came from the
+// address and port it queried; it is a parameter rather than a lookup so this
+// stays free of socket types.
+static inline Result validate(const uint8_t* data, size_t len, bool source_matches,
+                              Nonce nonce, uint32_t min_epoch, uint32_t max_epoch) {
+  Result r;
+  r.epoch = 0;
+
+  if (data == nullptr || len < kPacketSize) { r.reject = kShortPacket; return r; }
+  if (!source_matches)                      { r.reject = kWrongSource; return r; }
+
+  const uint8_t li = (uint8_t)((data[0] >> 6) & 0x03);
+  const uint8_t version = (uint8_t)((data[0] >> 3) & 0x07);
+  const uint8_t mode = (uint8_t)(data[0] & 0x07);
+  const uint8_t stratum = data[1];
+
+  if (version < 3 || version > 4) { r.reject = kBadVersion; return r; }
+  if (mode != 4)                  { r.reject = kBadMode; return r; }
+  if (li == 3)                    { r.reject = kLeapAlarm; return r; }
+  // Stratum 0 carries a kiss-of-death code (RATE/DENY/RSTR) rather than time;
+  // 16 and above means the server has no time to give.
+  if (stratum == 0 || stratum >= 16) { r.reject = kBadStratum; return r; }
+
+  if (readU32(data + 24) != nonce.seconds ||
+      readU32(data + 28) != nonce.fraction) { r.reject = kOriginMismatch; return r; }
+
+  const uint32_t transmit_seconds = readU32(data + 40);
+  if (transmit_seconds == 0) { r.reject = kZeroTransmit; return r; }
+
+  const uint32_t epoch = unixFromNtpSeconds(transmit_seconds);
+  if (epoch < min_epoch) { r.reject = kEpochTooEarly; return r; }
+  if (epoch > max_epoch) { r.reject = kEpochTooLate; return r; }
+
+  r.reject = kAccepted;
+  r.epoch = epoch;
+  return r;
+}
+
+// Short enough for a LoRa-bounded diagnostic reply.
+static inline const char* rejectReason(Reject reject) {
+  switch (reject) {
+    case kAccepted:       return "ok";
+    case kShortPacket:    return "short packet";
+    case kWrongSource:    return "wrong source";
+    case kBadVersion:     return "bad version";
+    case kBadMode:        return "not a server reply";
+    case kLeapAlarm:      return "server unsynced";
+    case kBadStratum:     return "bad stratum";
+    case kOriginMismatch: return "unsolicited reply";
+    case kZeroTransmit:   return "no timestamp";
+    case kEpochTooEarly:  return "time too old";
+    case kEpochTooLate:   return "time too far ahead";
+    default:              return "invalid";
+  }
+}
+
+} // namespace NtpValidation

src/helpers/RadioWatchdog.h

@@ -0,0 +1,116 @@
+#pragma once
+
+#include <stdint.h>
+
+// Decides when the observer radio watchdog should kick a radio that is parked in
+// RX but hearing nothing (see Dispatcher::loop()).
+//
+// Pure logic: no Arduino, radio or mesh headers. The caller feeds the raw 32-bit
+// activity timestamps it can see (last receive, last radio interrupt, last
+// successful transmit) plus its millis() counter.
+//
+// Why this is not a max() of those three timestamps: millis() wraps every ~49.7
+// days, and a pre-wrap timestamp is numerically larger than every fresh
+// post-wrap one. Picking the largest therefore latches onto an old transmit and
+// reports the radio silent while it is receiving continuously, resetting a
+// healthy radio once per watchdog interval until that timestamp stops winning.
+//
+// Instead the caller's timestamps are only ever compared with their own previous
+// values: any change is fresh activity, stamped against a monotonic 64-bit clock
+// extended from millis(). Nothing downstream has a rollover case, and evidence
+// older than one 32-bit cycle stays old instead of aliasing back to "recent".
+//
+// How far a timestamp may run behind the previous one and still count as an
+// out-of-order reading rather than a very long forward gap (same rule as
+// RadioActivityWindow).
+#define RADIO_WATCHDOG_BACKSTEP_TOLERANCE_MS 5000UL
+
+struct RadioWatchdogDecision {
+  bool     recover;      // trip now: idle + restart receive
+  bool     measurable;   // false until some activity has been observed
+  uint64_t silent_ms;    // age of the most recent activity (0 when !measurable)
+};
+
+class RadioWatchdog {
+public:
+  RadioWatchdog() { reset(0); }
+
+  void reset(uint32_t now_ms) {
+    _now_ms = now_ms;
+    _last_input_ms = now_ms;
+    _seeded = false;
+    _has_activity = false;
+    _last_recv = _last_irq = _last_tx = 0;
+    _last_activity_ms = now_ms;
+    _recovered = false;
+    _last_recovery_ms = 0;
+  }
+
+  // Call once per loop with the current activity timestamps. `in_recv_mode` and
+  // `watchdog_ms` (0 disables) gate the decision; the silence measurement is
+  // updated either way so a disabled or transmitting radio does not accumulate
+  // phantom silence.
+  RadioWatchdogDecision update(uint32_t now_ms, bool in_recv_mode, uint32_t watchdog_ms,
+                               uint32_t last_recv, uint32_t last_irq, uint32_t last_tx) {
+    tick(now_ms);
+
+    if (!_seeded) {
+      // First observation: the timestamps carry activity from before this
+      // tracker existed, so date it to now rather than trusting a raw value we
+      // cannot place on our own clock. Worst case that delays the first
+      // possible trip by one watchdog interval.
+      _seeded = true;
+      _has_activity = (last_recv != 0 || last_irq != 0 || last_tx != 0);
+      _last_activity_ms = _now_ms;
+    } else if (last_recv != _last_recv || last_irq != _last_irq || last_tx != _last_tx) {
+      _has_activity = true;
+      _last_activity_ms = _now_ms;
+    }
+    _last_recv = last_recv;
+    _last_irq = last_irq;
+    _last_tx = last_tx;
+
+    RadioWatchdogDecision d;
+    d.recover = false;
+    d.measurable = _has_activity;
+    d.silent_ms = _has_activity ? (_now_ms - _last_activity_ms) : 0;
+
+    if (!_has_activity || !in_recv_mode || watchdog_ms == 0) return d;
+    if (d.silent_ms <= watchdog_ms) return d;
+    // One recovery per watchdog interval: a radio that stays silent through a
+    // reset must not be reset every loop.
+    if (_recovered && (_now_ms - _last_recovery_ms) <= watchdog_ms) return d;
+
+    d.recover = true;
+    return d;
+  }
+
+  // Call after acting on a decision with recover set.
+  void noteRecovery() {
+    _recovered = true;
+    _last_recovery_ms = _now_ms;
+  }
+
+  uint64_t nowMs() const { return _now_ms; }
+
+private:
+  // Extends the caller's 32-bit millis() to a monotonic 64-bit clock.
+  void tick(uint32_t now_ms) {
+    uint32_t delta = now_ms - _last_input_ms;
+    if (delta > 0x80000000u &&
+        (uint32_t)(_last_input_ms - now_ms) <= RADIO_WATCHDOG_BACKSTEP_TOLERANCE_MS) {
+      return;   // out-of-order reading: no time has passed
+    }
+    _now_ms += delta;
+    _last_input_ms = now_ms;
+  }
+
+  uint64_t _now_ms;
+  uint32_t _last_input_ms;
+  bool     _seeded;
+  bool     _has_activity;
+  uint32_t _last_recv, _last_irq, _last_tx;
+  uint64_t _last_activity_ms;
+  bool     _recovered;
+  uint64_t _last_recovery_ms;
+};

src/helpers/WifiPowerSavePolicy.h

@@ -0,0 +1,76 @@
+#pragma once
+
+#include <stdint.h>
+#include <string.h>
+
+// The single mapping between the stored `wifi.powersave` preference, its CLI
+// name, and the IDF power-save mode.
+//
+// Pure lookup so host tests can hold startup, reconnect, CLI and web config to
+// one table: they used to map the same stored value differently, so a node set
+// to `min` silently ran with power save off after its first reconnect while
+// `get wifi.powersave` still said min.
+//
+// Stored values are fleet state — never renumber them. 0 was the shipped
+// default from 2026-01-02 to 2026-03-28, and every association path ran it with
+// power save off, so it keeps meaning `none`; an explicit `min` is stored as 3.
+// Older firmware repairs 3 to `none` when it loads /mqtt.json.
+namespace WifiPowerSavePolicy {
+
+enum StoredValue : uint8_t {
+  kLegacyDefault = 0,   // WIFI_PS_NONE: the old default, never an operator choice
+  kNone = 1,            // WIFI_PS_NONE  (default)
+  kMax  = 2,            // WIFI_PS_MAX_MODEM
+  kMin  = 3,            // WIFI_PS_MIN_MODEM
+  kMaxStoredValue = kMin,
+};
+
+// Mirrors wifi_ps_type_t. MQTTBridge.cpp static_asserts these against the SDK.
+enum Mode : uint8_t {
+  kModeNone     = 0,
+  kModeMinModem = 1,
+  kModeMaxModem = 2,
+};
+
+// Anything outside the known range reads as the default rather than as the
+// lowest-numbered mode, so a corrupt byte cannot silently enable modem sleep.
+static inline Mode modeFor(uint8_t stored) {
+  switch (stored) {
+    case kMin:  return kModeMinModem;
+    case kMax:  return kModeMaxModem;
+    case kNone: return kModeNone;
+    default:    return kModeNone;
+  }
+}
+
+static inline const char* nameFor(uint8_t stored) {
+  switch (stored) {
+    case kMin:  return "min";
+    case kMax:  return "max";
+    case kNone: return "none";
+    default:    return "none";
+  }
+}
+
+// Parses a CLI argument, which may be followed by trailing text (the observer
+// setters take the rest of the command line). Returns false and leaves *out
+// untouched for anything else.
+static inline bool parseName(const char* value, uint8_t* out) {
+  if (value == nullptr || out == nullptr) return false;
+  static const struct { const char* name; uint8_t stored; } kNames[] = {
+    { "min",  kMin  },
+    { "none", kNone },
+    { "max",  kMax  },
+  };
+  for (unsigned i = 0; i < sizeof(kNames) / sizeof(kNames[0]); i++) {
+    const size_t len = strlen(kNames[i].name);
+    if (strncmp(value, kNames[i].name, len) == 0 &&
+        (value[len] == '\0' || value[len] == ' ')) {
+      *out = kNames[i].stored;
+      return true;
+    }
+  }
+  return false;
+}
+
+} // namespace WifiPowerSavePolicy

src/helpers/bridges/MQTTBridge.h

@@ -4,7 +4,6 @@
 #include "helpers/bridges/BridgeBase.h"
 #include <PsychicMqttClient.h>
 #include <WiFi.h>
-#include <NTPClient.h>
 #include <WiFiUdp.h>
 #include <Timezone.h>
 #include "helpers/JWTHelper.h"
@@ -12,6 +11,8 @@
 #include "helpers/MQTTPresets.h"
 #include "helpers/MQTTLifecycle.h"
 #include "helpers/AlertFaultPolicy.h"
+#include "helpers/MQTTEffectiveConfig.h"
+#include "helpers/MQTTClientState.h"
 #include <atomic>
 
 #ifdef WITH_SNMP
@@ -92,9 +93,40 @@ public:
 private:
   static const size_t AUTH_TOKEN_SIZE = 768;
 
+  // Every client gets buffers sized for a JWT CONNECT (frame + 768-byte token),
+  // on every board: the SDK fixes the allocation at client init and does not
+  // resize it, so a per-slot size made the desired and allocated capacities
+  // disagree (F03).
+  static const uint16_t kMqttClientBufferSize = 896;
+
+  // NTP acceptance bounds. A reply outside them is rejected outright rather
+  // than allowed to set the clock, the RTC and every JWT minted afterwards.
+  static const uint16_t kNtpPort = 123;
+  static const uint32_t kNtpProbeTimeoutMs = 1000;
+  static const uint32_t kNtpMinValidEpoch = 1767225600UL;  // 2026-01-01 UTC
+  static const uint32_t kNtpMaxValidEpoch = 4102444800UL;  // 2100-01-01 UTC
+
+  // Per-slot SDK client state and the shutdown contract live in
+  // MQTTClientState.h so both are host-testable; ClientState is an alias so the
+  // bridge code reads naturally.
+  using ClientState = MqttClientState;
+  static const char* clientStateName(ClientState s) { return mqttClientStateName(s); }
+  static bool clientStateIsLive(ClientState s) { return mqttClientStateIsLive(s); }
+
   // Connection slot - each slot holds one MQTT connection
   struct MQTTSlot {
     PsychicMqttClient* client;
+    ClientState client_state;
+    // What this client was last successfully configured with, and the buffer
+    // capacity it actually owns (the SDK fixes that at client init). Together
+    // they answer "can the next configuration be applied in place?" — the
+    // question `initial_connect_done` was standing in for, wrongly.
+    MqttEffectiveConfig applied_config;
+    uint16_t allocated_buffer_size;
+    // Bumped on every start/stop. Only used for diagnostics and log lines: the
+    // accept/reject decision for a late callback is made on client_state, which
+    // the bridge task owns.
+    uint32_t generation;
     const MQTTPresetDef* preset;    // Points to MQTT_PRESETS[] entry, nullptr for custom/none
     bool enabled;                   // true when preset is not "none"
     bool connected;                 // Updated in callbacks
@@ -110,7 +142,14 @@ private:
     // esp-mqtt re-reads it whenever a later connect() re-applies a dirtied config.
     // Freed only alongside the client in destroySlotClients().
     char* auth_token;               // nullptr or empty string = no valid token
+    // Two expiries, deliberately. token_expires_at describes the token in the
+    // buffer (minting updates it immediately); applied_token_expires_at
+    // describes the credential the CONNECTION is actually using, and only
+    // advances when a connect/reconnect has carried it. The renewal decision
+    // reads the applied one, so a renewal whose bounce failed stays due and is
+    // retried instead of looking complete (F06).
     unsigned long token_expires_at;
+    unsigned long applied_token_expires_at;
     unsigned long last_token_renewal;
 
     // Custom broker settings (only used when preset_name is "custom")
@@ -135,6 +174,10 @@ private:
     int32_t last_tls_err;           // esp_tls_last_esp_err (0 = no error)
     int32_t last_tls_stack_err;     // mbedTLS stack error
     int last_sock_errno;            // socket errno
+    // CONNACK return code from the last broker refusal (0 = none). A refusal is
+    // not a transport failure, and without this the diag shows a slot with wrong
+    // credentials as an unexplained disconnect.
+    uint8_t last_connack_code;
     unsigned long last_error_time;  // millis() of last error
     uint32_t disconnect_count;      // Number of disconnect callbacks since boot
     unsigned long first_disconnect_time; // millis() of first disconnect after boot
@@ -212,9 +255,9 @@ private:
   #endif
   int _queue_count;  // Protected by queue operations or mutex
 
-  // NTP time sync
+  // NTP time sync. The socket is opened per probe and closed again (see
+  // probeNtpServer); nothing listens between syncs.
   WiFiUDP _ntp_udp;
-  NTPClient _ntp_client;
   unsigned long _last_ntp_sync;
   bool _ntp_synced;
   bool _ntp_sync_pending;  // Flag to trigger NTP sync from loop() instead of event handler
@@ -242,7 +285,7 @@ private:
   volatile bool _status_publish_pending[RUNTIME_MQTT_SLOTS];
 
   // CLI-requested forced NTP sync, marshalled onto the MQTT task (Core 0).
-  // All NTP I/O (_ntp_client, configTime) must run on Core 0; the CLI thread
+  // All NTP I/O must run on Core 0; the CLI thread
   // (Core 1) sets _ntp_force_requested and blocks in requestForcedNtpSync()
   // until the task publishes the outcome via _ntp_force_result/_ntp_force_done.
   // Single-requester assumption: CLI commands are serialized, so at most one
@@ -253,7 +296,7 @@ private:
 
   // CLI-requested NTP connectivity diagnostic, marshalled onto the MQTT task (Core 0)
   // with the same handshake as the forced sync. Probe-only: it queries each server and
-  // records the reported time but never calls configTime()/setCurrentTime(), so the
+  // records the reported time but never sets the system clock or the RTC, so the
   // system clock is left untouched. Results are written by the task and read by the CLI
   // thread once _ntp_diag_done is set.
   volatile bool _ntp_diag_requested;
@@ -262,19 +305,30 @@ private:
     char     server[64];
     bool     ok;
     uint32_t epoch;  // server-reported UTC epoch when ok
+    // Why a probe failed, as a static literal ("DNS failed", "unsolicited
+    // reply", ...). Previously every failure looked alike, and a name that never
+    // resolved could be credited with another server's reply.
+    const char* why;
   };
   NtpDiagResult _ntp_diag_results[kMaxNtpServers];
   int _ntp_diag_count;
 
-  // Cooperative-shutdown handshake (Phase 5). The loop task (Core 1) raises
+  // Cooperative-shutdown handshake. The loop task (Core 1) raises
   // _stop_requested through the lifecycle Coordinator; the MQTT task (Core 0)
   // sees it, tears down its own clients on Core 0 (where the mbedTLS contexts
-  // live), sets _stop_acked LAST, and self-terminates. end() waits for the ack
-  // before freeing the queue/buffers. Plain volatile matches the existing
-  // NTP/reconfigure handshake idiom above; replacing all of these with a command
-  // channel / task notifications is explicitly deferred (see MQTT_OWNERSHIP.md).
-  volatile bool _stop_requested = false;
-  volatile bool _stop_acked = false;
+  // live), records _teardown_complete, and publishes _stop_acked from the task
+  // trampoline immediately before vTaskDelete(nullptr). end() waits for that ack
+  // before anything is freed.
+  //
+  // std::atomic, not volatile: this is a two-flag release/acquire handshake
+  // across cores, and the owner acts on it by freeing memory the other task can
+  // reach. `volatile` orders nothing and was the soak campaign's blocker #20.
+  // Publishing the ack from the trampoline is what makes it mean "this task is
+  // about to cease executing" rather than "teardown returned"; _teardown_complete
+  // gates it so an unexpected return from mqttTaskLoop() cannot claim a clean stop.
+  std::atomic<bool> _stop_requested{false};
+  std::atomic<bool> _stop_acked{false};
+  std::atomic<bool> _teardown_complete{false};
 
   // Timezone handling.
   // _timezone_storage is inline class storage (zero heap) that is reconfigured
@@ -451,24 +505,50 @@ private:
   // This avoids delete/new cycles that shed ~40 KB of mbedTLS buffers per
   // reconfigure and fragment the internal heap on non-PSRAM boards.
   bool ensureSlotClient(int index);    // Allocate this slot's persistent client + callbacks on first use
+  // Stop, destroy and re-allocate this slot's client. Only for configuration
+  // changes that cannot be applied to a live client, and only after the stop is
+  // proven — false means the client is quarantined and must not be reused.
+  bool recreateSlotClient(int index);
   bool ensureSlotAuthToken(int index); // Allocate this slot's JWT token buffer on first token creation
   void releaseSlotAuthToken(int index);// Free the token buffer (only with the client — see MQTTSlot)
-  // force=true stops each client without waiting for its DISCONNECTED event. Only the
-  // dirty-stop fallback passes it: disconnect()'s wait is unbounded, so a client already
-  // wedged in mbedTLS would block the caller — MyMesh::loop() — indefinitely.
-  void destroySlotClients(bool force = false);  // Delete all persistent clients (shutdown only)
+  // No force variant: the only caller that ever passed one was the dirty-stop
+  // fallback, and that fallback is gone (F01). A client whose stop cannot be
+  // proven is now left alone rather than force-stopped and deleted under a
+  // possibly-live SDK task.
+  void destroySlotClients();  // Delete all persistent clients (shutdown only)
   bool setupSlot(int index);           // Configure and connect the slot; false = not activated
   // Single definition of "this slot holds one of the _max_active_slots positions":
   // it is enabled and has been through a successful setupSlot(). Startup, the
   // setup-retry path, and live reconfigure all gate on these so the cap cannot be
   // exceeded by one route while another enforces it.
   int activatedSlotCount() const;
+  // Positions held by every slot except `skip` (-1 for none). canActivateSlot()
+  // excludes the candidate so a live reconfigure cannot fail its own cap check.
+  int activatedSlotCountExcluding(int skip) const;
   bool canActivateSlot(int index) const;
   // force as in destroySlotClients(): skip the unbounded wait, dirty-stop path only.
-  void teardownSlot(int index, bool force = false);  // Disconnect the slot's client (keeps the object alive)
+  // Why a slot is being torn down. The distinction is not cosmetic:
+  //  - Reconfigure: the slot is about to connect somewhere else, so the
+  //    transport must close (or an in-flight handshake could complete against
+  //    the OLD endpoint) but the esp-mqtt task should stay. Stopping it returns
+  //    its 6 KiB stack into the hole the two 16 KiB mbedTLS record buffers just
+  //    vacated, which is the fork's documented internal-heap fragmentation
+  //    driver; softDisconnect() avoids exactly that.
+  //  - Disable: the slot is going away, so the task and its transport must go
+  //    with it. Here the stop IS the point.
+  enum class TeardownReason : uint8_t { Reconfigure, Disable };
+  void teardownSlot(int index, TeardownReason reason = TeardownReason::Disable);
+  // Close a live client for a reconfigure: softDisconnect where that is enough,
+  // a real stop where an in-flight connection attempt has to be cancelled.
+  void closeLiveClientForReconfigure(int index);
+  // Stop a live client, recording Stopped (proven) or Quarantined (not joined).
+  void stopSlotClient(int index);
   // Reconnect a slot, starting it instead when the client is stopped (reconnect() is a
   // no-op on a stopped client). See the definition.
-  void reconnectSlotClient(int index);
+  // ESP_OK when the reconnect/start was accepted by the SDK. A local failure
+  // (uninitialised client, failed config transaction) is not a broker fault and
+  // must not advance this slot's backoff ladder — see maintainSlotConnection().
+  esp_err_t reconnectSlotClient(int index);
   void maintainSlotConnections();      // Maintain all slot connections (token renewal, reconnect)
   void maintainSlotConnection(int index, unsigned long now_millis, unsigned long current_time, bool time_synced, bool& reconnect_attempted, bool& teardown_attempted);
   bool createSlotAuthToken(int index); // Create/renew JWT token for a slot
@@ -505,6 +585,10 @@ private:
   bool isAnySlotConnected();
   void refreshNTP();  // Lightweight periodic NTP refresh (non-blocking)
   void runNtpDiagProbe();  // Probe every server for connectivity; never sets the clock. Core 0 only.
+  // One validated NTP exchange with one server on a fresh ephemeral socket.
+  // Core 0 only; never touches the clock. *why receives a static reason literal.
+  bool probeNtpServer(const char* server, uint32_t min_epoch,
+                      uint32_t* epoch_out, const char** why);
   // Populates dst_out/std_out with TimeChangeRules for the given IANA or
   // abbreviation string. Returns false if the string is not recognized
   // (callers should fall back to UTC). Zero-allocation.
@@ -517,6 +601,7 @@ private:
   void getClientVersion(char* buffer, size_t buffer_size) const;
   void logMemoryStatus();
   void refreshOriginFromPrefs();
+  void applyWifiPowerSave();   // one mapping, applied on every association
   // begin()/end()-scoped PSRAM buffers. Each allocation is independent so a
   // transient heap shortage degrades to the existing stack fallback instead
   // of making the bridge unusable.
@@ -651,6 +736,19 @@ public:
    *  OTA flashing is withheld until a clean start/stop cycle. Mirrors
    *  MQTTLifecycle::mayBeginFlash(); read on the loop task (Core 1). */
   bool canFlashAfterStop() const { return _lifecycle.mayBeginFlash(); }
+  // True when a stop passed its deadline without the MQTT task acknowledging:
+  // the bridge is down, nothing was released, and it will not restart until the
+  // task acknowledges late (pollLateStopAck) or the node reboots.
+  bool isStopUnproven() const { return _lifecycle.isStopUnproven(); }
+  // The unproven stop has since been acknowledged, so begin() will release the
+  // withheld resources and start. Loop task only.
+  bool stopAcknowledgedLate() const;
+  // Honours a stop acknowledgement that arrived after the deadline: releases the
+  // withheld resources and makes the bridge restartable. Loop task only.
+  void pollLateStopAck();
+  // Survives end() clearing the diagnostic singleton, so `get mqtt.status` can
+  // still explain why a stopped bridge will not come back without a reboot.
+  static bool stopUnprovenLatched();
 
   static unsigned long getWifiConnectedAtMillis();
 
@@ -692,7 +790,7 @@ public:
    *  mistyped hostname fails fast instead of blocking through the whole fallback list.
    *  Performs blocking NTP I/O and must only be called from the MQTT task (Core 0).
    *  Other tasks (e.g. the CLI on Core 1) must use requestForcedNtpSync() instead. */
-  bool syncTimeWithNTP(bool force = false, bool primary_only = false);
+  bool syncTimeWithNTP(bool force = false, bool primary_only = false, int attempts_per_server = 2);
   /** Request a forced NTP sync from another task (e.g. CLI on Core 1). Marshals the
    *  work onto the MQTT task so all NTP I/O stays on Core 0, then blocks up to
    *  timeout_ms for the result. Returns true if the sync succeeded, false on failure,

src/helpers/bridges/MQTTErrorLabels.h

@@ -0,0 +1,125 @@
+#pragma once
+
+#include <stdint.h>
+
+// Operator-facing labels for the SDK error codes the observer reports through
+// `get wifi.status`, `get mqttN.diag` and the web config panel.
+//
+// Pure lookup: no Arduino or ESP headers, so the table is host-testable. The
+// numeric values below are the installed SDK's; MQTTBridge.cpp static_asserts
+// every one of them against the real enum/#define, so a framework bump either
+// keeps these labels honest or fails the build. Codes with no entry return
+// nullptr and the caller prints the raw number — an unknown code must never be
+// given a neighbouring code's name.
+namespace MQTTErrorLabels {
+
+// esp_wifi_types.h wifi_err_reason_t. Values below 200 are 802.11 reason codes
+// forwarded from the AP; 200+ are Espressif's own local failures.
+enum WifiReason : uint16_t {
+  kWifiAuthExpire            = 2,
+  kWifiAssocExpire           = 4,
+  kWifiAssocLeave            = 8,
+  kWifi4WayHandshakeTimeout  = 15,
+  kWifiGroupCipherInvalid    = 18,
+  kWifiCipherSuiteRejected   = 24,
+  kWifiMissingAcks           = 34,
+  kWifiTimeout               = 39,
+  kWifiInvalidPmkid          = 49,
+  kWifiBssTransitionDisassoc = 12,
+  kWifiBeaconTimeout         = 200,
+  kWifiNoApFound             = 201,
+  kWifiAuthFail              = 202,
+  kWifiAssocFail             = 203,
+  kWifiHandshakeTimeout      = 204,
+  kWifiConnectionFail        = 205,
+  kWifiApTsfReset            = 206,
+  kWifiRoaming               = 207,
+  kWifiSaQueryTimeout        = 209,
+};
+
+// esp_tls_errors.h, ESP_ERR_ESP_TLS_BASE (0x8000) + offset.
+enum TlsError : int32_t {
+  kTlsCannotResolveHostname = 0x8001,
+  kTlsCannotCreateSocket    = 0x8002,
+  kTlsUnsupportedProtoFamily= 0x8003,
+  kTlsFailedConnectToHost   = 0x8004,
+  kTlsSocketSetoptFailed    = 0x8005,
+  kTlsConnectionTimeout     = 0x8006,
+  kTlsTcpClosedFin          = 0x8008,
+  kTlsMbedtlsCertPartlyOk   = 0x8010,
+  kTlsMbedtlsSetHostname    = 0x8012,
+  kTlsMbedtlsX509ParseFailed= 0x8015,
+  kTlsMbedtlsSslSetupFailed = 0x8017,
+  kTlsMbedtlsSslWriteFailed = 0x8018,
+  kTlsMbedtlsHandshakeFailed= 0x801A,
+};
+
+// MQTT 3.1.1 CONNACK return codes (esp_mqtt_error_codes.connect_return_code).
+enum ConnackCode : uint8_t {
+  kConnackAccepted        = 0,
+  kConnackBadProtocol     = 1,
+  kConnackIdRejected      = 2,
+  kConnackServerUnavail   = 3,
+  kConnackBadCredentials  = 4,
+  kConnackNotAuthorized   = 5,
+};
+
+// Short enough to sit inside a LoRa-bounded diagnostic reply next to the raw code.
+static inline const char* wifiReason(uint8_t reason) {
+  switch (reason) {
+    case kWifiAuthExpire:            return "auth expired";
+    case kWifiAssocExpire:           return "assoc expired";
+    case kWifiAssocLeave:            return "AP disconnected";
+    case kWifiBssTransitionDisassoc: return "AP steered us away";
+    case kWifi4WayHandshakeTimeout:  return "4-way handshake timeout";
+    case kWifiGroupCipherInvalid:    return "group cipher mismatch";
+    case kWifiCipherSuiteRejected:   return "cipher suite rejected";
+    case kWifiMissingAcks:           return "AP saw no acks";
+    case kWifiTimeout:               return "802.11 timeout";
+    case kWifiInvalidPmkid:          return "invalid PMKID";
+    case kWifiBeaconTimeout:         return "beacon lost";
+    case kWifiNoApFound:             return "SSID not found";
+    case kWifiAuthFail:              return "auth failed (check password)";
+    case kWifiAssocFail:             return "association failed";
+    case kWifiHandshakeTimeout:      return "handshake timeout";
+    case kWifiConnectionFail:        return "connect failed";
+    case kWifiApTsfReset:            return "AP restarted";
+    case kWifiRoaming:               return "roaming";
+    case kWifiSaQueryTimeout:        return "SA query timeout (PMF)";
+    default:                         return nullptr;
+  }
+}
+
+static inline const char* tlsError(int32_t err) {
+  switch (err) {
+    case kTlsCannotResolveHostname:  return "DNS failed";
+    case kTlsCannotCreateSocket:     return "socket error";
+    case kTlsUnsupportedProtoFamily: return "unsupported protocol family";
+    case kTlsFailedConnectToHost:    return "connect failed";
+    case kTlsSocketSetoptFailed:     return "socket setopt failed";
+    case kTlsConnectionTimeout:      return "connection timeout";
+    case kTlsTcpClosedFin:           return "server closed connection";
+    case kTlsMbedtlsCertPartlyOk:    return "cert chain partly parsed";
+    case kTlsMbedtlsSetHostname:     return "SNI hostname rejected";
+    case kTlsMbedtlsX509ParseFailed: return "cert parse failed";
+    case kTlsMbedtlsSslSetupFailed:  return "TLS setup failed";
+    case kTlsMbedtlsSslWriteFailed:  return "TLS write failed";
+    case kTlsMbedtlsHandshakeFailed: return "TLS handshake failed";
+    default:                         return nullptr;
+  }
+}
+
+// The broker answered and refused: this is what an operator needs instead of a
+// transport error, which is why the bridge keeps the CONNACK code separately.
+static inline const char* connackReason(uint8_t code) {
+  switch (code) {
+    case kConnackBadProtocol:    return "protocol rejected";
+    case kConnackIdRejected:     return "client id rejected";
+    case kConnackServerUnavail:  return "server unavailable";
+    case kConnackBadCredentials: return "bad user/password";
+    case kConnackNotAuthorized:  return "not authorized";
+    default:                     return nullptr;
+  }
+}
+
+} // namespace MQTTErrorLabels

src/helpers/esp32/WebConfigServer.cpp

@@ -14,6 +14,7 @@
 #include <helpers/MQTTPacketFilter.h>
 #include <helpers/MQTTPresets.h>
 #include <helpers/WebConfigKeys.h>
+#include <helpers/WifiPowerSavePolicy.h>
 #include <helpers/bridges/MQTTBridge.h>
 
 #include "WebConfigHtml.h"
@@ -722,8 +723,7 @@ void WebConfigServer::handleConfigGet(AsyncWebServerRequest* req) {
     JsonObject wifi = doc.createNestedObject("wifi");
     wifi["ssid"] = (const char*)_obs->wifi_ssid;
     wifi["pwd"] = _obs->wifi_password[0] ? SECRET_SENTINEL : "";
-    wifi["powersave"] = _obs->wifi_power_save == 0 ? "min"
-                        : _obs->wifi_power_save == 2 ? "max" : "none";
+    wifi["powersave"] = WifiPowerSavePolicy::nameFor(_obs->wifi_power_save);
 
     JsonObject mqtt = doc.createNestedObject("mqtt");
     mqtt["origin"] = (const char*)_obs->mqtt_origin;

test/test_mqtt_client_state/test_mqtt_client_state.cpp

@@ -0,0 +1,89 @@
+#include "helpers/MQTTClientState.h"
+
+#include <gtest/gtest.h>
+
+#include <string.h>
+#include <vector>
+
+namespace {
+
+const MqttClientState kAllStates[] = {
+    MqttClientState::Absent,       MqttClientState::Configured,
+    MqttClientState::Starting,     MqttClientState::Connected,
+    MqttClientState::Disconnected, MqttClientState::Stopped,
+    MqttClientState::Quarantined,
+};
+
+bool ack(std::vector<MqttClientState> states) {
+  return mqttStopMayBeAcknowledged(states.data(), (int)states.size());
+}
+
+} // namespace
+
+TEST(MqttClientState, LiveMeansStartedAndNotProvenStopped) {
+  EXPECT_TRUE(mqttClientStateIsLive(MqttClientState::Starting));
+  EXPECT_TRUE(mqttClientStateIsLive(MqttClientState::Connected));
+  EXPECT_TRUE(mqttClientStateIsLive(MqttClientState::Disconnected));
+
+  EXPECT_FALSE(mqttClientStateIsLive(MqttClientState::Absent));
+  EXPECT_FALSE(mqttClientStateIsLive(MqttClientState::Configured));
+  EXPECT_FALSE(mqttClientStateIsLive(MqttClientState::Stopped));
+  // Quarantined is not "live": it is worse. It may own a task and we can never
+  // find out, which is why it is not simply lumped in with the live states.
+  EXPECT_FALSE(mqttClientStateIsLive(MqttClientState::Quarantined));
+}
+
+TEST(MqttClientState, OnlyStartingHasAnAttemptInFlight) {
+  for (MqttClientState s : kAllStates) {
+    EXPECT_EQ(s == MqttClientState::Starting, mqttClientStateHasAttemptInFlight(s))
+        << mqttClientStateName(s);
+  }
+}
+
+TEST(MqttClientState, ProvenStoppedIsTheComplementOfLiveAndQuarantined) {
+  for (MqttClientState s : kAllStates) {
+    const bool proven = mqttClientStateIsProvenStopped(s);
+    EXPECT_EQ(!mqttClientStateIsLive(s) && s != MqttClientState::Quarantined, proven)
+        << mqttClientStateName(s);
+  }
+}
+
+// The invariant the shutdown contract rests on: the acknowledgement means the
+// task destroyed its clients, so it may not be published while any client's
+// stop is unproven.
+TEST(MqttClientState, StopIsAcknowledgedOnlyWhenEveryClientIsProvenStopped) {
+  EXPECT_TRUE(ack({}));   // no slots configured at all
+  EXPECT_TRUE(ack({MqttClientState::Absent, MqttClientState::Stopped,
+                   MqttClientState::Configured}));
+
+  // One quarantined client withholds the acknowledgement, whatever the others did.
+  EXPECT_FALSE(ack({MqttClientState::Quarantined}));
+  EXPECT_FALSE(ack({MqttClientState::Stopped, MqttClientState::Stopped,
+                    MqttClientState::Quarantined, MqttClientState::Absent}));
+  EXPECT_FALSE(ack({MqttClientState::Quarantined, MqttClientState::Absent,
+                    MqttClientState::Absent, MqttClientState::Absent,
+                    MqttClientState::Absent, MqttClientState::Absent}));
+
+  // So does a client that is merely still live: teardown did not finish it.
+  EXPECT_FALSE(ack({MqttClientState::Stopped, MqttClientState::Connected}));
+  EXPECT_FALSE(ack({MqttClientState::Starting}));
+  EXPECT_FALSE(ack({MqttClientState::Disconnected}));
+
+  // An empty set is trivially proven; a null array with a nonzero count is a
+  // caller bug and must never read as proof.
+  EXPECT_TRUE(mqttStopMayBeAcknowledged(nullptr, 0));
+  EXPECT_FALSE(mqttStopMayBeAcknowledged(nullptr, 3));
+}
+
+TEST(MqttClientState, EveryStateHasAName) {
+  for (MqttClientState s : kAllStates) {
+    ASSERT_NE(nullptr, mqttClientStateName(s));
+    EXPECT_GT(strlen(mqttClientStateName(s)), 0u);
+  }
+  EXPECT_STREQ("quarantined", mqttClientStateName(MqttClientState::Quarantined));
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/test_mqtt_effective_config/test_mqtt_effective_config.cpp

@@ -0,0 +1,203 @@
+#include "helpers/MQTTEffectiveConfig.h"
+
+#include <gtest/gtest.h>
+
+#include <string.h>
+
+namespace {
+
+const char kPemA[] = "-----BEGIN CERTIFICATE-----A";
+const char kPemB[] = "-----BEGIN CERTIFICATE-----B";
+const char kJwtUser[] = "v1_CC5D3CFD9C4C7B84";
+const char kToken[] = "eyJhbGciOi...";
+
+MqttEffectiveConfig build(const char* uri,
+                          MqttAuth auth = MqttAuth::None,
+                          const char* user = nullptr,
+                          const char* pass = nullptr,
+                          MqttTrust trust = MqttTrust::Plaintext,
+                          const char* pem = nullptr,
+                          uint16_t buffer = 896) {
+  MqttEffectiveConfig c;
+  mqttBuildEffectiveConfig(uri, auth, user, pass, trust, pem, buffer, 75, &c);
+  return c;
+}
+
+MqttRecreateDecision decide(const MqttEffectiveConfig& applied,
+                            const MqttEffectiveConfig& desired,
+                            uint16_t allocated = 896) {
+  return mqttConfigRecreateDecision(applied, desired, allocated);
+}
+
+} // namespace
+
+TEST(MqttEffectiveConfig, TransportComesFromTheUriScheme) {
+  EXPECT_EQ(MqttTransport::Tcp, mqttTransportFromUri("mqtt://broker:1883"));
+  EXPECT_EQ(MqttTransport::Tls, mqttTransportFromUri("mqtts://broker:8883"));
+  EXPECT_EQ(MqttTransport::Ws, mqttTransportFromUri("ws://broker/mqtt"));
+  EXPECT_EQ(MqttTransport::Wss, mqttTransportFromUri("wss://broker:443/mqtt"));
+  EXPECT_EQ(MqttTransport::Unknown, mqttTransportFromUri("broker:1883"));
+  EXPECT_EQ(MqttTransport::Unknown, mqttTransportFromUri(""));
+  EXPECT_EQ(MqttTransport::Unknown, mqttTransportFromUri(nullptr));
+
+  EXPECT_TRUE(mqttTransportIsEncrypted(MqttTransport::Tls));
+  EXPECT_TRUE(mqttTransportIsEncrypted(MqttTransport::Wss));
+  EXPECT_FALSE(mqttTransportIsEncrypted(MqttTransport::Tcp));
+  EXPECT_FALSE(mqttTransportIsEncrypted(MqttTransport::Ws));
+}
+
+TEST(MqttEffectiveConfig, BuildRejectsUnusableUris) {
+  MqttEffectiveConfig c;
+  EXPECT_FALSE(mqttBuildEffectiveConfig("", MqttAuth::None, nullptr, nullptr,
+                                        MqttTrust::Plaintext, nullptr, 896, 75, &c));
+  EXPECT_FALSE(c.valid);
+  EXPECT_FALSE(mqttBuildEffectiveConfig(nullptr, MqttAuth::None, nullptr, nullptr,
+                                        MqttTrust::Plaintext, nullptr, 896, 75, &c));
+  EXPECT_FALSE(mqttBuildEffectiveConfig("broker:1883", MqttAuth::None, nullptr, nullptr,
+                                        MqttTrust::Plaintext, nullptr, 896, 75, &c));
+
+  char too_long[MQTT_EFFECTIVE_URI_MAX + 16];
+  memset(too_long, 'x', sizeof(too_long));
+  memcpy(too_long, "mqtt://", 7);
+  too_long[sizeof(too_long) - 1] = '\0';
+  EXPECT_FALSE(mqttBuildEffectiveConfig(too_long, MqttAuth::None, nullptr, nullptr,
+                                        MqttTrust::Plaintext, nullptr, 896, 75, &c));
+}
+
+TEST(MqttEffectiveConfig, BuildOwnsItsUriCopy) {
+  char uri[64];
+  strcpy(uri, "mqtt://192.168.50.231:1883");
+  MqttEffectiveConfig c = build(uri);
+  ASSERT_TRUE(c.valid);
+
+  strcpy(uri, "mqtt://other:1883");   // slot.broker_uri is rewritten in place
+  EXPECT_STREQ("mqtt://192.168.50.231:1883", c.uri);
+}
+
+// An unencrypted transport verifies nothing, whatever certificate material was
+// requested. Recording anything else would make a plaintext endpoint look like
+// it kept a verified policy.
+TEST(MqttEffectiveConfig, TrustIsNormalisedAgainstTheTransport) {
+  MqttEffectiveConfig plain = build("mqtt://broker:1883", MqttAuth::None, nullptr, nullptr,
+                                    MqttTrust::Bundle, kPemA);
+  EXPECT_EQ(MqttTrust::Plaintext, plain.trust);
+  EXPECT_EQ(nullptr, plain.pem);
+
+  MqttEffectiveConfig bundle = build("wss://broker:443/mqtt", MqttAuth::Jwt, kJwtUser, kToken,
+                                     MqttTrust::Bundle, nullptr);
+  EXPECT_EQ(MqttTrust::Bundle, bundle.trust);
+
+  MqttEffectiveConfig pem = build("mqtts://broker:8883", MqttAuth::UserPass, "u", "p",
+                                  MqttTrust::PemCert, kPemA);
+  EXPECT_EQ(MqttTrust::PemCert, pem.trust);
+  EXPECT_EQ(kPemA, pem.pem);
+
+  // Encrypted transport with no usable material is not a verified policy.
+  MqttEffectiveConfig none = build("mqtts://broker:8883", MqttAuth::None, nullptr, nullptr,
+                                   MqttTrust::PemCert, nullptr);
+  EXPECT_EQ(MqttTrust::Plaintext, none.trust);
+  EXPECT_EQ(nullptr, none.pem);
+}
+
+// The six F02 transitions from the review, plus the one reproduced on hardware.
+// None of them needs a new client: credentials are rewritten unconditionally.
+TEST(MqttEffectiveConfig, CredentialAndAuthChangesReuseTheClient) {
+  const MqttEffectiveConfig jwt = build("wss://broker:443/mqtt", MqttAuth::Jwt, kJwtUser, kToken,
+                                        MqttTrust::Bundle, nullptr);
+  const MqttEffectiveConfig anon = build("wss://broker:443/mqtt", MqttAuth::None, nullptr, nullptr,
+                                         MqttTrust::Bundle, nullptr);
+  const MqttEffectiveConfig userpass = build("wss://broker:443/mqtt", MqttAuth::UserPass, "u", "p",
+                                             MqttTrust::Bundle, nullptr);
+  const MqttEffectiveConfig renewed = build("wss://broker:443/mqtt", MqttAuth::Jwt, kJwtUser,
+                                            "eyJhbGciOi...new", MqttTrust::Bundle, nullptr);
+
+  EXPECT_FALSE(decide(jwt, anon).recreate);        // JWT -> anonymous
+  EXPECT_FALSE(decide(userpass, anon).recreate);   // user/pass -> anonymous
+  EXPECT_FALSE(decide(anon, jwt).recreate);        // none -> configured
+  EXPECT_FALSE(decide(userpass, jwt).recreate);    // user/pass -> JWT
+  EXPECT_FALSE(decide(jwt, renewed).recreate);     // token renewal
+  EXPECT_FALSE(decide(jwt, jwt).recreate);         // ordinary reconnect
+  EXPECT_STREQ("reuse", decide(jwt, jwt).reason);
+}
+
+// Host/port/path within one scheme is an endpoint move, not a structural one.
+TEST(MqttEffectiveConfig, EndpointMoveWithinOneSchemeReusesTheClient) {
+  const MqttEffectiveConfig a = build("mqtt://192.168.50.231:1883");
+  const MqttEffectiveConfig b = build("mqtt://192.168.50.9:1884");
+  EXPECT_FALSE(decide(a, b).recreate);
+
+  const MqttEffectiveConfig wss_a = build("wss://a.example:443/mqtt", MqttAuth::Jwt, kJwtUser,
+                                          kToken, MqttTrust::Bundle, nullptr);
+  const MqttEffectiveConfig wss_b = build("wss://b.example:443/other", MqttAuth::Jwt, kJwtUser,
+                                          kToken, MqttTrust::Bundle, nullptr);
+  EXPECT_FALSE(decide(wss_a, wss_b).recreate);
+}
+
+TEST(MqttEffectiveConfig, TransportChangeForcesRecreate) {
+  const MqttEffectiveConfig wss = build("wss://broker:443/mqtt", MqttAuth::Jwt, kJwtUser, kToken,
+                                        MqttTrust::Bundle, nullptr);
+  const MqttEffectiveConfig tcp = build("mqtt://192.168.50.231:1883");
+  const MqttEffectiveConfig tls = build("mqtts://broker:8883", MqttAuth::UserPass, "u", "p",
+                                        MqttTrust::Bundle, nullptr);
+  const MqttEffectiveConfig ws = build("ws://broker/mqtt");
+
+  EXPECT_TRUE(decide(wss, tcp).recreate);
+  EXPECT_STREQ("transport-change", decide(wss, tcp).reason);
+  EXPECT_TRUE(decide(tcp, wss).recreate);
+  EXPECT_TRUE(decide(wss, tls).recreate);   // both encrypted, still a scheme change
+  EXPECT_TRUE(decide(tcp, ws).recreate);    // both plaintext, still a scheme change
+}
+
+TEST(MqttEffectiveConfig, TrustChangeForcesRecreate) {
+  const MqttEffectiveConfig bundle = build("mqtts://broker:8883", MqttAuth::None, nullptr, nullptr,
+                                           MqttTrust::Bundle, nullptr);
+  const MqttEffectiveConfig pem_a = build("mqtts://broker:8883", MqttAuth::None, nullptr, nullptr,
+                                          MqttTrust::PemCert, kPemA);
+  const MqttEffectiveConfig pem_b = build("mqtts://broker:8883", MqttAuth::None, nullptr, nullptr,
+                                          MqttTrust::PemCert, kPemB);
+
+  EXPECT_TRUE(decide(bundle, pem_a).recreate);
+  EXPECT_STREQ("trust-change", decide(bundle, pem_a).reason);
+  EXPECT_TRUE(decide(pem_a, bundle).recreate);
+  EXPECT_TRUE(decide(pem_a, pem_b).recreate);
+  EXPECT_STREQ("ca-cert-change", decide(pem_a, pem_b).reason);
+  EXPECT_FALSE(decide(pem_a, pem_a).recreate);
+}
+
+// IDF 4.4 allocates the MQTT buffers at client init and does not resize them,
+// and the wrapper's reassembly buffer is allocated once for the client's life.
+TEST(MqttEffectiveConfig, BufferGrowthForcesRecreateButShrinkDoesNot) {
+  const MqttEffectiveConfig small = build("mqtt://broker:1883", MqttAuth::None, nullptr, nullptr,
+                                          MqttTrust::Plaintext, nullptr, 512);
+  const MqttEffectiveConfig large = build("mqtt://broker:1883", MqttAuth::Jwt, kJwtUser, kToken,
+                                          MqttTrust::Plaintext, nullptr, 896);
+
+  EXPECT_TRUE(decide(small, large, /*allocated=*/512).recreate);
+  EXPECT_STREQ("buffer-growth", decide(small, large, 512).reason);
+  EXPECT_FALSE(decide(small, large, /*allocated=*/896).recreate)
+      << "a client that already owns the capacity does not need recreating";
+  EXPECT_FALSE(decide(large, small, /*allocated=*/896).recreate);
+}
+
+TEST(MqttEffectiveConfig, FirstApplyAndInvalidDesiredNeverRecreate) {
+  const MqttEffectiveConfig unset;
+  const MqttEffectiveConfig wss = build("wss://broker:443/mqtt", MqttAuth::Jwt, kJwtUser, kToken,
+                                        MqttTrust::Bundle, nullptr);
+  EXPECT_FALSE(unset.valid);
+  EXPECT_FALSE(decide(unset, wss).recreate);
+  EXPECT_STREQ("first-apply", decide(unset, wss).reason);
+
+  EXPECT_FALSE(decide(wss, unset).recreate);
+  EXPECT_STREQ("invalid-desired", decide(wss, unset).reason);
+}
+
+// Rule 1: absent is a value that gets written, never an omission.
+TEST(MqttEffectiveConfig, AbsentFieldsAreWrittenAsEmptyStrings) {
+  EXPECT_STREQ("", mqttFieldOrEmpty(nullptr));
+  EXPECT_STREQ("alice", mqttFieldOrEmpty("alice"));
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/test_mqtt_error_labels/test_mqtt_error_labels.cpp

@@ -0,0 +1,78 @@
+#include "helpers/bridges/MQTTErrorLabels.h"
+
+#include <gtest/gtest.h>
+
+#include <string.h>
+
+using namespace MQTTErrorLabels;
+
+namespace {
+
+bool labelIs(const char* actual, const char* expected) {
+  return actual != nullptr && strcmp(actual, expected) == 0;
+}
+
+} // namespace
+
+// F14: these four were the mislabels found in review. 201 is "no AP found"
+// (usually a wrong/hidden SSID), not a security mismatch; 202 is an auth
+// failure (usually a wrong password), not a rejected auth mode.
+TEST(MQTTErrorLabels, WifiReasonsMatchTheSdkMeaning) {
+  EXPECT_TRUE(labelIs(wifiReason(201), "SSID not found"));
+  EXPECT_TRUE(labelIs(wifiReason(202), "auth failed (check password)"));
+  EXPECT_TRUE(labelIs(wifiReason(39), "802.11 timeout"));      // was "SSID not found"
+  EXPECT_TRUE(labelIs(wifiReason(34), "AP saw no acks"));      // was "AP state mismatch"
+}
+
+TEST(MQTTErrorLabels, TlsErrorsMatchTheSdkMeaning) {
+  EXPECT_TRUE(labelIs(tlsError(0x8008), "server closed connection"));   // was "connection timeout"
+  EXPECT_TRUE(labelIs(tlsError(0x8010), "cert chain partly parsed"));   // was "mbedTLS error"
+  EXPECT_TRUE(labelIs(tlsError(0x8006), "connection timeout"));
+  EXPECT_TRUE(labelIs(tlsError(0x8001), "DNS failed"));
+  EXPECT_TRUE(labelIs(tlsError(0x801A), "TLS handshake failed"));
+}
+
+// An unknown code must fall through to the caller's raw-number formatting
+// rather than borrow a neighbouring code's label. 0x800B is the one the review
+// caught: no such esp-tls error exists, but it was labelled "cert verify failed".
+TEST(MQTTErrorLabels, UnknownCodesHaveNoLabel) {
+  EXPECT_EQ(nullptr, tlsError(0x800B));
+  EXPECT_EQ(nullptr, tlsError(0));
+  EXPECT_EQ(nullptr, tlsError(-1));
+  EXPECT_EQ(nullptr, tlsError(0x8099));
+
+  EXPECT_EQ(nullptr, wifiReason(0));
+  EXPECT_EQ(nullptr, wifiReason(61));    // not defined by the installed SDK
+  EXPECT_EQ(nullptr, wifiReason(88));
+  EXPECT_EQ(nullptr, wifiReason(168));
+  EXPECT_EQ(nullptr, wifiReason(255));
+}
+
+TEST(MQTTErrorLabels, ConnackReasonsCoverTheRefusalCodes) {
+  EXPECT_TRUE(labelIs(connackReason(1), "protocol rejected"));
+  EXPECT_TRUE(labelIs(connackReason(2), "client id rejected"));
+  EXPECT_TRUE(labelIs(connackReason(3), "server unavailable"));
+  EXPECT_TRUE(labelIs(connackReason(4), "bad user/password"));
+  EXPECT_TRUE(labelIs(connackReason(5), "not authorized"));
+  EXPECT_EQ(nullptr, connackReason(0));   // accepted: not an error to report
+  EXPECT_EQ(nullptr, connackReason(6));
+}
+
+// Labels share a bounded LoRa reply with the rest of the diagnostic line.
+TEST(MQTTErrorLabels, LabelsStayShortEnoughForABoundedReply) {
+  for (int i = 0; i <= 255; i++) {
+    const char* w = wifiReason((uint8_t)i);
+    if (w) EXPECT_LE(strlen(w), 30u) << "wifi reason " << i;
+    const char* c = connackReason((uint8_t)i);
+    if (c) EXPECT_LE(strlen(c), 30u) << "connack code " << i;
+  }
+  for (int32_t e = 0x8000; e <= 0x8040; e++) {
+    const char* t = tlsError(e);
+    if (t) EXPECT_LE(strlen(t), 30u) << "tls error " << e;
+  }
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/test_mqtt_lifecycle/test_mqtt_lifecycle.cpp

@@ -174,9 +174,12 @@ TEST(MQTTLifecycle, RestartAfterStop) {
 
 // --- Timeout / fallback ----------------------------------------------------
 
-// Handoff: "Timeout/fallback behavior when the MQTT task or client does not
-// acknowledge." Models the reviewed fallback replacing the abrupt vTaskDelete.
-TEST(MQTTLifecycle, StopTimeoutFiresReviewedFallback) {
+// F01: a stop that passes its deadline unacknowledged releases NOTHING. The
+// MQTT task may still be inside mbedTLS or holding the SDK's API mutex, so the
+// queue, buffers, clients and the task itself all stay owned. The OTA barrier
+// is still unblocked — so it can abort — but nothing is freed and the bridge
+// may not restart.
+TEST(MQTTLifecycle, StopTimeoutReleasesNothingAndBlocksRestart) {
   FakeOps ops;
   L::Coordinator c(ops, kStopTimeoutMs);
   bringUpToRunning(c);
@@ -191,18 +194,70 @@ TEST(MQTTLifecycle, StopTimeoutFiresReviewedFallback) {
   EXPECT_EQ(L::State::StopRequested, c.state());
   EXPECT_EQ(0, ops.release_calls);
 
-  // At the deadline the fallback fires: forced release, dirty OTA signal.
   ops.now = 1000 + kStopTimeoutMs;
   c.tick();
-  EXPECT_EQ(L::State::Stopped, c.state());
+
+  EXPECT_EQ(L::State::StopUnproven, c.state());
+  EXPECT_TRUE(c.isStopUnproven());
   EXPECT_TRUE(c.stopTimedOut());
-  EXPECT_EQ(1, ops.release_calls);
+  EXPECT_EQ(0, ops.release_calls) << "an unproven stop must not release";
   EXPECT_EQ(1, ops.stop_complete_calls);
   EXPECT_FALSE(ops.last_stop_clean);
 
-  // A late ack after the fallback does not double-release.
+  // Everything the task can reach is still owned, and neither a restart nor a
+  // flash is permitted. A repeated tick does not change that.
+  EXPECT_TRUE(c.mayTouchOwnedState());
+  EXPECT_FALSE(c.mayRestart());
+  EXPECT_FALSE(c.mayBeginFlash());
+  EXPECT_FALSE(c.acceptsNewWork());
+  EXPECT_FALSE(c.isStopInProgress()) << "end()'s wait loop must terminate";
+
+  ops.now += kStopTimeoutMs * 10;
+  c.tick();
+  EXPECT_EQ(L::State::StopUnproven, c.state());
+  EXPECT_EQ(0, ops.release_calls);
+
+  // A start attempt is refused outright: a new start is not proof that the
+  // previous clients stopped.
+  EXPECT_FALSE(c.requestStart());
+  EXPECT_EQ(L::State::StopUnproven, c.state());
+  EXPECT_EQ(1, ops.start_task_calls) << "no second task while unproven";
+  EXPECT_TRUE(c.stopTimedOut()) << "a refused start cannot clear the dirty latch";
+}
+
+// The deadline is an availability policy, not a statement about what an ack
+// means: the task publishes it only after tearing its clients down, so a late
+// ack is exactly as trustworthy as a timely one. Honouring it releases the
+// resources and makes the bridge restartable without a reboot.
+TEST(MQTTLifecycle, LateAckRecoversFromAnUnprovenStop) {
+  FakeOps ops;
+  L::Coordinator c(ops, kStopTimeoutMs);
+  bringUpToRunning(c);
+
+  ops.now = 1000;
+  ASSERT_TRUE(c.requestStop());
+  ops.now = 1000 + kStopTimeoutMs;
+  c.tick();
+  ASSERT_EQ(L::State::StopUnproven, c.state());
+  ASSERT_EQ(0, ops.release_calls);
+
+  ops.now = 1000 + kStopTimeoutMs * 4;
+  ASSERT_TRUE(c.onTaskStopped());
+
+  EXPECT_EQ(L::State::Stopped, c.state());
+  EXPECT_EQ(1, ops.release_calls) << "the proof arrived; release now";
+  EXPECT_TRUE(c.mayRestart());
+  // The OTA barrier already reported dirty and its caller acted on that, so the
+  // flash gate stays shut until a clean start/stop cycle.
+  EXPECT_EQ(1, ops.stop_complete_calls);
+  EXPECT_FALSE(c.mayBeginFlash());
+  EXPECT_TRUE(c.stopTimedOut());
+
+  // Restarting from the proven stop clears the latch, and a second ack is inert.
   EXPECT_FALSE(c.onTaskStopped());
   EXPECT_EQ(1, ops.release_calls);
+  ASSERT_TRUE(c.requestStart());
+  EXPECT_FALSE(c.stopTimedOut());
 }
 
 TEST(MQTTLifecycle, TickWithoutPendingStopIsNoop) {
@@ -371,7 +426,9 @@ TEST(MQTTLifecycle, OtaFlashBlockedUntilCleanStopAcknowledged) {
 }
 
 // Handoff OTA barrier: "MQTT stop times out: OTA aborts safely rather than
-// writing under uncertain ownership."
+// writing under uncertain ownership." The barrier is still released so the
+// caller aborts, but flashing stays shut and — unlike the pre-F01 contract —
+// the bridge cannot be restarted to paper over it.
 TEST(MQTTLifecycle, OtaAbortsWhenStopTimesOut) {
   FakeOps ops;
   L::Coordinator c(ops, kStopTimeoutMs);
@@ -382,11 +439,16 @@ TEST(MQTTLifecycle, OtaAbortsWhenStopTimesOut) {
   ops.now = 500 + kStopTimeoutMs;
   c.tick();
 
-  EXPECT_EQ(L::State::Stopped, c.state());
-  EXPECT_FALSE(c.mayBeginFlash());  // dirty stop => flashing withheld
-  EXPECT_FALSE(ops.last_stop_clean);
+  EXPECT_EQ(L::State::StopUnproven, c.state());
+  EXPECT_EQ(1, ops.stop_complete_calls);  // barrier released...
+  EXPECT_FALSE(ops.last_stop_clean);      // ...with "do not flash"
+  EXPECT_FALSE(c.mayBeginFlash());
+  EXPECT_FALSE(c.requestStart());         // and no restart to hide it
 
-  // A fresh clean start/stop cycle clears the latch and re-enables flashing.
+  // Only the task's late acknowledgement makes the bridge usable again, and
+  // only a clean cycle after that re-enables flashing.
+  ASSERT_TRUE(c.onTaskStopped());
+  EXPECT_FALSE(c.mayBeginFlash());
   ASSERT_TRUE(c.requestStart());
   EXPECT_FALSE(c.stopTimedOut());
   ASSERT_TRUE(c.onTaskStarted());
@@ -415,8 +477,13 @@ TEST(MQTTLifecycle, NoRestartWhileStopInProgress) {
 }
 
 // Handoff OTA barrier: "Repeated failed OTA attempts do not ... leave the
-// bridge permanently stopped."
-TEST(MQTTLifecycle, RepeatedFailedStopsLeaveBridgeRestartable) {
+// bridge permanently stopped." Re-encoded for the F01 contract: what keeps the
+// bridge usable across repeated slow stops is the task's acknowledgement, not
+// the deadline. A stop that is merely slow recovers every time; a stop that is
+// never acknowledged deliberately does NOT, because restarting on top of
+// clients that may still be running is the failure this state exists to
+// prevent. That is an availability trade, made knowingly.
+TEST(MQTTLifecycle, RepeatedSlowStopsRecoverOnTheirLateAck) {
   FakeOps ops;
   L::Coordinator c(ops, kStopTimeoutMs);
 
@@ -427,19 +494,51 @@ TEST(MQTTLifecycle, RepeatedFailedStopsLeaveBridgeRestartable) {
     ops.now += 1000;
     ASSERT_TRUE(c.requestStop());
     ops.now += kStopTimeoutMs;
-    c.tick();  // times out (dirty)
+    c.tick();                      // deadline passes with no ack
+    EXPECT_EQ(L::State::StopUnproven, c.state());
+    EXPECT_FALSE(c.mayRestart());  // not until the task proves it finished
+    EXPECT_EQ(attempt, ops.release_calls);
+
+    ops.now += 1000;
+    ASSERT_TRUE(c.onTaskStopped());  // the slow teardown completes
     EXPECT_EQ(L::State::Stopped, c.state());
-    EXPECT_TRUE(c.mayRestart());  // never permanently stuck
+    EXPECT_TRUE(c.mayRestart());
+    EXPECT_EQ(attempt + 1, ops.release_calls);
   }
   EXPECT_EQ(3, ops.start_task_calls);
 }
 
+// A never-acknowledged stop stays unusable for the rest of the boot: no
+// release, no restart, no flash, however long the owner waits or however many
+// times it asks.
+TEST(MQTTLifecycle, NeverAcknowledgedStopStaysUnproven) {
+  FakeOps ops;
+  L::Coordinator c(ops, kStopTimeoutMs);
+  bringUpToRunning(c);
+
+  ASSERT_TRUE(c.requestStop());
+  ops.now += kStopTimeoutMs;
+  c.tick();
+
+  for (int i = 0; i < 5; ++i) {
+    ops.now += 60'000;
+    c.tick();
+    EXPECT_FALSE(c.requestStart());
+    EXPECT_FALSE(c.requestStop());
+    EXPECT_EQ(L::State::StopUnproven, c.state());
+  }
+  EXPECT_EQ(0, ops.release_calls);
+  EXPECT_FALSE(c.mayBeginFlash());
+  EXPECT_TRUE(c.mayTouchOwnedState());
+}
+
 // --- Diagnostics -----------------------------------------------------------
 
 TEST(MQTTLifecycle, StateAndEventNamesAreStable) {
   EXPECT_STREQ("Stopped", L::stateName(L::State::Stopped));
   EXPECT_STREQ("Running", L::stateName(L::State::Running));
   EXPECT_STREQ("StopRequested", L::stateName(L::State::StopRequested));
+  EXPECT_STREQ("StopUnproven", L::stateName(L::State::StopUnproven));
   EXPECT_STREQ("StartRequested", L::eventName(L::Event::StartRequested));
   EXPECT_STREQ("StopTimedOut", L::eventName(L::Event::StopTimedOut));
 }

test/test_ntp_validation/test_ntp_validation.cpp

@@ -0,0 +1,184 @@
+#include "helpers/NtpValidation.h"
+
+#include <gtest/gtest.h>
+
+#include <string.h>
+
+using namespace NtpValidation;
+
+namespace {
+
+const uint32_t MIN_EPOCH = 1767225600UL;   // 2026-01-01, the bridge's floor
+const uint32_t MAX_EPOCH = 4102444800UL;   // 2100-01-01
+const Nonce NONCE = { 0xA1B2C3D4UL, 0x0F1E2D3CUL };
+
+// A well-formed reply to buildRequest(): stratum 2, mode 4, our nonce echoed.
+void makeReply(uint8_t out[48], uint32_t unix_epoch = 1789000000UL) {
+  memset(out, 0, 48);
+  out[0] = (0 << 6) | (4 << 3) | 4;   // LI=0, VN=4, mode=4 (server)
+  out[1] = 2;                         // stratum
+  writeU32(out + 24, NONCE.seconds);      // originate = our transmit timestamp
+  writeU32(out + 28, NONCE.fraction);
+  writeU32(out + 32, unix_epoch + kUnixEpochOffset);   // receive
+  writeU32(out + 40, unix_epoch + kUnixEpochOffset);   // transmit
+}
+
+Result check(const uint8_t* data, size_t len, bool source_matches = true) {
+  return validate(data, len, source_matches, NONCE, MIN_EPOCH, MAX_EPOCH);
+}
+
+} // namespace
+
+TEST(NtpValidation, RequestIsAClientPacketCarryingTheNonce) {
+  uint8_t req[48];
+  memset(req, 0xFF, sizeof(req));
+  buildRequest(req, NONCE);
+
+  EXPECT_EQ(0u, (req[0] >> 6) & 0x03);   // LI = 0, not the library's bogus alarm
+  EXPECT_EQ(4u, (req[0] >> 3) & 0x07);   // version 4
+  EXPECT_EQ(3u, req[0] & 0x07);          // mode 3 = client
+  EXPECT_EQ(NONCE.seconds, readU32(req + 40));
+  EXPECT_EQ(NONCE.fraction, readU32(req + 44));
+}
+
+TEST(NtpValidation, ValidReplyIsAccepted) {
+  uint8_t pkt[48];
+  makeReply(pkt, 1789000000UL);
+
+  Result r = check(pkt, sizeof(pkt));
+
+  EXPECT_EQ(kAccepted, r.reject);
+  EXPECT_EQ(1789000000UL, r.epoch);
+}
+
+// F07's reproduction: the installed NTPClient accepted a one-byte non-NTP
+// datagram and derived epoch 2085978496 from the uninitialised buffer.
+TEST(NtpValidation, OneByteNonNtpDatagramIsRejected) {
+  const uint8_t junk[1] = { 0x00 };
+
+  Result r = check(junk, sizeof(junk));
+
+  EXPECT_EQ(kShortPacket, r.reject);
+  EXPECT_EQ(0u, r.epoch);
+}
+
+TEST(NtpValidation, ShortAndEmptyDatagramsAreRejected) {
+  uint8_t pkt[48];
+  makeReply(pkt);
+
+  for (size_t len = 0; len < 48; len++) {
+    EXPECT_EQ(kShortPacket, check(pkt, len).reject) << "len " << len;
+  }
+  EXPECT_EQ(kShortPacket, check(nullptr, 48).reject);
+}
+
+TEST(NtpValidation, ReplyFromAnotherSourceIsRejected) {
+  uint8_t pkt[48];
+  makeReply(pkt);
+
+  EXPECT_EQ(kWrongSource, check(pkt, sizeof(pkt), /*source_matches=*/false).reject);
+}
+
+// An off-path sender that never saw the query cannot echo the nonce.
+TEST(NtpValidation, UnsolicitedReplyIsRejected) {
+  uint8_t pkt[48];
+  makeReply(pkt);
+  writeU32(pkt + 24, 0);
+  writeU32(pkt + 28, 0);
+
+  EXPECT_EQ(kOriginMismatch, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt);
+  writeU32(pkt + 28, NONCE.fraction ^ 1u);   // one bit off is still not ours
+  EXPECT_EQ(kOriginMismatch, check(pkt, sizeof(pkt)).reject);
+}
+
+TEST(NtpValidation, ClientModeAndWrongVersionAreRejected) {
+  uint8_t pkt[48];
+
+  makeReply(pkt);
+  pkt[0] = (0 << 6) | (4 << 3) | 3;          // our own request echoed back
+  EXPECT_EQ(kBadMode, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt);
+  pkt[0] = (0 << 6) | (5 << 3) | 4;          // version 5 does not exist
+  EXPECT_EQ(kBadVersion, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt);
+  pkt[0] = (0 << 6) | (3 << 3) | 4;          // NTPv3 servers are fine
+  EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject);
+}
+
+TEST(NtpValidation, UnsynchronisedServersAreRejected) {
+  uint8_t pkt[48];
+
+  makeReply(pkt);
+  pkt[0] |= (3 << 6);                        // LI = 3: alarm, clock not set
+  EXPECT_EQ(kLeapAlarm, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt);
+  pkt[1] = 0;                                // stratum 0: kiss-of-death
+  EXPECT_EQ(kBadStratum, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt);
+  pkt[1] = 16;                               // unsynchronised
+  EXPECT_EQ(kBadStratum, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt);
+  pkt[1] = 15;                               // last usable stratum
+  EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject);
+}
+
+TEST(NtpValidation, MissingTransmitTimestampIsRejected) {
+  uint8_t pkt[48];
+  makeReply(pkt);
+  writeU32(pkt + 40, 0);
+
+  EXPECT_EQ(kZeroTransmit, check(pkt, sizeof(pkt)).reject);
+}
+
+// The bridge's floor is the only defence against a stale or rolled-back clock,
+// and nothing previously stopped an absurd forward jump.
+TEST(NtpValidation, ImplausibleTimesAreRejected) {
+  uint8_t pkt[48];
+
+  makeReply(pkt, MIN_EPOCH - 1);
+  EXPECT_EQ(kEpochTooEarly, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt, MIN_EPOCH);
+  EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt, MAX_EPOCH + 1);
+  EXPECT_EQ(kEpochTooLate, check(pkt, sizeof(pkt)).reject);
+
+  makeReply(pkt, MAX_EPOCH);
+  EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject);
+}
+
+// Era 1 begins 2036-02-07; a naive subtraction turns those seconds into 1900.
+TEST(NtpValidation, Era1TimestampsConvertForward) {
+  EXPECT_EQ(0u, unixFromNtpSeconds(kUnixEpochOffset));
+  EXPECT_EQ(2085978496UL, unixFromNtpSeconds(0));          // 2036-02-07
+  EXPECT_GT(unixFromNtpSeconds(1000), unixFromNtpSeconds(0xFFFFFFFFu));
+
+  uint8_t pkt[48];
+  makeReply(pkt);
+  writeU32(pkt + 40, 1000);          // 1000 s into era 1
+  Result r = check(pkt, sizeof(pkt));
+  EXPECT_EQ(kAccepted, r.reject);
+  EXPECT_EQ(2085979496UL, r.epoch);
+}
+
+TEST(NtpValidation, EveryRejectionHasAShortReason) {
+  for (int i = 0; i <= (int)kEpochTooLate; i++) {
+    const char* reason = rejectReason((Reject)i);
+    ASSERT_NE(nullptr, reason);
+    EXPECT_GT(strlen(reason), 0u);
+    EXPECT_LE(strlen(reason), 20u) << "reject " << i;
+  }
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/test_radio_watchdog/test_radio_watchdog.cpp

@@ -0,0 +1,204 @@
+#include "helpers/RadioWatchdog.h"
+
+#include <gtest/gtest.h>
+
+namespace {
+
+const uint32_t WATCHDOG_MS = 300000UL;   // RADIO_WATCHDOG_MS default: 5 minutes
+
+// Mirrors Dispatcher::loop(): the radio's three timestamps are sampled every
+// pass, and a recovery is acknowledged as soon as it is decided.
+struct Radio {
+  uint32_t last_recv = 0;
+  uint32_t last_irq = 0;
+  uint32_t last_tx = 0;
+};
+
+RadioWatchdogDecision run(RadioWatchdog& w, const Radio& r, uint32_t now,
+                          bool in_recv_mode = true, uint32_t watchdog_ms = WATCHDOG_MS) {
+  RadioWatchdogDecision d = w.update(now, in_recv_mode, watchdog_ms,
+                                     r.last_recv, r.last_irq, r.last_tx);
+  if (d.recover) w.noteRecovery();
+  return d;
+}
+
+} // namespace
+
+TEST(RadioWatchdog, NoActivitySinceBootNeverTrips) {
+  RadioWatchdog w;
+  w.reset(0);
+  Radio r;   // radio has never received, interrupted or transmitted
+
+  for (uint32_t t = 0; t <= 4 * WATCHDOG_MS; t += 10000) {
+    RadioWatchdogDecision d = run(w, r, t);
+    EXPECT_FALSE(d.recover) << "tripped at t=" << t;
+    EXPECT_FALSE(d.measurable);
+  }
+}
+
+TEST(RadioWatchdog, ContinuousReceiveNeverTrips) {
+  RadioWatchdog w;
+  w.reset(0);
+  Radio r;
+
+  for (uint32_t t = 1000; t <= 3 * WATCHDOG_MS; t += 1000) {
+    r.last_recv = t;
+    r.last_irq = t;
+    RadioWatchdogDecision d = run(w, r, t);
+    EXPECT_FALSE(d.recover) << "tripped at t=" << t;
+    EXPECT_LE(d.silent_ms, 1000u);
+  }
+}
+
+TEST(RadioWatchdog, SilentRadioTripsOncePerInterval) {
+  RadioWatchdog w;
+  w.reset(0);
+  Radio r;
+  r.last_recv = 1000;
+  run(w, r, 1000);
+
+  int trips = 0;
+  uint32_t first_trip = 0;
+  for (uint32_t t = 2000; t <= 1000000; t += 1000) {
+    if (run(w, r, t).recover) {
+      if (trips == 0) first_trip = t;
+      trips++;
+    }
+  }
+
+  EXPECT_EQ(302000u, first_trip);           // strictly past the interval, from the last receive
+  EXPECT_EQ(3, trips);                      // ~1 M ms / 300 s, never per-loop
+}
+
+// F13: a transmit timestamp from just before the millis() wrap is numerically
+// larger than every fresh post-wrap receive. Selecting the largest timestamp
+// reported ~24 days of silence and reset a radio that was receiving normally.
+TEST(RadioWatchdog, PreWrapTransmitDoesNotSilenceFreshPostWrapReceives) {
+  const uint32_t WRAP = 0xFFFFFFFFu;
+  const uint32_t TX_AT = 0x80000000u;       // upper half: beats any post-wrap value
+
+  RadioWatchdog w;
+  w.reset(TX_AT - 60000);
+  Radio r;
+  r.last_recv = TX_AT - 60000;
+  r.last_irq = TX_AT - 60000;
+  run(w, r, TX_AT - 60000);
+
+  r.last_tx = TX_AT;
+  run(w, r, TX_AT);
+
+  // Receive steadily right up to the wrap...
+  for (uint32_t t = TX_AT + 1000; t < WRAP - 1000; t += 1000) {
+    r.last_recv = t;
+    r.last_irq = t;
+    ASSERT_FALSE(run(w, r, t).recover) << "tripped before wrap at t=" << t;
+  }
+
+  // ...and straight through it. The stale transmit is still the numerically
+  // largest of the three timestamps for the next ~24 days.
+  for (uint32_t t = 1000; t < 2 * WATCHDOG_MS; t += 1000) {
+    r.last_recv = t;
+    r.last_irq = t;
+    RadioWatchdogDecision d = run(w, r, t);
+    ASSERT_FALSE(d.recover) << "tripped after wrap at t=" << t;
+    ASSERT_LE(d.silent_ms, 1000u);
+  }
+}
+
+// The wrap must not hide a genuinely silent radio either.
+TEST(RadioWatchdog, SilenceAcrossTheWrapIsStillDetected) {
+  const uint32_t LAST_RX = 0xFFFF0000u;     // ~65 s before the wrap
+
+  RadioWatchdog w;
+  w.reset(LAST_RX - 1000);
+  Radio r;
+  r.last_recv = LAST_RX;
+  r.last_irq = LAST_RX;
+  run(w, r, LAST_RX);
+
+  bool tripped = false;
+  uint64_t silent_at_trip = 0;
+  for (uint32_t step = 1000; step <= WATCHDOG_MS + 5000; step += 1000) {
+    uint32_t t = LAST_RX + step;            // wraps on its own
+    RadioWatchdogDecision d = run(w, r, t);
+    if (d.recover) {
+      tripped = true;
+      silent_at_trip = d.silent_ms;
+      break;
+    }
+  }
+
+  EXPECT_TRUE(tripped);
+  EXPECT_GT(silent_at_trip, (uint64_t)WATCHDOG_MS);
+}
+
+// Uptime past 2^32 ms must not alias a long silence back to "recent".
+TEST(RadioWatchdog, SilenceBeyondOneMillisCycleKeepsGrowing) {
+  RadioWatchdog w;
+  w.reset(0);
+  Radio r;
+  r.last_recv = 1000;
+  run(w, r, 1000);
+
+  // Walk a full cycle in 1-minute steps; the raw timestamp aliases back to its
+  // starting value, the tracked age does not.
+  uint64_t last_silent = 0;
+  for (uint64_t step = 60000; step <= 0x100000000ull + 600000ull; step += 60000) {
+    uint32_t t = (uint32_t)((1000ull + step) & 0xFFFFFFFFull);
+    RadioWatchdogDecision d = w.update(t, true, WATCHDOG_MS, r.last_recv, r.last_irq, r.last_tx);
+    if (d.recover) w.noteRecovery();
+    ASSERT_GT(d.silent_ms, last_silent);
+    last_silent = d.silent_ms;
+  }
+
+  EXPECT_GT(last_silent, 0x100000000ull);
+}
+
+TEST(RadioWatchdog, DisabledWatchdogAndNonRecvModeNeverTrip) {
+  RadioWatchdog w;
+  w.reset(0);
+  Radio r;
+  r.last_recv = 1000;
+  run(w, r, 1000);
+
+  for (uint32_t t = 2000; t <= 1000000; t += 10000) {
+    EXPECT_FALSE(run(w, r, t, /*in_recv_mode=*/true, /*watchdog_ms=*/0).recover);
+    EXPECT_FALSE(run(w, r, t, /*in_recv_mode=*/false).recover);
+  }
+}
+
+// Transmit-only traffic still counts as a live radio.
+TEST(RadioWatchdog, TransmitActivityRefreshesSilence) {
+  RadioWatchdog w;
+  w.reset(0);
+  Radio r;
+  r.last_recv = 1000;
+  run(w, r, 1000);
+
+  for (uint32_t t = 100000; t <= 900000; t += 100000) {
+    r.last_tx = t;
+    EXPECT_FALSE(run(w, r, t).recover) << "tripped at t=" << t;
+  }
+}
+
+// A blocked mesh loop (e.g. a 30 s NTP probe) collapses several events into one
+// observation. That may only shorten measured silence, never lengthen it.
+TEST(RadioWatchdog, LoopStallDoesNotManufactureSilence) {
+  RadioWatchdog w;
+  w.reset(0);
+  Radio r;
+  r.last_recv = 1000;
+  run(w, r, 1000);
+
+  r.last_recv = 20000;   // arrived during the stall
+  r.last_irq = 20000;
+  RadioWatchdogDecision d = run(w, r, 31000);
+
+  EXPECT_FALSE(d.recover);
+  EXPECT_EQ(0u, d.silent_ms);
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/test_wifi_power_save_policy/test_wifi_power_save_policy.cpp

@@ -0,0 +1,68 @@
+#include "helpers/WifiPowerSavePolicy.h"
+
+#include <gtest/gtest.h>
+
+using namespace WifiPowerSavePolicy;
+
+// F11: the CLI applied stored 0 as MIN_MODEM while the reconnect path applied it
+// as NONE, so a node configured for `min` changed behaviour after every
+// reconnect and `get wifi.powersave` still reported min. One table, one answer.
+TEST(WifiPowerSavePolicy, StoredValuesMapToOneModeEach) {
+  EXPECT_EQ(kModeMinModem, modeFor(kMin));
+  EXPECT_EQ(kModeNone, modeFor(kNone));
+  EXPECT_EQ(kModeMaxModem, modeFor(kMax));
+}
+
+// Stored 0 was the default for nodes set up 2026-01-02..03-28, and they have
+// always run with power save off; reading it as `min` would put them to sleep.
+TEST(WifiPowerSavePolicy, LegacyDefaultKeepsPowerSaveOff) {
+  EXPECT_EQ(kModeNone, modeFor(kLegacyDefault));
+  EXPECT_STREQ("none", nameFor(kLegacyDefault));
+  uint8_t parsed = 0xFF;
+  ASSERT_TRUE(parseName("min", &parsed));
+  EXPECT_NE(kLegacyDefault, parsed);
+}
+
+TEST(WifiPowerSavePolicy, NamesRoundTripWithStoredValues) {
+  for (uint8_t stored = kNone; stored <= kMaxStoredValue; stored++) {
+    uint8_t parsed = 0xFF;
+    ASSERT_TRUE(parseName(nameFor(stored), &parsed)) << "stored " << (int)stored;
+    EXPECT_EQ(stored, parsed);
+    EXPECT_EQ(modeFor(stored), modeFor(parsed));
+  }
+}
+
+// A byte outside the stored range must read as the product default, not as
+// whatever mode happens to sit at that index.
+TEST(WifiPowerSavePolicy, OutOfRangeStoredValueReadsAsDefault) {
+  for (int stored = kMaxStoredValue + 1; stored <= 255; stored++) {
+    EXPECT_EQ(kModeNone, modeFor((uint8_t)stored)) << "stored " << stored;
+    EXPECT_STREQ("none", nameFor((uint8_t)stored));
+  }
+}
+
+// The setters take the remainder of the command line, so a trailing argument
+// must still parse — and a longer word starting with a valid name must not.
+TEST(WifiPowerSavePolicy, ParsesCliArgumentsExactly) {
+  uint8_t stored = 0xFF;
+
+  EXPECT_TRUE(parseName("min", &stored));
+  EXPECT_EQ(kMin, stored);
+  EXPECT_TRUE(parseName("none extra", &stored));
+  EXPECT_EQ(kNone, stored);
+  EXPECT_TRUE(parseName("max ", &stored));
+  EXPECT_EQ(kMax, stored);
+
+  stored = 0xFF;
+  EXPECT_FALSE(parseName("minimum", &stored));
+  EXPECT_FALSE(parseName("", &stored));
+  EXPECT_FALSE(parseName("off", &stored));
+  EXPECT_FALSE(parseName("MIN", &stored));
+  EXPECT_FALSE(parseName(nullptr, &stored));
+  EXPECT_EQ(0xFF, stored);   // rejected input leaves the caller's value alone
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}