Report uncertain preference saves without promising rollback

.github/workflows/run-unit-tests.yml

@@ -360,6 +360,8 @@ jobs:
           python3 -B test/test_companion_radio_gain_restore.py -v
           python3 -B test/test_common_radio_persistence.py -v
           python3 -B test/test_common_cli_save_results.py -v
+          python3 -B test/test_common_prefs_uncertain_reply.py -v
+          python3 -B test/test_common_prefs_apply_reply.py -v
           python3 -B test/test_common_prefs_commit.py -v
           python3 -B test/test_common_prefs_legacy_rxgain.py -v
           python3 -B test/test_flood_policy_capacity.py -v

src/helpers/CommonCLI.cpp

@@ -2852,6 +2852,9 @@ void CommonCLI::savePrefs(PrefsSaveRouting::Scope scope) {
   }
 }
 
+// Failure does not prove the previous image is still on disk: publication can
+// succeed before readback fails. Callers can restore runtime values, but must
+// describe the durable save as unconfirmed rather than promise disk rollback.
 bool CommonCLI::trySavePrefs(PrefsSaveRouting::Scope scope) {
   const PrefsSaveRouting::Plan plan = PrefsSaveRouting::planFor(scope);
   const uint8_t old_advert_interval = _prefs->advert_interval;
@@ -3242,7 +3245,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, char* command, char* re
       StrHelper::strncpy(_prefs->password, &command[9], sizeof(_prefs->password));
       if (!trySavePrefs()) {
         memcpy(_prefs->password, previous, sizeof(previous));
-        strcpy(reply, "Error: password not saved; unchanged");
+        strcpy(reply, "Error: save unconfirmed; reboot may differ");
         return;
       }
       sprintf(reply, "password now: %s", _prefs->password);   // echo back just to let admin know for sure!!
@@ -3827,7 +3830,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
     _prefs->trace_when_repeat_off = strcmp(value, "on") == 0 ? 1 : 0;
     if (!trySavePrefs()) {
       _prefs->trace_when_repeat_off = previous;
-      strcpy(reply, "Error: Repeat trace not saved; unchanged");
+      strcpy(reply, "Error: save unconfirmed; reboot may differ");
       return;
     }
     _callbacks->onRetryConfigChanged();
@@ -3865,7 +3868,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
       _prefs->gps_sync_interval_hours = hours;
       if (!trySavePrefs()) {
         _prefs->gps_sync_interval_hours = previous;
-        strcpy(reply, "Error: GPS sync interval could not be saved");
+        strcpy(reply, "Error: save unconfirmed; reboot may differ");
       } else {
         _sensors->applyGpsTimeSyncInterval(hours);
         snprintf(reply, 160, "OK - GPS sync interval %u hours (saved)", (unsigned)hours);
@@ -3923,7 +3926,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
       _prefs->usb_logging_enabled = enabled ? 1 : 0;
       if (!trySavePrefs()) {
         _prefs->usb_logging_enabled = previous;
-        strcpy(reply, "Error: USB logging not saved; unchanged");
+        strcpy(reply, "Error: save unconfirmed; reboot may differ");
         return;
       }
       // Commit before changing the stream's live protocol ownership.
@@ -3945,7 +3948,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
       _prefs->usb_debug_enabled = enabled ? 1 : 0;
       if (!trySavePrefs()) {
         _prefs->usb_debug_enabled = previous;
-        strcpy(reply, "Error: USB debug not saved; unchanged");
+        strcpy(reply, "Error: save unconfirmed; reboot may differ");
         return;
       }
       mesh::setUsbDebugEnabled(enabled);
@@ -3978,7 +3981,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
     if (!trySavePrefs()) {
       _prefs->usb_logging_enabled = previous_usb;
       _prefs->bridge_enabled = previous_bridge;
-      strcpy(reply, "Error: logging output not saved; unchanged");
+      strcpy(reply, "Error: save unconfirmed; reboot may differ");
       return;
     }
     mesh::setUsbLoggingEnabled(usb_enabled);
@@ -4053,7 +4056,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
       // intent first so a failed write leaves the running service unchanged.
       if (!trySavePrefs()) {
         _prefs->bridge_enabled = previous;
-        strcpy(reply, "Error: MQTT setting not saved; unchanged");
+        strcpy(reply, "Error: save unconfirmed; reboot may differ");
         return;
       }
       const bool applied = enable ? _callbacks->setMqttBridgeState(true)
@@ -4075,7 +4078,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
     _prefs->rs232_heartbeat = enable ? 1 : 0;
     if (!trySavePrefs()) {
       _prefs->rs232_heartbeat = previous;
-      strcpy(reply, "Error: RS232 heartbeat not saved; unchanged");
+      strcpy(reply, "Error: save unconfirmed; reboot may differ");
     } else {
       strcpy(reply, enable ? "OK - RS232 heartbeat enabled" : "OK - heartbeat and standby disabled");
     }
@@ -4092,7 +4095,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
     _prefs->rs232_autosleep = force ? 2 : enable ? 1 : 0;
     if (!trySavePrefs()) {
       _prefs->rs232_autosleep = previous;
-      strcpy(reply, "Error: RS232 autosleep not saved; unchanged");
+      strcpy(reply, "Error: save unconfirmed; reboot may differ");
     } else {
       strcpy(reply, force ? "OK - forced standby; stock/unknown peers can miss packets"
           : enable ? "OK - standby after peer heartbeat confirmed; stock stays active"
@@ -4112,7 +4115,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
     _prefs->rs232_idle_seconds = static_cast<uint16_t>(seconds);
     if (!trySavePrefs()) {
       _prefs->rs232_idle_seconds = previous;
-      strcpy(reply, "Error: RS232 idle not saved; unchanged");
+      strcpy(reply, "Error: save unconfirmed; reboot may differ");
     } else {
       sprintf(reply, "OK - RS232 idle %us", static_cast<unsigned>(seconds));
     }
@@ -4147,9 +4150,11 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
       // Saved-on may currently be in soft standby. Restore an owner as well
       // as its intent, otherwise no object remains to execute the wake timer.
       const bool restored = _callbacks->setRs232BridgeState(previous_enabled != 0);
-      strcpy(reply, restored
-          ? "Error: RS232 change failed; setting unchanged"
-          : "Error: RS232 change failed; previous runtime could not be restored");
+      strcpy(reply, applied
+          ? (restored ? "Error: save unconfirmed; reboot may differ"
+                      : "Error: save unconfirmed; runtime restore failed; reboot may differ")
+          : (restored ? "Error: RS232 change failed; setting unchanged"
+                      : "Error: RS232 change failed; previous runtime could not be restored"));
     } else {
       strcpy(reply, enable && _sensors->gpsUsesSerialUart(_prefs->bridge_uart)
           ? "OK - UART GPS paused" : "OK");
@@ -4313,7 +4318,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
       _prefs->flood_advert_interval = (uint8_t)(hours);
       if (!trySavePrefs()) {
         _prefs->flood_advert_interval = previous;
-        strcpy(reply, "Error: interval not saved; unchanged");
+        strcpy(reply, "Error: save unconfirmed; reboot may differ");
         return;
       }
       _callbacks->updateFloodAdvertTimer();
@@ -4329,7 +4334,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
       _prefs->advert_interval = (uint8_t)(mins / 2);
       if (!trySavePrefs()) {
         _prefs->advert_interval = previous;
-        strcpy(reply, "Error: interval not saved; unchanged");
+        strcpy(reply, "Error: save unconfirmed; reboot may differ");
         return;
       }
       _callbacks->updateAdvertTimer();
@@ -4341,7 +4346,7 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
     StrHelper::strncpy(_prefs->guest_password, &config[15], sizeof(_prefs->guest_password));
     if (!trySavePrefs()) {
       memcpy(_prefs->guest_password, previous, sizeof(previous));
-      strcpy(reply, "Error: guest password not saved; unchanged");
+      strcpy(reply, "Error: save unconfirmed; reboot may differ");
       return;
     }
     strcpy(reply, "OK");
@@ -5103,9 +5108,11 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
           // non-running heap-backed bridge instance.
           restored = _callbacks->setRs232BridgeState(previous_running);
         }
-        strcpy(reply, restored
-            ? "Error: bridge state change failed; setting unchanged"
-            : "Error: bridge state change failed; previous runtime state could not be restored");
+        strcpy(reply, applied
+            ? (restored ? "Error: save unconfirmed; reboot may differ"
+                        : "Error: save unconfirmed; runtime restore failed; reboot may differ")
+            : (restored ? "Error: bridge state change failed; setting unchanged"
+                        : "Error: bridge state change failed; previous runtime state could not be restored"));
       }
 #else
       // MQTT and ESP-NOW preserve configured intent across missing WiFi
@@ -5176,9 +5183,11 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
         const bool restored = previous_running
             ? _callbacks->restartRs232Bridge()
             : _callbacks->setRs232BridgeState(false);
-        strcpy(reply, restored
-            ? "Error: bridge failed to restart; baud unchanged"
-            : "Error: bridge failed to restart; previous baud could not be restored");
+        strcpy(reply, applied
+            ? (restored ? "Error: save unconfirmed; reboot may differ"
+                        : "Error: save unconfirmed; runtime restore failed; reboot may differ")
+            : (restored ? "Error: bridge failed to restart; baud unchanged"
+                        : "Error: bridge failed to restart; previous baud could not be restored"));
       }
     } else {
       sprintf(reply, "Error: baud rate must be between 9600-%d",BRIDGE_MAX_BAUD);
@@ -5231,9 +5240,11 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep
         const bool restored = previous_running
             ? _callbacks->restartRs232Bridge()
             : _callbacks->setRs232BridgeState(false);
-        strcpy(reply, restored
-            ? "Error: bridge failed to restart; UART unchanged"
-            : "Error: bridge failed to restart; previous UART could not be restored");
+        strcpy(reply, applied
+            ? (restored ? "Error: save unconfirmed; reboot may differ"
+                        : "Error: save unconfirmed; runtime restore failed; reboot may differ")
+            : (restored ? "Error: bridge failed to restart; UART unchanged"
+                        : "Error: bridge failed to restart; previous UART could not be restored"));
       } else if (
 #ifdef WITH_MQTT_BRIDGE
                  _prefs->rs232_bridge_enabled

src/helpers/CommonCLI_Observer.cpp

@@ -1326,7 +1326,7 @@ bool CommonCLI::handleObserverCommand(uint32_t sender_timestamp, char* command,
         _prefs->ota_channel = ch;
         if (!saveCommonPrefs()) {
           _prefs->ota_channel = previous;
-          strcpy(reply, "ERR: OTA channel save failed; setting unchanged");
+          strcpy(reply, "ERR: save unconfirmed; reboot may differ");
         } else {
           snprintf(reply, 160, "channel set to %s, base %s; run ota update to switch",
                    ota_channel_name(ch), ota_resolve_base(ch));

src/helpers/PeripheralModePrefs.h

@@ -92,7 +92,7 @@ bool handleCommand(uint8_t& shared, const char* command, char* reply,
   shared = strcmp(value, "shared") == 0 ? 1 : 0;
   if (!save()) {
     shared = previous;
-    snprintf(reply, capacity, "Error: peripheral mode could not be saved");
+    snprintf(reply, capacity, "Error: save unconfirmed; reboot may differ");
   } else {
     snprintf(reply, capacity, "OK - %s saved; reboot to apply", value);
   }

src/helpers/PrefsSaveReplyGuard.h

@@ -15,7 +15,7 @@ public:
       : _failures(failures), _start(failures), _reply(reply) {}
   ~PrefsSaveReplyGuard() {
     if (_failures != _start) {
-      strcpy(_reply, "Error: settings not saved; unsaved changes may remain active until reboot");
+      strcpy(_reply, "Error: save unconfirmed; changes may be active; reboot may differ");
     }
   }
 };

test/fixtures/common_cli_save_results/main.cpp

@@ -112,7 +112,7 @@ int main() {
     cli.callbacks.common_ok = false;
     if (guest) cli.set("guest.password new-guest", reply);
     else cli.password("password new-admin", reply);
-    assert(error(reply) && strstr(reply, "unchanged"));
+    assert(error(reply) && strstr(reply, "unconfirmed") && strstr(reply, "reboot"));
     assert(strcmp(cli.prefs.password, "old-admin") == 0);
     assert(strcmp(cli.prefs.guest_password, "old-guest") == 0);
     assert(cli.prefs.advert_interval == 2 && cli.callbacks.local_updates == 0);
@@ -143,7 +143,7 @@ int main() {
     assert(error(reply) && cli.callbacks.saves == 0);
     cli.callbacks.common_ok = false;
     cli.set((std::string(key) + (flood ? "3" : "120")).c_str(), reply);
-    assert(error(reply) && strstr(reply, "unchanged"));
+    assert(error(reply) && strstr(reply, "unconfirmed") && strstr(reply, "reboot"));
     assert(cli.prefs.advert_interval == 30 && cli.prefs.flood_advert_interval == 12);
     assert(cli.callbacks.local_updates == 0 && cli.callbacks.flood_updates == 0);
     cli.callbacks.common_ok = true;
@@ -188,7 +188,8 @@ int main() {
     cli.prefs.dirty = false;
     cli.callbacks.common_ok = false;
     cli.set("multi.acks 2", reply);
-    assert(error(reply) && strstr(reply, "unsaved changes"));
+    assert(error(reply) && strstr(reply, "unconfirmed") && strstr(reply, "may be active")
+           && strstr(reply, "reboot may differ"));
     assert(cli.prefs.multi_acks == 2 && cli.callbacks.persisted.multi_acks == 0);
     assert(cli.prefs.dirty && cli._prefs_save_failures == 1);
     cli.callbacks.common_ok = true;

test/fixtures/common_prefs_apply_reply.cpp

@@ -0,0 +1,320 @@
+// Insert only actual production parsing and the four bounded CLI branches.
+#include <cassert>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+#include <string>
+#include <vector>
+#include <helpers/CLICommandUtils.h>
+
+#define BRIDGE_MAX_BAUD 115200
+
+struct NodePrefs {
+  uint8_t bridge_enabled = 0;
+  uint8_t rs232_bridge_enabled = 0;
+  uint32_t bridge_baud = 9600;
+  uint8_t bridge_uart = 1;
+  uint8_t espnow_bridge_enabled = 1;
+  bool operator==(const NodePrefs& other) const {
+    return bridge_enabled == other.bridge_enabled
+        && rs232_bridge_enabled == other.rs232_bridge_enabled
+        && bridge_baud == other.bridge_baud && bridge_uart == other.bridge_uart
+        && espnow_bridge_enabled == other.espnow_bridge_enabled;
+  }
+};
+
+struct Event {
+  std::string kind;
+  bool enabled;
+  NodePrefs prefs;
+};
+
+struct Sensors {
+  bool gps = false;
+  bool conflict = false;
+  bool yield = true;
+  bool gpsUsesSerialUart(uint8_t) const { return gps; }
+  bool gpsSerialTransportMayConflict(uint8_t) const { return conflict; }
+  bool gpsSerialTransportCanYield(uint8_t) const { return yield; }
+};
+
+struct Callbacks {
+  NodePrefs* prefs;
+  std::vector<Event> events;
+  std::vector<bool> responses;
+  size_t response_index = 0;
+  bool running = false;
+  bool save_result = true;
+  unsigned save_calls = 0;
+  unsigned running_queries = 0;
+
+  bool result(const char* kind, bool enabled) {
+    events.push_back({kind, enabled, *prefs});
+    assert(response_index < responses.size());
+    const bool accepted = responses[response_index++];
+    // A failed apply can have torn down its previous runtime owner. This
+    // forces rollback rather than accidentally hiding that branch in tests.
+    running = accepted && enabled;
+    return accepted;
+  }
+  bool setRs232BridgeState(bool enabled) { return result("state", enabled); }
+  bool restartRs232Bridge() { return result("restart", true); }
+  bool isRs232BridgeRunning() { ++running_queries; return running; }
+  bool save() {
+    events.push_back({"save", false, *prefs});
+    ++save_calls;
+    return save_result;
+  }
+};
+
+struct CommonCLI {
+  NodePrefs* _prefs;
+  Callbacks* _callbacks;
+  Sensors* _sensors;
+  bool trySavePrefs() { return _callbacks->save(); }
+  void rs232(const char*, char*);
+  void bridge(const char*, char*);
+  void baud(const char*, char*);
+  void uart(const char*, char*);
+};
+
+@PARSE_ON_OFF@
+@PRODUCTION_BRANCHES@
+
+enum class Branch { Rs232, Bridge, Baud, Uart };
+
+static bool isEnable(Branch branch) {
+  return branch == Branch::Rs232 || branch == Branch::Bridge;
+}
+
+static uint8_t& intent(NodePrefs& prefs) {
+#ifdef WITH_MQTT_BRIDGE
+  return prefs.rs232_bridge_enabled;
+#else
+  return prefs.bridge_enabled;
+#endif
+}
+
+static NodePrefs initial(bool enabled) {
+  NodePrefs prefs;
+  intent(prefs) = enabled;
+#ifdef WITH_MQTT_BRIDGE
+  prefs.bridge_enabled = !enabled;  // Independent MQTT intent must survive.
+#else
+  prefs.rs232_bridge_enabled = enabled;
+#endif
+  return prefs;
+}
+
+static void run(CommonCLI& cli, Branch branch, bool enabled, char* reply,
+                bool unchanged = false) {
+  switch (branch) {
+    case Branch::Rs232:
+      cli.rs232(enabled ? "rs232.enabled on" : "rs232.enabled off", reply);
+      break;
+    case Branch::Bridge:
+#ifndef WITH_MQTT_BRIDGE
+      cli.bridge(enabled ? "bridge.enabled on" : "bridge.enabled off", reply);
+#else
+      assert(false && "standalone bridge branch is not an MQTT command");
+#endif
+      break;
+    case Branch::Baud:
+      cli.baud(unchanged ? "bridge.baud 9600" : "bridge.baud 19200", reply);
+      break;
+    case Branch::Uart:
+      cli.uart(unchanged ? "bridge.uart 1" : "bridge.uart 2", reply);
+      break;
+  }
+}
+
+static NodePrefs candidateFor(NodePrefs before, Branch branch, bool enabled,
+                              bool unchanged = false) {
+  if (isEnable(branch)) intent(before) = enabled;
+  else if (branch == Branch::Baud) before.bridge_baud = unchanged ? 9600 : 19200;
+  else before.bridge_uart = unchanged ? 1 : 2;
+  return before;
+}
+
+static const char* failedReply(Branch branch, bool applied, bool restored) {
+  if (applied) {
+    return restored ? "Error: save unconfirmed; reboot may differ"
+                    : "Error: save unconfirmed; runtime restore failed; reboot may differ";
+  }
+  switch (branch) {
+    case Branch::Rs232:
+      return restored ? "Error: RS232 change failed; setting unchanged"
+                      : "Error: RS232 change failed; previous runtime could not be restored";
+    case Branch::Bridge:
+      return restored ? "Error: bridge state change failed; setting unchanged"
+                      : "Error: bridge state change failed; previous runtime state could not be restored";
+    case Branch::Baud:
+      return restored ? "Error: bridge failed to restart; baud unchanged"
+                      : "Error: bridge failed to restart; previous baud could not be restored";
+    case Branch::Uart:
+      return restored ? "Error: bridge failed to restart; UART unchanged"
+                      : "Error: bridge failed to restart; previous UART could not be restored";
+  }
+  assert(false);
+  return "";
+}
+
+static unsigned cases = 0;
+
+static void matrixCase(Branch branch, bool previous, bool applied, bool saved,
+                       bool restored) {
+  NodePrefs prefs = initial(previous);
+  const NodePrefs before = prefs;
+  const bool requested = !previous;
+  const NodePrefs candidate = candidateFor(before, branch, requested);
+  Callbacks callbacks{&prefs, {}, {applied, restored}};
+  callbacks.running = previous;
+  callbacks.save_result = saved;
+  Sensors sensors;
+  CommonCLI cli{&prefs, &callbacks, &sensors};
+  char reply[160] = {};
+  run(cli, branch, requested, reply);
+  assert(callbacks.save_calls == (applied ? 1u : 0u));
+  assert(callbacks.events[0].kind == (isEnable(branch) ? "state" : "restart"));
+  assert(callbacks.events[0].prefs == candidate);
+  if (isEnable(branch)) assert(callbacks.events[0].enabled == requested);
+  if (applied) {
+    assert(callbacks.events[1].kind == "save");
+    assert(callbacks.events[1].prefs == candidate);
+  }
+  if (applied && saved) {
+    assert(!strcmp(reply, "OK"));
+    assert(prefs == candidate);
+    assert(callbacks.events.size() == 2);
+    assert(callbacks.response_index == 1);
+  } else {
+    assert(!strcmp(reply, failedReply(branch, applied, restored)));
+    assert(prefs == before);
+    assert(callbacks.events.size() == (applied ? 3u : 2u));
+    const Event& rollback = callbacks.events.back();
+    assert(rollback.kind == (isEnable(branch) ? "state" : "restart"));
+    assert(rollback.prefs == before);
+    assert(rollback.enabled == (isEnable(branch) ? previous : true));
+    assert(callbacks.response_index == 2);
+    if (applied) {
+      assert(strstr(reply, "unconfirmed"));
+      assert(strstr(reply, "reboot may differ"));
+      assert(!strstr(reply, "unchanged"));
+    } else {
+      assert(!strstr(reply, "unconfirmed"));
+    }
+  }
+  ++cases;
+}
+
+static void disabledSettingCase(Branch branch, bool saved, bool restored) {
+  NodePrefs prefs = initial(false);
+  const NodePrefs before = prefs;
+  const NodePrefs candidate = candidateFor(before, branch, false);
+  Callbacks callbacks{&prefs, {}, {restored}};
+  callbacks.save_result = saved;
+  Sensors sensors;
+  CommonCLI cli{&prefs, &callbacks, &sensors};
+  char reply[160] = {};
+  run(cli, branch, false, reply);
+  // A disabled bridge needs no runtime restart to apply UART/baud changes.
+  assert(callbacks.save_calls == 1);
+  assert(callbacks.events[0].kind == "save");
+  assert(callbacks.events[0].prefs == candidate);
+  if (saved) {
+    assert(!strcmp(reply, "OK"));
+    assert(prefs == candidate);
+    assert(callbacks.events.size() == 1);
+    assert(callbacks.response_index == 0);
+  } else {
+    assert(!strcmp(reply, failedReply(branch, true, restored)));
+    assert(prefs == before);
+    assert(callbacks.events.size() == 2);
+    assert(callbacks.events[1].kind == "state");
+    assert(!callbacks.events[1].enabled);
+    assert(callbacks.events[1].prefs == before);
+    assert(callbacks.response_index == 1);
+  }
+  ++cases;
+}
+
+static void healthyCase(Branch branch, bool previous, bool gps, bool unchanged) {
+  NodePrefs prefs = initial(previous);
+  const NodePrefs before = prefs;
+  const bool requested = unchanged ? previous : !previous;
+  const NodePrefs candidate = candidateFor(before, branch, requested, unchanged);
+  const bool applies = isEnable(branch) || previous;
+  Callbacks callbacks{&prefs, {}, {true}};
+  callbacks.running = previous;
+  Sensors sensors;
+  sensors.gps = gps;
+  CommonCLI cli{&prefs, &callbacks, &sensors};
+  char reply[160] = {};
+  run(cli, branch, requested, reply, unchanged);
+  const bool paused = gps && ((isEnable(branch) && requested)
+                              || (branch == Branch::Uart && previous));
+  assert(!strcmp(reply, paused ? "OK - UART GPS paused" : "OK"));
+  assert(prefs == candidate);
+  assert(callbacks.save_calls == 1);
+  assert(callbacks.events.size() == (applies ? 2u : 1u));
+  assert(callbacks.response_index == (applies ? 1u : 0u));
+  for (const Event& event : callbacks.events) assert(event.prefs == candidate);
+  ++cases;
+}
+
+#ifndef WITH_MQTT_BRIDGE
+static void alreadyRestoredCase() {
+  NodePrefs prefs = initial(true);
+  const NodePrefs before = prefs;
+  Callbacks callbacks{&prefs, {}, {true}};
+  callbacks.running = true;
+  callbacks.save_result = false;
+  Sensors sensors;
+  CommonCLI cli{&prefs, &callbacks, &sensors};
+  char reply[160] = {};
+  // A successfully applied same-value ON leaves the old runtime running.
+  // A refused disable also leaves it stopped in this adapter and must call
+  // the restoration callback instead, as covered by matrixCase above.
+  run(cli, Branch::Bridge, true, reply, true);
+  assert(!strcmp(reply, "Error: save unconfirmed; reboot may differ"));
+  assert(prefs == before);
+  assert(callbacks.events.size() == 2);
+  assert(callbacks.response_index == 1);
+  assert(callbacks.running_queries == 1);
+  ++cases;
+}
+#endif
+
+int main() {
+  const Branch branches[] = {Branch::Rs232,
+#ifndef WITH_MQTT_BRIDGE
+                            Branch::Bridge,
+#endif
+                            Branch::Baud, Branch::Uart};
+  for (Branch branch : branches) {
+    for (bool applied : {false, true}) {
+      for (bool saved : {false, true}) {
+        for (bool restored : {false, true}) {
+          matrixCase(branch, true, applied, saved, restored);
+          if (isEnable(branch)) matrixCase(branch, false, applied, saved, restored);
+        }
+      }
+    }
+    for (bool previous : {false, true}) {
+      for (bool gps : {false, true}) {
+        for (bool unchanged : {false, true}) {
+          healthyCase(branch, previous, gps, unchanged);
+        }
+      }
+    }
+  }
+  for (Branch branch : {Branch::Baud, Branch::Uart}) {
+    for (bool saved : {false, true}) {
+      for (bool restored : {false, true}) disabledSettingCase(branch, saved, restored);
+    }
+  }
+#ifndef WITH_MQTT_BRIDGE
+  alreadyRestoredCase();
+#endif
+  printf("%u actual CommonCLI apply/save/restore cases passed\n", cases);
+}

test/fixtures/common_prefs_uncertain_reply/main.cpp

@@ -0,0 +1,209 @@
+
+// Included after the extracted production CommonCLI methods and serializer.
+#include <cstdio>
+
+static const char* oldSecret(bool guest) { return guest ? "old-guest" : "old-admin"; }
+static const char* newSecret(bool guest) { return guest ? "new-guest" : "new-admin"; }
+static const char* retrySecret(bool guest) { return guest ? "retry-guest" : "retry-admin"; }
+static const char* currentSecret(const CommonCLI& cli, bool guest) {
+  return guest ? cli.prefs.guest_password : cli.prefs.password;
+}
+
+static void seed(CommonCLI& cli) {
+  strcpy(cli.prefs.password, oldSecret(false));
+  strcpy(cli.prefs.guest_password, oldSecret(true));
+  cli.savePrefs(&cli.fs, PrefsSaveRouting::Scope::Common);
+  assert(cli._common_save_result_known && cli._common_save_succeeded);
+}
+
+static void change(CommonCLI& cli, bool guest, const char* secret, char* reply) {
+  char command[80] = {};
+  snprintf(command, sizeof(command), "%s%s", guest ? "guest.password " : "password ", secret);
+  memset(reply, 0, 160);
+  if (guest) cli.guestPassword(command, reply);
+  else cli.password(command, reply);
+}
+
+static void assertError(const char* reply, bool guest, const char* attempted) {
+  assert(strncmp(reply, "Error:", 6) == 0);
+  assert(strstr(reply, "unconfirmed") != nullptr);
+  assert(strstr(reply, "reboot") != nullptr);
+  assert(strstr(reply, "unchanged") == nullptr);
+  assert(strstr(reply, "OK") == nullptr);
+  // Neither an old credential nor any attempted credential belongs in an
+  // error, even though the successful admin command intentionally echoes it.
+  for (const char* secret : {oldSecret(false), oldSecret(true),
+                            newSecret(false), newSecret(true), attempted}) {
+    assert(strstr(reply, secret) == nullptr);
+  }
+  (void)guest;
+}
+
+static void assertSuccess(const CommonCLI& cli, bool guest,
+                          const char* secret, const char* reply) {
+  assert(cli._common_save_result_known && cli._common_save_succeeded);
+  assert(strcmp(currentSecret(cli, guest), secret) == 0);
+  if (guest) assert(strcmp(reply, "OK") == 0);
+  else {
+    char expected[80];
+    snprintf(expected, sizeof(expected), "password now: %s", secret);
+    assert(strcmp(reply, expected) == 0);
+  }
+}
+
+static void assertReboot(const CommonCLI& cli, bool guest, const char* expected) {
+  CommonCLI reboot;
+  reboot.fs.files = cli.fs.files;
+  reboot.loadPrefsInt(&reboot.fs, "/com_prefs");
+  assert(strcmp(currentSecret(reboot, guest), expected) == 0);
+  assert(strcmp(currentSecret(reboot, !guest), oldSecret(!guest)) == 0);
+}
+
+static void clearFaults(CommonCLI& cli) {
+  cli.fs.fail_write = false;
+  cli.fs.fail_write_after = -1;
+  cli.fs.fail_read_open = false;
+  cli.fs.fail_after_publication = false;
+  cli.fs.fail_rename = 0;
+}
+
+static void assertLegacyError(const CommonCLI& cli, const char* reply) {
+  assert(!cli._common_save_succeeded && cli._prefs_save_failures == 1);
+  assert(cli.prefs.dirty);
+  assert(strncmp(reply, "Error:", 6) == 0);
+  assert(strstr(reply, "unconfirmed") != nullptr);
+  assert(strstr(reply, "active") != nullptr);
+  assert(strstr(reply, "reboot") != nullptr);
+  assert(strstr(reply, "unchanged") == nullptr && strstr(reply, "OK") == nullptr);
+  assert(strstr(reply, "unsaved") == nullptr);
+}
+
+static void assertLegacyReboot(const CommonCLI& cli, uint8_t expected) {
+  CommonCLI reboot;
+  reboot.fs.files = cli.fs.files;
+  reboot.loadPrefsInt(&reboot.fs, "/com_prefs");
+  assert(reboot.prefs.multi_acks == expected);
+  assert(strcmp(reboot.prefs.password, oldSecret(false)) == 0);
+  assert(strcmp(reboot.prefs.guest_password, oldSecret(true)) == 0);
+}
+
+int main() {
+  unsigned scenarios = 0;
+  for (bool guest : {false, true}) {
+    char reply[160] = {};
+    CommonCLI healthy;
+    seed(healthy);
+    const auto previous = healthy.fs.files.at("/com_prefs");
+    change(healthy, guest, newSecret(guest), reply);
+    assertSuccess(healthy, guest, newSecret(guest), reply);
+    assertReboot(healthy, guest, newSecret(guest));
+    const auto candidate = healthy.fs.files.at("/com_prefs");
+    assert(candidate != previous && candidate.size() == previous.size());
+    ++scenarios;
+
+    // Each ordinary rejection leaves the old image on disk. The bool API
+    // cannot promise that for every false result, so its reply stays cautious.
+    for (unsigned fault = 0; fault != 4; ++fault) {
+      CommonCLI cli;
+      seed(cli);
+      if (fault == 0) cli.fs.fail_write = true;
+      if (fault == 1) cli.fs.fail_write_after = 64;
+      if (fault == 2) cli.fs.fail_read_open = true;
+      if (fault == 3) cli.fs.fail_rename = 1;
+      change(cli, guest, newSecret(guest), reply);
+      assert(!cli._common_save_succeeded);
+      assertError(reply, guest, newSecret(guest));
+      assert(strcmp(currentSecret(cli, guest), oldSecret(guest)) == 0);
+      assert(cli.fs.files.at("/com_prefs") == previous);
+      clearFaults(cli);
+      assertReboot(cli, guest, oldSecret(guest));
+      change(cli, guest, retrySecret(guest), reply);
+      assertSuccess(cli, guest, retrySecret(guest), reply);
+      assertReboot(cli, guest, retrySecret(guest));
+      ++scenarios;
+    }
+
+#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
+    // Atomic publication succeeded, then every public-image readback failed.
+    // The real writer returns false/Indeterminate, runtime credentials roll
+    // back, but fresh boot must read the new durable credential. An "unchanged"
+    // promise here is both false and dangerous for remote administration.
+    {
+      CommonCLI cli;
+      seed(cli);
+      cli.fs.fail_after_publication = true;
+      change(cli, guest, newSecret(guest), reply);
+      assert(!cli._common_save_succeeded);
+      assertError(reply, guest, newSecret(guest));
+      assert(strcmp(currentSecret(cli, guest), oldSecret(guest)) == 0);
+      assert(cli.fs.files.at("/com_prefs") == candidate);
+      clearFaults(cli);
+      assertReboot(cli, guest, newSecret(guest));
+      change(cli, guest, retrySecret(guest), reply);
+      assertSuccess(cli, guest, retrySecret(guest), reply);
+      assertReboot(cli, guest, retrySecret(guest));
+      ++scenarios;
+    }
+#endif
+  }
+
+  // Legacy setters retain their live changes after save failure. Exercise the
+  // actual outer reply guard so the branch's final OK cannot hide either
+  // known rejection or post-publication uncertainty.
+  {
+    char reply[160] = {};
+    CommonCLI healthy;
+    seed(healthy);
+    assert(healthy.prefs.multi_acks == 0);
+    const auto previous = healthy.fs.files.at("/com_prefs");
+    healthy.legacySet("multi.acks 1", reply);
+    assert(strcmp(reply, "OK") == 0 && healthy.prefs.multi_acks == 1);
+    assert(healthy._common_save_succeeded && healthy._prefs_save_failures == 0);
+    assert(!healthy.prefs.dirty);
+    assertLegacyReboot(healthy, 1);
+    const auto candidate = healthy.fs.files.at("/com_prefs");
+    assert(candidate != previous && candidate.size() == previous.size());
+    ++scenarios;
+
+    for (unsigned fault = 0; fault != 4; ++fault) {
+      CommonCLI cli;
+      seed(cli);
+      if (fault == 0) cli.fs.fail_write = true;
+      if (fault == 1) cli.fs.fail_write_after = 64;
+      if (fault == 2) cli.fs.fail_read_open = true;
+      if (fault == 3) cli.fs.fail_rename = 1;
+      cli.legacySet("multi.acks 1", reply);
+      assertLegacyError(cli, reply);
+      assert(cli.prefs.multi_acks == 1);
+      assert(cli.fs.files.at("/com_prefs") == previous);
+      clearFaults(cli);
+      assertLegacyReboot(cli, 0);
+      // Retry the same live value: an unchanged assignment must still save.
+      cli.legacySet("multi.acks 1", reply);
+      assert(strcmp(reply, "OK") == 0 && cli._common_save_succeeded);
+      assert(cli._prefs_save_failures == 1 && !cli.prefs.dirty);
+      assertLegacyReboot(cli, 1);
+      ++scenarios;
+    }
+
+#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
+    {
+      CommonCLI cli;
+      seed(cli);
+      cli.fs.fail_after_publication = true;
+      cli.legacySet("multi.acks 1", reply);
+      assertLegacyError(cli, reply);
+      assert(cli.prefs.multi_acks == 1);
+      assert(cli.fs.files.at("/com_prefs") == candidate);
+      clearFaults(cli);
+      assertLegacyReboot(cli, 1);
+      cli.legacySet("multi.acks 0", reply);
+      assert(strcmp(reply, "OK") == 0 && cli._common_save_succeeded);
+      assert(cli._prefs_save_failures == 1 && !cli.prefs.dirty);
+      assertLegacyReboot(cli, 0);
+      ++scenarios;
+    }
+#endif
+  }
+  printf("%u actual CommonCLI save scenarios passed\n", scenarios);
+}

test/fixtures/rs232_mqtt_runtime.cpp

@@ -373,18 +373,18 @@ int main() {
   assert(cli.saves == disabled_saves && mesh.isMqttBridgeRunning() && mesh.isEspNowBridgeRunning());
   cli.fail_save = true;
   cli.set("rs232.enabled on", reply);
-  assert(strstr(reply, "setting unchanged") && !mesh.bridge && !mesh._prefs.rs232_bridge_enabled);
+  assert(strstr(reply, "unconfirmed") && strstr(reply, "reboot") && !mesh.bridge && !mesh._prefs.rs232_bridge_enabled);
   assert(cli.saves == disabled_saves && mesh.isMqttBridgeRunning() && mesh.isEspNowBridgeRunning());
   cli.fail_save = false;
   cli.set("rs232.enabled on", reply); assert(strcmp(reply, "OK") == 0);
   const unsigned saved_after_enable = cli.saves;
   cli.fail_save = true;
   cli.set("rs232.enabled off", reply);
-  assert(strstr(reply, "setting unchanged") && mesh.isRs232BridgeRunning() && mesh._prefs.rs232_bridge_enabled);
+  assert(strstr(reply, "unconfirmed") && strstr(reply, "reboot") && mesh.isRs232BridgeRunning() && mesh._prefs.rs232_bridge_enabled);
   cli.set("bridge.baud 19200", reply);
-  assert(strstr(reply, "baud unchanged") && mesh._prefs.bridge_baud == 57600 && Serial2.baud == 57600);
+  assert(strstr(reply, "unconfirmed") && strstr(reply, "reboot") && mesh._prefs.bridge_baud == 57600 && Serial2.baud == 57600);
   cli.set("bridge.uart 2", reply);
-  assert(strstr(reply, "UART unchanged") && mesh._prefs.bridge_uart == 2 && mesh.isRs232BridgeRunning());
+  assert(strstr(reply, "unconfirmed") && strstr(reply, "reboot") && mesh._prefs.bridge_uart == 2 && mesh.isRs232BridgeRunning());
   assert(cli.saves == saved_after_enable && mesh.mqtt_bridge->begins == wifi_starts);
   assert(mesh.espnow_bridge.begins == wireless_starts);
   cli.fail_save = false;
@@ -405,7 +405,7 @@ int main() {
   // A failed intent save must not signal an uncancellable worker stop.
   cli.fail_save = true;
   cli.set("mqtt.enabled off", reply);
-  assert(strstr(reply, "not saved; unchanged") && mesh._prefs.bridge_enabled);
+  assert(strstr(reply, "unconfirmed") && strstr(reply, "reboot") && mesh._prefs.bridge_enabled);
   assert(mesh.isMqttBridgeRunning() && !mesh.isMqttBridgeStopping());
   cli.fail_save = false;
   cli.set("mqtt.enabled off", reply);
@@ -427,7 +427,7 @@ int main() {
   expect_get(cli, "mqtt.stopping", "> off");
   cli.fail_save = true;
   cli.set("mqtt.enabled on", reply);
-  assert(strstr(reply, "not saved; unchanged") && !mesh._prefs.bridge_enabled);
+  assert(strstr(reply, "unconfirmed") && strstr(reply, "reboot") && !mesh._prefs.bridge_enabled);
   assert(!mesh.isMqttBridgeRunning());
   cli.fail_save = false;
   cli.set("espnow.enabled on", reply); assert(strcmp(reply, "OK") == 0);

test/test_common_cli_logging_transaction.py

@@ -57,7 +57,8 @@ static void check(const char* command, bool usb, bool wifi, bool failed) {
   assert(cli.persisted.usb_logging_enabled == (failed ? !usb : usb));
   assert(live_usb == (failed ? !usb : usb));
   assert(usb_calls == (failed ? 0u : 1u));
-  assert(strstr(reply, failed ? "unchanged" : "(saved)"));
+  assert(strstr(reply, failed ? "unconfirmed" : "(saved)"));
+  if (failed) assert(strstr(reply, "reboot may differ"));
   if (strncmp(command, "logging.output", 14) == 0) {
     assert(cli.prefs.bridge_enabled == (failed ? !wifi : wifi));
     assert(cli.persisted.bridge_enabled == (failed ? !wifi : wifi));

test/test_common_prefs_apply_reply.py

@@ -0,0 +1,85 @@
+"""Run real RS-232 apply/save branches with controlled callback outcomes.
+
+The callbacks model application and save results, not filesystem transactions.
+Actual post-publication storage failures are exercised separately by
+test_common_prefs_uncertain_reply.py.
+"""
+from pathlib import Path
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+
+from test_replay_reset_integration import extract_braced
+
+ROOT = Path(__file__).resolve().parents[1]
+
+
+def actual_harness(compiler, flags):
+    source = (ROOT / "src/helpers/CommonCLI.cpp").read_text(encoding="ascii")
+    # Preprocess the whole real method first: platform conditionals can
+    # straddle the closing brace between adjacent else-if commands.
+    selected = subprocess.run([
+        compiler, "-E", "-P", "-x", "c++", *flags, "-",
+    ], input=extract_braced(source, "void CommonCLI::handleSetCmd("),
+        capture_output=True, text=True, check=True, timeout=20).stdout
+    branches = []
+    for method, signature in (
+        ("rs232", 'if (strncmp(config, "rs232.enabled ", 14) == 0)'),
+        ("bridge", 'if (memcmp(config, "bridge.enabled ", 15) == 0)'),
+        ("baud", 'if (memcmp(config, "bridge.baud ", 12) == 0)'),
+        ("uart", 'if (memcmp(config, "bridge.uart ", 12) == 0)'),
+    ):
+        definition = ("void CommonCLI::" + method
+                      + "(const char* config, char* reply) {\n"
+                      + extract_braced(selected, signature) + "\n}\n")
+        if method == "bridge":
+            # The strict apply/save bridge.enabled branch is standalone UART
+            # only. WITH_MQTT_BRIDGE keeps its independent transport policy.
+            definition = "#ifndef WITH_MQTT_BRIDGE\n" + definition + "#endif\n"
+        branches.append(definition)
+    fixture = (ROOT / "test/fixtures/common_prefs_apply_reply.cpp").read_text(
+        encoding="ascii")
+    return fixture.replace("@PARSE_ON_OFF@", extract_braced(
+        source, "static bool parseOnOffStrict(")).replace(
+            "@PRODUCTION_BRANCHES@", "\n".join(branches))
+
+
+class CommonPrefsApplyReplyTests(unittest.TestCase):
+    def test_actual_apply_save_rollback_combinations(self):
+        compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++")
+        self.assertIsNotNone(compiler, "a host C++17 compiler is required")
+        sanitizer_flags = (["-fsanitize=address,undefined", "-fno-sanitize-recover=all",
+                            "-fno-pie", "-no-pie"]
+                           if sys.platform.startswith("linux") else [])
+        with tempfile.TemporaryDirectory(prefix="common-prefs-apply-reply-") as directory:
+            work = Path(directory)
+            for mqtt in (False, True):
+                with self.subTest(mqtt=mqtt):
+                    flags = ["-DWITH_BRIDGE=1", "-DWITH_RS232_BRIDGE=1",
+                             "-DWITH_RS232_BRIDGE_UART=1", "-DWITH_RS232_BRIDGE_ALT=1",
+                             "-DWITH_RS232_BRIDGE_ALT_UART=2"]
+                    if mqtt:
+                        flags.append("-DWITH_MQTT_BRIDGE=1")
+                    (work / "test.cpp").write_text(actual_harness(compiler, flags),
+                                                  encoding="ascii")
+                    binary = work / ("test.exe" if os.name == "nt" else "test")
+                    built = subprocess.run([
+                        compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror",
+                        *flags, *sanitizer_flags, "-I" + str(ROOT / "src"),
+                        str(work / "test.cpp"), "-o", str(binary),
+                    ], capture_output=True, text=True, timeout=60)
+                    self.assertEqual(built.returncode, 0, built.stdout + built.stderr)
+                    tested = subprocess.run([str(binary)], capture_output=True,
+                                            text=True, timeout=20)
+                    self.assertEqual(tested.returncode, 0, tested.stdout + tested.stderr)
+                    cases = 64 if mqtt else 89
+                    self.assertIn(
+                        f"{cases} actual CommonCLI apply/save/restore cases passed",
+                        tested.stdout)
+
+
+if __name__ == "__main__":
+    unittest.main()

test/test_common_prefs_uncertain_reply.py

@@ -0,0 +1,133 @@
+"""Exercise real CommonCLI password and legacy save uncertainty replies."""
+from pathlib import Path
+import os
+import re
+import subprocess
+import tempfile
+import unittest
+
+from test_common_usb_debug import HARNESS
+from test_replay_reset_integration import extract_braced
+
+ROOT = Path(__file__).resolve().parents[1]
+
+
+def actual_harness():
+    source = (ROOT / 'src/helpers/CommonCLI.cpp').read_text(encoding='ascii')
+    header = (ROOT / 'src/helpers/CommonCLI.h').read_text(encoding='ascii')
+    fields = header.split('class NodePrefs :', 1)[1].split('private:', 1)[0]
+    fields = '\n'.join(re.findall(
+        r'^\s*(?:float|double|char|u?int(?:8|16|32)_t)\s+[^;]+;', fields, re.M))
+    constants = '\n'.join(re.findall(
+        r'^\s*#define\s+\w+\s+(?:0x[\dA-Fa-f]+|\d+)\s*$', header, re.M))
+    constants += '\n' + '\n'.join(re.findall(
+        r'^static const size_t LEGACY_\w+\s*=[^;]+;', source, re.M))
+    constants += '\n' + re.search(
+        r'^static constexpr uint8_t DEFAULT_ADVERT_LOC_POLICY\s*=[^;]+;',
+        header, re.M).group()
+    helpers = '\n'.join(extract_braced(source, signature) for signature in (
+        'static bool bwMatches(', 'static bool isValidLoRaBandwidth(',
+        'static float defaultLoRaBandwidth(', 'static void markDirectRetryPrefsValid(',
+        'static void applyFloodRetryPreset(', 'static void applyDirectRetryPreset(',
+        'static void setDefaultDirectRetryPrefs(', 'static bool directRetryPrefsValid('))
+    methods = '\n'.join(extract_braced(source, signature) for signature in (
+        'static const char* commonPrefsSaveResultName(',
+        'bool CommonCLI::recoverCommonPrefsFiles(',
+        'bool CommonCLI::saveCommonPrefsImageAtomically(',
+        'void CommonCLI::savePrefs(FILESYSTEM*',
+        'void CommonCLI::savePrefs(PrefsSaveRouting::Scope scope)',
+        'bool CommonCLI::trySavePrefs(', 'void CommonCLI::loadPrefsInt('))
+
+    # Reuse the established adapter, but not its USB-specific main or dynamic
+    # schema tests. Production serializers and transaction code remain real.
+    code = HARNESS[:HARNESS.index('// Execute the production dynamic schema')]
+    code = code.replace('  void set(const char* config, char* reply) {',
+                        '  void password(const char*, char*);\n'
+                        '  void guestPassword(const char*, char*);\n'
+                        '  void legacySet(const char*, char*);\n'
+                        '  void set(const char* config, char* reply) {')
+    code = '#include <cstdlib>\n#include <helpers/PrefsSaveReplyGuard.h>\n' + code
+    text_helpers = (ROOT / 'src/helpers/TxtDataHelpers.cpp').read_text(encoding='ascii')
+    code = code.replace('struct CommonCLI;\n',
+                        'struct StrHelper { static void strncpy(char*, const char*, size_t); };\n'
+                        + extract_braced(text_helpers, 'void StrHelper::strncpy(')
+                        + '\nstruct CommonCLI;\n', 1)
+    for key, value in {
+        'FIELDS': fields, 'CONSTANTS': constants, 'HELPERS': helpers,
+        'LEGACY': extract_braced(header, 'struct LegacyObserverTail') + ';',
+        'SERIALIZER': 'template<typename Writer>\n' + extract_braced(
+            source, 'static bool writeCommonPrefsImage('),
+        'STORE': extract_braced(source, 'class CommonPrefsFileStore'),
+        'METHODS': methods,
+        'SET_DEBUG': extract_braced(source, 'if (strncmp(config, "usb.debug", 9)'),
+        'SET_MASTER': extract_braced(source, 'if (strncmp(config, "usb.logging", 11)'),
+        'GET_DEBUG': extract_braced(source, 'if (strcmp(config, "usb.debug") == 0)'),
+        'GET_MASTER': extract_braced(source, 'if (strcmp(config, "usb.logging") == 0)'),
+    }.items():
+        code = code.replace('@' + key + '@', value)
+    code += '\nvoid CommonCLI::password(const char* command, char* reply) {\n'
+    code += extract_braced(source, 'if (memcmp(command, "password ", 9) == 0)') + '\n}\n'
+    code += '\nvoid CommonCLI::guestPassword(const char* config, char* reply) {\n'
+    code += extract_braced(source, 'if (memcmp(config, "guest.password ", 15) == 0)') + '\n}\n'
+    handle = extract_braced(source, 'void CommonCLI::handleCommand(')
+    guard = re.search(r'PrefsSaveReplyGuard save_reply\([^;]+;', handle).group()
+    code += '\nvoid CommonCLI::legacySet(const char* config, char* reply) {\n'
+    code += guard + '\n'
+    code += extract_braced(source, 'if (memcmp(config, "multi.acks ", 11) == 0)') + '\n}\n'
+    return code + (ROOT / 'test/fixtures/common_prefs_uncertain_reply/main.cpp').read_text(
+        encoding='ascii')
+
+
+class CommonPrefsUncertainReplyTest(unittest.TestCase):
+    def test_actual_password_commands_and_durable_images(self):
+        code = actual_harness()
+        mock = (ROOT / 'test/fixtures/radio_profiles/mocks/helpers/IdentityStore.h').read_text(
+            encoding='ascii')
+        # Fail only after a real rename publishes the complete candidate. The
+        # real AtomicFileWriter still performs all its normal verification.
+        old_field = '  bool fail_read_open = false;'
+        self.assertEqual(mock.count(old_field), 1)
+        mock = mock.replace(old_field, old_field + '\n  bool fail_after_publication = false;')
+        old_rename = '    files[to] = files[from]; files.erase(from); return true;'
+        self.assertEqual(mock.count(old_rename), 1)
+        mock = mock.replace(old_rename,
+                            '    files[to] = files[from]; files.erase(from);\n'
+                            '    if (fail_after_publication && !strcmp(to, "/com_prefs"))\n'
+                            '      fail_read_open = true;\n'
+                            '    return true;')
+        sanitizer_flags = [] if os.name == 'nt' else [
+            '-fsanitize=address,undefined', '-fno-sanitize-recover=all',
+            '-fno-pie', '-no-pie']
+        with tempfile.TemporaryDirectory(prefix='common-prefs-uncertain-') as directory:
+            work = Path(directory)
+            (work / 'helpers').mkdir()
+            (work / 'helpers/IdentityStore.h').write_text(mock, encoding='ascii')
+            # Preserve the production transaction body; redirect only its
+            # quoted filesystem include to the same injected adapter.
+            transaction = (ROOT / 'src/helpers/ContactFileTransaction.h').read_text(
+                encoding='ascii')
+            self.assertEqual(transaction.count('#include "IdentityStore.h"'), 1)
+            (work / 'ContactFileTransaction.h').write_text(transaction.replace(
+                '#include "IdentityStore.h"', '#include <helpers/IdentityStore.h>'),
+                encoding='ascii')
+            (work / 'test.cpp').write_text(code, encoding='ascii')
+            for platform in ('NRF52_PLATFORM', 'STM32_PLATFORM',
+                             'ESP32_PLATFORM', 'RP2040_PLATFORM'):
+                with self.subTest(platform=platform):
+                    binary = work / ('test.exe' if os.name == 'nt' else 'test')
+                    built = subprocess.run([
+                        os.environ.get('CXX', 'g++'), '-std=c++17', '-DENABLE_OTA=1',
+                        '-D' + platform + '=1', *sanitizer_flags,
+                        '-I' + str(work), '-I' + str(ROOT / 'src'),
+                        '-I' + str(ROOT / 'src/helpers'), str(work / 'test.cpp'),
+                        '-o', str(binary)], capture_output=True, text=True, timeout=60)
+                    self.assertEqual(built.returncode, 0, built.stdout + built.stderr)
+                    tested = subprocess.run([str(binary)], capture_output=True,
+                                            text=True, timeout=20)
+                    self.assertEqual(tested.returncode, 0, tested.stdout + tested.stderr)
+                    scenarios = 18 if platform in ('NRF52_PLATFORM', 'STM32_PLATFORM') else 15
+                    self.assertIn(f'{scenarios} actual CommonCLI save scenarios passed', tested.stdout)
+
+
+if __name__ == '__main__':
+    unittest.main()

test/test_common_usb_debug.py

@@ -213,7 +213,7 @@ int main() {
     if (fault == 3) cli.fs.fail_rename = 1;
     const unsigned updates = mesh::debug_updates;
     cli.set("usb.debug on", reply);
-    assert(strcmp(reply, "Error: USB debug not saved; unchanged") == 0);
+    assert(strcmp(reply, "Error: save unconfirmed; reboot may differ") == 0);
     assert(cli.prefs.usb_debug_enabled == 0 && !mesh::isUsbDebugEnabled());
     assert(mesh::debug_updates == updates && !mesh::isUsbLoggingEnabled());
     assert(cli.fs.files["/com_prefs"] == debug_off);

test/test_gps_sync_preferences.py

@@ -86,7 +86,7 @@ int main() {
   }
   common.save_ok=companion.save_ok=false;
   const unsigned common_applied=common.sensors.applications, companion_applied=companion.sensors.applications;
-  common.set("gps.sync.interval 7",reply);assert(strstr(reply,"could not be saved"));
+  common.set("gps.sync.interval 7",reply);assert(strstr(reply,"unconfirmed")&&strstr(reply,"reboot"));
   assert(companion.command("set gps.sync.interval 7",reply,sizeof(reply)) && strstr(reply,"could not be saved"));
   assert(common.prefs.gps_sync_interval_hours==336 && common.persisted.gps_sync_interval_hours==336);
   assert(companion._prefs.gps_sync_interval_hours==336 && companion.persisted.gps_sync_interval_hours==336);

test/test_observer_ota_channel.py

@@ -183,7 +183,7 @@ int main() {
   cli.save_ok=false;
   assert(cli.run("ota branch prod", reply));
   assert(cli.prefs.ota_channel==OTA_CH_DEV && cli.persisted==OTA_CH_DEV);
-  assert(strstr(reply, "ERR: OTA channel save failed"));
+  assert(strstr(reply, "ERR: save unconfirmed") && strstr(reply, "reboot may differ"));
   const int saves=cli.saves;
   assert(cli.run("ota branch Beta", reply) && cli.saves==saves);
   assert(!cli.run("ota branches", reply));

test/test_peripheral_mode_prefs.py

@@ -127,8 +127,8 @@ int main(){Common common;Companion companion;char reply[160];
  assert(companion._prefs.peripherals_shared==1&&companion.durable.peripherals_shared==1&&strstr(reply,"reboot"));
  assert(common.sensor.applications==0&&companion.sensors.applications==0);
  common.ok=companion.ok=false;
- common.set("set peripherals.mode gps-only",reply);assert(strstr(reply,"could not be saved"));
- assert(companion.command("set peripherals.mode gps-only",reply,sizeof(reply))&&strstr(reply,"could not be saved"));
+ common.set("set peripherals.mode gps-only",reply);assert(strstr(reply,"unconfirmed")&&strstr(reply,"reboot"));
+ assert(companion.command("set peripherals.mode gps-only",reply,sizeof(reply))&&strstr(reply,"unconfirmed")&&strstr(reply,"reboot"));
  assert(common.prefs.peripherals_shared==1&&companion._prefs.peripherals_shared==1);
  common.get("get peripherals.mode",reply);assert(strstr(reply,"shared"));
  assert(companion.command("get peripherals.mode",reply,sizeof(reply))&&strstr(reply,"shared"));
@@ -164,7 +164,7 @@ int main(){
   assert(strstr(reply,"Error")&&saves==before&&mode==durable);
  }
  ok=false;assert(mesh::peripherals::handleCommand(mode,"set peripherals.mode shared",reply,sizeof(reply),true,save));
- assert(mode==0&&durable==0&&strstr(reply,"could not be saved"));
+ assert(mode==0&&durable==0&&strstr(reply,"unconfirmed")&&strstr(reply,"reboot"));
  unsigned before=saves;
  assert(mesh::peripherals::handleCommand(mode,"set peripherals.mode shared",reply,sizeof(reply),false,save));
  assert(strstr(reply,"unsupported")&&saves==before&&mode==0);

test/test_repeater_trace_repeat.py

@@ -81,7 +81,7 @@ int main() {
       if (fault == 2) cli.fs.fail_read_open = true;
       if (fault == 3) cli.fs.fail_rename = 1;
       cli.set(desired ? "repeat.trace on" : "repeat.trace off", reply);
-      assert(!strcmp(reply, "Error: Repeat trace not saved; unchanged"));
+      assert(!strcmp(reply, "Error: save unconfirmed; reboot may differ"));
       assert(cli.prefs.trace_when_repeat_off == uint8_t(!desired));
       assert(cli.fs.files["/com_prefs"] == committed);
       assert(cli.callbacks.retry_updates == callbacks_before);