From 4ff118ce52c2e2d13d433b107ba2262e608e0e00 Mon Sep 17 00:00:00 2001
From: agessaman <adam@gessaman.com>
Date: Sun, 19 Jul 2026 15:45:24 -0700
Subject: [PATCH] fix(prefs): stop spurious VFS error on every prefs save

Saving observer prefs logged an ESP32 error line on every save:

  [E][vfs_api.cpp:182] remove(): /mqtt_prefs.tmp does not exists or is directory

MQTTPrefsFileStore::begin() cleared a stale transaction with an unconditional
remove("/mqtt_prefs.tmp"). On the normal path there is no stale tmp - commit()
renames it away - so the remove always failed and the ESP32 VFS layer logged it
at [E] level. The save itself succeeded; the noise just reads as a fault in the
serial log at exactly the moment an operator is watching a config change.

Guard each remove on exists(), at all four sites: begin() and abort() for both
the /mqtt_prefs and /com_prefs stores. Semantics are unchanged - a genuinely
stale tmp is still cleared, and a failure to clear it still aborts the
transaction - it just stops issuing a syscall that can only fail.

Fixed here on webconfig (the 1.16.0-based line that carries the atomic store)
so it flows to flex with the rest of that work. NOT applicable to
mqtt-bridge-implementation-flex today: flex has no .tmp/rename handling at all,
so the code path does not exist there.

Verified: Heltec_v3_repeater_observer_mqtt builds; hardware confirmation of the
silenced log pending.
---
 src/helpers/CommonCLI.cpp | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp
index b4c2ed55..02569e4c 100644
--- a/src/helpers/CommonCLI.cpp
+++ b/src/helpers/CommonCLI.cpp
@@ -490,8 +490,16 @@ public:
     _finished = false;
     _open = false;
     _bytes_written = 0;
-    _fs->remove("/mqtt_prefs.tmp");  // clear only a stale, unpublished transaction
-    if (_fs->exists("/mqtt_prefs.tmp")) return false;
+    // Clear only a stale, unpublished transaction. Guarded on exists(): on the
+    // normal path commit() renames the tmp away, so there is nothing to remove
+    // and an unconditional remove() makes the ESP32 VFS layer log
+    // "[E] vfs_api.cpp remove(): /mqtt_prefs.tmp does not exists" on EVERY save.
+    // Functionally harmless, but it looks like a fault to operators reading the
+    // serial log during a config change.
+    if (_fs->exists("/mqtt_prefs.tmp")) {
+      _fs->remove("/mqtt_prefs.tmp");
+      if (_fs->exists("/mqtt_prefs.tmp")) return false;  // could not clear it
+    }
 #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
     _file = _fs->open("/mqtt_prefs.tmp", FILE_O_WRITE);
 #elif defined(RP2040_PLATFORM)
@@ -535,7 +543,7 @@ public:
     if (_open) _file.close();
     _open = false;
     _finished = false;
-    _fs->remove("/mqtt_prefs.tmp");
+    if (_fs->exists("/mqtt_prefs.tmp")) _fs->remove("/mqtt_prefs.tmp");
   }
 
 private:
@@ -562,8 +570,13 @@ public:
     // The old-name migration only starts when /com_prefs is absent. Refuse to
     // overwrite a destination that appeared unexpectedly before this handoff.
     if (_fs->exists("/com_prefs")) return false;
-    _fs->remove("/com_prefs.tmp");  // clear only stale, unpublished output
-    if (_fs->exists("/com_prefs.tmp")) return false;
+    // Clear only stale, unpublished output. Guarded on exists() for the same
+    // reason as the /mqtt_prefs store above: remove() on a missing file logs a
+    // spurious VFS error on ESP32.
+    if (_fs->exists("/com_prefs.tmp")) {
+      _fs->remove("/com_prefs.tmp");
+      if (_fs->exists("/com_prefs.tmp")) return false;  // could not clear it
+    }
 #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
     _file = _fs->open("/com_prefs.tmp", FILE_O_WRITE);
 #elif defined(RP2040_PLATFORM)
@@ -607,7 +620,7 @@ public:
     if (_open) _file.close();
     _open = false;
     _finished = false;
-    _fs->remove("/com_prefs.tmp");
+    if (_fs->exists("/com_prefs.tmp")) _fs->remove("/com_prefs.tmp");
   }
 
 private: