From b1ceaf01a82ac7884200bb1bb8393d789b400694 Mon Sep 17 00:00:00 2001
From: agessaman <adam@gessaman.com>
Date: Fri, 14 Aug 2026 19:40:48 -0700
Subject: [PATCH] fix(mqtt): require real SNTP completion before crediting a
 fallback server
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The fallback configured a server, waited 500 ms, and accepted any plausible
system clock as proof that server had answered. It usually has not answered
that fast — and the device usually already holds valid time, from an earlier
sync or the RTC — so the first server in the list was credited unconditionally,
the walk stopped there, _last_ntp_sync was refreshed, and an unreachable host
was logged as the source. On the `set mqtt.ntp` validation path, where the
single-server walk exists so a typo fails fast, that reported a bad server as OK.

Poll sntp_get_sync_status() for SNTP_SYNC_STATUS_COMPLETED instead, which is the
layer's own statement that a packet arrived. The status is one-shot — reading
COMPLETED clears it — so a result left by an earlier sync would latch on the
first poll; clear it before the loop.

An implausible epoch after a completed sync now moves to the next server rather
than spinning out the remaining attempts against a server that has answered.
---
 src/helpers/bridges/MQTTBridge.cpp | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp
index a325d0ab..b73ab34b 100644
--- a/src/helpers/bridges/MQTTBridge.cpp
+++ b/src/helpers/bridges/MQTTBridge.cpp
@@ -21,6 +21,7 @@
 
 #ifdef ESP_PLATFORM
 #include <esp_wifi.h>
+#include <esp_sntp.h>
 #include <esp_heap_caps.h>
 #include <freertos/FreeRTOS.h>
 #include <freertos/task.h>
@@ -3986,15 +3987,26 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only) {
       const char* server = servers[s];
       MQTT_DEBUG_PRINTLN("SNTP fallback trying %s...", server);
       configTime(0, 0, server);
+      // A plausible clock is not evidence this server answered. The device usually
+      // already holds valid time here — from an earlier sync, or the RTC — so polling
+      // time(nullptr) declared the very first server successful without a packet ever
+      // arriving, stopped the fallback walk there, and refreshed _last_ntp_sync. Worse
+      // on the `set mqtt.ntp` validation path, where a typo is supposed to fail fast.
+      // Wait for SNTP itself to report completion. The status is one-shot — reading
+      // COMPLETED clears it — so drop any result an earlier sync left behind.
+      sntp_set_sync_status(SNTP_SYNC_STATUS_RESET);
       for (int i = 0; i < 20; i++) {
         delay(500);
+        if (sntp_get_sync_status() != SNTP_SYNC_STATUS_COMPLETED) continue;
         epochTime = (unsigned long)time(nullptr);
         if (epochTime >= kMinValidEpoch) {
           ntp_ok = true;
           ntp_server_used = server;
           MQTT_DEBUG_PRINTLN("SNTP fallback succeeded on %s: %lu", server, epochTime);
-          break;
+        } else {
+          MQTT_DEBUG_PRINTLN("SNTP fallback: %s synced an implausible epoch %lu", server, epochTime);
         }
+        break;
       }
     }
   }