Merge fix/mqtt-review-2026-09-09 into feat/abstract-networking

#54 lands first, so the network abstraction is reworked on top of the
per-slot client ownership model instead of alongside it.

Two resolutions are semantic, not textual:

- Link-transition teardown goes through the ownership API. It called
  softDisconnect() on every started client, which bypassed client_state and
  ignored the typed result. A slot that is still Starting now gets a real
  stop: softDisconnect() returns immediately on a client that is not yet
  connected, so its attempt would otherwise complete against the old route and
  deliver a CONNECTED event indistinguishable from the new one's (F04). On a
  medium switch the old route can still be briefly usable, so that is not
  hypothetical. A connected client keeps the cheap bounded path, and a
  quarantined one is left alone.

- NetworkLink::applyPowerPrefs() adopts WifiPowerSavePolicy. Its local
  `2 ? MAX : NONE` mapping would have reintroduced F11 and read the new stored
  value 3 (explicit `min`) as none. It also applies the setting when the link
  starts already associated, which is the case the bridge used to cover.

The NTP probe from #54 keeps its validation and its one-attempt-per-server
bound, but resolves and gates on the selected link rather than on WiFi, so it
works on an Ethernet-preferred node.

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

@@ -16,6 +16,7 @@
 #include "AlertReporter.h"  // for alertReporterBannedChannelMatch[Hex]()
 #include "MQTTObserverValidation.h"  // pure input validators (host-testable)
 #include "NetworkLink.h"
+#include "WifiPowerSavePolicy.h"     // one powersave value->mode/name mapping
 #include <Utils.h>
 #include <climits>
 #include <new>
@@ -441,42 +442,29 @@ 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 (strcmp(activeNetworkLink().mediumName(), "wifi") == 0 &&
           activeNetworkLink().isConnected()) {
-        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 link 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
     }
@@ -1121,9 +1109,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
 
@@ -54,7 +55,7 @@ class MQTTPrefsSerializer : public ConfigSerializer {
         : _prefs(prefs), _power_save(prefs->wifi_power_save),
           _setup_complete(prefs->network_setup_complete) {}
     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/NetworkLink.cpp

@@ -3,6 +3,7 @@
 #if defined(ESP_PLATFORM)
 
 #include "MQTTConnectionPolicy.h"
+#include "WifiPowerSavePolicy.h"
 
 #include <atomic>
 #include <climits>
@@ -90,9 +91,22 @@ class WiFiNetworkLink final : public NetworkLinkBase {
   unsigned long _last_reconnect_attempt = 0;
   uint8_t _reconnect_backoff_attempt = 0;
 
+  // One mapping for startup, reconnect and CLI (see WifiPowerSavePolicy). The
+  // stored default is `none`; `min` means MIN_MODEM here exactly as the CLI
+  // says it does. Mapping the stored value locally is what let a node set to
+  // `min` silently run with power save off after its first reconnect.
   void applyPowerPrefs(uint8_t wifi_power_save) {
-    wifi_ps_type_t ps_mode = wifi_power_save == 2 ? WIFI_PS_MAX_MODEM : WIFI_PS_NONE;
-    esp_wifi_set_ps(ps_mode);
+    static_assert((int)WifiPowerSavePolicy::kModeNone == (int)WIFI_PS_NONE, "wifi_ps_type_t drift");
+    static_assert((int)WifiPowerSavePolicy::kModeMinModem == (int)WIFI_PS_MIN_MODEM, "wifi_ps_type_t drift");
+    static_assert((int)WifiPowerSavePolicy::kModeMaxModem == (int)WIFI_PS_MAX_MODEM, "wifi_ps_type_t drift");
+    esp_wifi_set_ps((wifi_ps_type_t)WifiPowerSavePolicy::modeFor(wifi_power_save));
+    // Read back rather than logging what we asked for: this is the only place
+    // the mode is observable on a running node, and the setting used to change
+    // meaning between the CLI and this path.
+    wifi_ps_type_t applied = WIFI_PS_NONE;
+    esp_wifi_get_ps(&applied);
+    NETWORK_DEBUG_PRINTLN("WiFi power save: %s (mode=%d)",
+                          WifiPowerSavePolicy::nameFor(wifi_power_save), (int)applied);
 #ifdef MQTT_WIFI_TX_POWER
     WiFi.setTxPower(MQTT_WIFI_TX_POWER);
 #else
@@ -182,6 +196,10 @@ class WiFiNetworkLink final : public NetworkLinkBase {
       _status_initialized = true;
       setOutage(AlertFaultPolicy::applyWifiStatus(
           now_ms, connected, outage(), false));
+      // Already associated when the link started (end()/begin() leaves STA up):
+      // there is no connect transition below to carry the setting, so apply it
+      // here or the node keeps running whatever mode was set before.
+      if (connected) applyPowerPrefs(wifi_power_save);
     }
 
     if ((uint32_t)(now_ms - _last_status_check) <= 10000) {

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,7 @@
 #include "helpers/bridges/BridgeBase.h"
 #include "helpers/NetworkLink.h"
 #include <PsychicMqttClient.h>
-#include <NTPClient.h>
+#include <WiFi.h>
 #include <WiFiUdp.h>
 #include <Timezone.h>
 #include "helpers/JWTHelper.h"
@@ -12,6 +12,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 +94,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 +143,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 +175,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 +256,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
@@ -237,7 +281,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
@@ -248,7 +292,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;
@@ -257,19 +301,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
@@ -429,24 +484,53 @@ 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);
+  // Close a live client because the route changed under it: bounded where that
+  // is enough, a real stop where an in-flight attempt has to be cancelled.
+  void closeLiveClientForLinkTransition(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
@@ -483,6 +567,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.
@@ -630,6 +718,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();
 
@@ -679,7 +780,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

@@ -17,6 +17,7 @@
 #include <helpers/MQTTPacketFilter.h>
 #include <helpers/MQTTPresets.h>
 #include <helpers/WebConfigKeys.h>
+#include <helpers/WifiPowerSavePolicy.h>
 #include <helpers/bridges/MQTTBridge.h>
 
 #include "HttpPort80Lease.h"
@@ -881,8 +882,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;