refactor(sigenergy): drop the durable offboard latch, the switch already persists

Rounds 2 and 3 added a persisted offboard_done set, a restore path, a
_offboard_done_loaded tracking set and a reconcile function whose clearing
depended on switch state. Three of round 3's six findings were in that cluster,
which is the signature of machinery generating its own defects.

It was redundant twice over. The offboard switch is a control entity, so its
state is restored on startup like every other one — which is why the pre-existing
startup guard already read it, and why customers' Demo mode and mode selections
survive every image rollout. And onboard_status was already in
SIGENERGY_CACHE_KEYS before any of this, so the user-visible state persisted
through the existing cache regardless.

So the durable latch was defending against a restart that the switch already
covered, and preserving a status the existing cache already preserved. Removing
it also lets the two run() blocks go back to reading the switch directly instead
of reconciling it against a second source of truth.

What remains is the part that fixes the bug: exit VPP explicitly rather than
assuming offboard does it, require telemetry confirmation of an owner-controlled
mode before offboarding, latch each step only on success, and treat per-item
offboard failures as failures. The in-memory _offboard_done still prevents a
repeat call within a process and is cleared when the switch goes off.

184 insertions down to 110. Suite: 84 pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: mgazza

apps/predbat/sigenergy.py

@@ -221,7 +221,7 @@
 # Storage cache keys for poll-interval system state persisted between restarts (see
 # load_cached_data()). System/device discovery is deliberately excluded — it is always
 # re-fetched fresh on startup.
-SIGENERGY_CACHE_KEYS = ["energy_flow", "daily_summary", "history_totals", "onboard_status", "offboard_done"]
+SIGENERGY_CACHE_KEYS = ["energy_flow", "daily_summary", "history_totals", "onboard_status"]
 
 # Sentinel returned by _request() when the API responds with code=0 but an empty/null data field.
 # Distinguishes "success with no payload" from None which always means "request failed".
@@ -319,8 +319,7 @@ def initialize(self, app_key, app_secret, base_url=None, mqtt_host=None, ca_cert
         self.last_contended_by = {}  # systemId → mode name of the controller that last displaced Predbat
         self._axle_standoff_logged = {}  # systemId → True while the Axle stand-down has been announced
         self._offboard_vpp_exit_done = set()  # systemIds confirmed out of VPP ahead of an offboard
-        self._offboard_done = set()           # systemIds successfully offboarded
-        self._offboard_done_loaded = set()    # subset of the above restored from cache, not yet reconciled
+        self._offboard_done = set()           # systemIds successfully offboarded this process
         self.onboard_status = {}  # systemId → onboarding status string (published for the SaaS UI)
 
         # Age (datetime of last update) of each SIGENERGY_CACHE_KEYS category, used to avoid an
@@ -2015,9 +2014,7 @@ async def _update_control(self, entity_id, value, direction, field, system_id):
                 # Re-onboarding — let a future offboard run both steps again.
                 self._offboard_vpp_exit_done.discard(system_id)
                 self._offboard_done.discard(system_id)
-                self._offboard_done_loaded.discard(system_id)
                 self.onboard_status[str(system_id)] = "not_onboarded"
-                await self._save_offboard_done()
                 await self._save_cache("onboard_status", self.onboard_status)
                 self._publish_onboard_status()
 
@@ -2273,9 +2270,7 @@ async def _offboard_system_if_needed(self, system_id):
             self.log("Warn: SigenergyAPI: Offboard failed for {} — will retry on the next poll".format(system_id))
             return False
         self._offboard_done.add(system_id)
-        self._offboard_done_loaded.discard(system_id)
         self.onboard_status[str(system_id)] = "offboarded"
-        await self._save_offboard_done()
         await self._save_cache("onboard_status", self.onboard_status)
         self._publish_onboard_status()
         return True
@@ -2389,45 +2384,6 @@ def _publish_onboard_status(self):
     # Storage cache
     # -----------------------------------------------------------------------
 
-    async def _save_offboard_done(self):
-        """Persist completed offboards without the one-day telemetry-cache expiry.
-
-        Offboarding remains true until the user explicitly re-onboards, so expiring this
-        latch like energy telemetry would make a later process restart forget the external
-        action and publish an untrue status.
-        """
-        if self.storage:
-            # A restored completion may be provisionally cleared in memory while a still
-            # visible system is offboarded again. Keep that recovery point durable until
-            # the retry succeeds or the user explicitly requests re-onboarding.
-            await self.storage.save("sigenergy", "offboard_done", sorted(self._offboard_done | self._offboard_done_loaded), format="json")
-
-    async def _reconcile_restored_offboards(self):
-        """Reconcile restored completions for systems now visible as authorised.
-
-        This runs after any startup onboarding attempt, because a system that was absent
-        on the first discovery can become visible in the second fetch. Visibility means a
-        future offboard must run both steps again, but does not by itself prove that the
-        owner requested re-onboarding: the authorised list may still be converging after
-        a successful offboard. Only an explicit off switch clears the durable recovery
-        point; otherwise it is retained until the retry succeeds.
-        """
-        restored_visible = set(self.systems.keys()) & self._offboard_done_loaded
-        if not restored_visible:
-            return
-        save_offboard_done = False
-        for sid in restored_visible:
-            slug = self._system_slug(sid)
-            offboard_state = self.get_state_wrapper("switch.{}_sigenergy_{}_offboard".format(self.prefix, slug), default=None)
-            self._offboard_done.discard(sid)
-            self.onboard_status[str(sid)] = "active"
-            if offboard_state == "off":
-                self._offboard_done_loaded.discard(sid)
-                save_offboard_done = True
-        if save_offboard_done:
-            await self._save_offboard_done()
-        await self._save_cache("onboard_status", self.onboard_status)
-
     def _data_age_minutes(self, key):
         """Return the age in minutes of the in-memory data for a cache key, or None if unknown."""
         timestamp = self.data_age.get(key, None)
@@ -2507,15 +2463,6 @@ async def load_cached_data(self):
         if onboard_status is not None:
             self.onboard_status = onboard_status
 
-        offboard_done = await self._load_cache("offboard_done")
-        if offboard_done is not None:
-            self._offboard_done = set(offboard_done)
-            # Keep track of restored entries separately. A freshly discovered authorised
-            # system must be offboarded again, but the durable completion remains a safe
-            # recovery point until that retry lands or re-onboarding is explicit.
-            self._offboard_done_loaded = set(offboard_done)
-            for sid in self._offboard_done:
-                self.onboard_status[str(sid)] = "offboarded"
 
         self.log("SigenergyAPI: Restored cached poll-interval state from storage")
 
@@ -2556,18 +2503,13 @@ async def run(self, seconds, first):
             for sid in missing_ids:
                 self.onboard_status.setdefault(str(sid), "not_onboarded")
                 slug = self._system_slug(sid)
-                offboard_state = self.get_state_wrapper("switch.{}_sigenergy_{}_offboard".format(self.prefix, slug), default=None)
-                is_offboard_at_start = offboard_state == "on" or (sid in self._offboard_done and offboard_state != "off")
+                # The switch is the source of truth: it is a control entity, so its state
+                # is restored on startup like every other one. Onboarding a system the
+                # owner deliberately left would cost them a fresh approval email.
+                is_offboard_at_start = self.get_state_wrapper("switch.{}_sigenergy_{}_offboard".format(self.prefix, slug), default="off") == "on"
                 if is_offboard_at_start:
-                    self.log("SigenergyAPI: System {} is marked offboarded — skipping onboard attempt".format(sid))
+                    self.log("SigenergyAPI: System {} offboard toggle is on — skipping onboard attempt".format(sid))
                     continue
-                if sid in self._offboard_done:
-                    # An explicit off state is a re-onboarding request made while this
-                    # component was stopped, so clear the durable completion first.
-                    self._offboard_done.discard(sid)
-                    self._offboard_done_loaded.discard(sid)
-                    self.onboard_status[str(sid)] = "not_onboarded"
-                    await self._save_offboard_done()
                 self.log("SigenergyAPI: System {} not found in authorised list — attempting onboard".format(sid))
                 result = await self.onboard_systems([sid])
                 if result is not True:
@@ -2576,10 +2518,6 @@ async def run(self, seconds, first):
                     return False
                 await self.fetch_system_list()
 
-            # The completion latch survives restarts. Reconcile after onboarding because
-            # its second fetch can make a previously absent system visible during this same
-            # startup, requiring either an offboard retry or an explicit re-onboarding clear.
-            await self._reconcile_restored_offboards()
 
             if not self.systems:
                 # An intentionally offboarded system is absent from the authorised list.
@@ -2628,15 +2566,7 @@ async def run(self, seconds, first):
                     self.log("SigenergyAPI: Skipping VPP registration check for {} — operating mode not yet known".format(sid))
                     continue
                 slug = self._system_slug(sid)
-                offboard_state = self.get_state_wrapper("switch.{}_sigenergy_{}_offboard".format(self.prefix, slug), default=None)
-                if offboard_state == "off" and (sid in self._offboard_done or sid in self._offboard_done_loaded):
-                    # The state may have changed while this component was stopped and no
-                    # switch event was delivered. Treat an explicit off as re-onboarding.
-                    self._offboard_vpp_exit_done.discard(sid)
-                    self._offboard_done.discard(sid)
-                    self._offboard_done_loaded.discard(sid)
-                    await self._save_offboard_done()
-                is_offboard = offboard_state == "on" or (offboard_state is None and (sid in self._offboard_done or sid in self._offboard_done_loaded))
+                is_offboard = self.get_state_wrapper("switch.{}_sigenergy_{}_offboard".format(self.prefix, slug), default="off") == "on"
                 await self._manage_vpp_registration(sid, is_readonly_vpp, is_offboard)
                 # Derive the user-facing onboarding status for the visible system.
                 # A system sitting in a third-party mode is fully onboarded — another

apps/predbat/tests/test_sigenergy.py

@@ -2450,85 +2450,31 @@ def test_sigenergy_offboard_unknown_mode_defers(my_predbat):
     return failed
 
 
-def test_sigenergy_offboard_latches_survive_restart_safely(my_predbat):
-    """Restart restores only completed offboards; an in-flight exit is re-confirmed live."""
-    failed = False
-    sid = "SIG001"
-    storage = FakeStorage()
-
-    # A restart after the mode command landed but before offboarding has no in-memory
-    # exit latch. The REST/MQTT-repopulated MSC mode is enough to resume safely.
-    api_inflight = _make_api_with_system(sid)
-    api_inflight._mock_storage = storage
-    api_inflight.current_mode[sid] = SIGENERGY_MODE_MSC
-    api_inflight.offboard_systems = AsyncMock(return_value=[])
-    assert run_async(api_inflight._offboard_system_if_needed(sid)) is True
-
-    # Completion is persisted, so a restart after authorisation disappears does not
-    # lose the truthful status or retry an endpoint it may no longer be allowed to call.
-    api_restarted = MockSigenergyAPI()
-    api_restarted._mock_storage = storage
-    run_async(api_restarted.load_cached_data())
-    assert sid in api_restarted._offboard_done, "Completed offboard latch restored"
-    assert sid not in api_restarted._offboard_vpp_exit_done, "VPP-exit latch remains live-state only"
-    assert api_restarted.onboard_status[sid] == "offboarded", "Restart restores truthful status"
-
-    api_restarted.system_id_filter = {sid}
-    api_restarted.get_access_token = AsyncMock(return_value="tok")
-    api_restarted.fetch_system_list = AsyncMock()  # Correctly absent after offboarding.
-    api_restarted.onboard_systems = AsyncMock()
-    assert run_async(api_restarted.run(seconds=0, first=True)) is False
-    api_restarted.onboard_systems.assert_not_awaited()
-    sensor_key = "sensor.predbat_sigenergy_sig001_onboard_status"
-    assert api_restarted.dashboard_items[sensor_key]["state"] == "offboarded", "Restart publishes completion even when the switch state is temporarily unavailable"
+def test_sigenergy_offboard_switch_survives_restart(my_predbat):
+    """A restart must not re-onboard a system the owner deliberately left.
 
-    # If the owner explicitly requests re-onboarding after restart, live visibility
-    # invalidates the restored completion and persists that clearing for the next process.
-    api_restarted.systems[sid] = {}
-    api_restarted.dashboard_items["switch.predbat_sigenergy_sig001_offboard"] = {"state": "off"}
-    run_async(api_restarted._reconcile_restored_offboards())
-    assert sid not in api_restarted._offboard_done, "Visible authorised system clears restored completion"
-    assert api_restarted.onboard_status[sid] == "active", "Re-onboarded system no longer reports offboarded"
-    api_after_re_onboard = MockSigenergyAPI()
-    api_after_re_onboard._mock_storage = storage
-    run_async(api_after_re_onboard.load_cached_data())
-    assert sid not in api_after_re_onboard._offboard_done, "Cleared completion remains cleared after another restart"
-
-    return failed
-
-
-def test_sigenergy_visible_restored_offboard_keeps_recovery_point(my_predbat):
-    """A lagging authorised list must not erase a completed offboard before its retry lands."""
+    The offboard switch is a control entity, so its state is restored on startup like
+    every other one — no separate durable latch is needed, and re-onboarding would cost
+    the owner a fresh approval email.
+    """
     failed = False
     sid = "SIG001"
-    storage = FakeStorage()
-    storage.data[("sigenergy", "offboard_done")] = [sid]
-
     api = MockSigenergyAPI()
-    api._mock_storage = storage
-    run_async(api.load_cached_data())
-    api.systems[sid] = {}
-    run_async(api._reconcile_restored_offboards())
+    api.system_id_filter = {sid}
+    api.systems = {}
+    slug = api._system_slug(sid)
+    api.dashboard_items["switch.predbat_sigenergy_{}_offboard".format(slug)] = {"state": "on"}
 
-    assert sid not in api._offboard_done, "Visible system is retried rather than accepted as complete"
-    assert sid in api._offboard_done_loaded, "Durable recovery point remains until the retry lands"
-    assert storage.data[("sigenergy", "offboard_done")] == [sid], "Lagging visibility must not clear durable completion"
-    assert api.onboard_status[sid] == "active", "Visible authorised system is reported active while retrying"
+    onboarded = []
 
-    api.current_mode[sid] = SIGENERGY_MODE_MSC
-    api.offboard_systems = AsyncMock(return_value=None)
-    api.enable_controls = False
-    mqtt_task = MagicMock()
-    mqtt_task.done = MagicMock(return_value=False)
-    api._mqtt_task = mqtt_task
-    assert run_async(api.run(seconds=60, first=False)) is True
-    api.offboard_systems.assert_awaited_once_with(sid)
-    assert api.onboard_status[sid] == "active", "Unavailable switch state preserves the offboard retry instead of reclaiming VPP"
+    async def mock_onboard(system_ids):
+        onboarded.append(system_ids)
+        return True
+
+    api.onboard_systems = mock_onboard
 
-    api_after_failure = MockSigenergyAPI()
-    api_after_failure._mock_storage = storage
-    run_async(api_after_failure.load_cached_data())
-    assert sid in api_after_failure._offboard_done, "A restart after retry failure restores the last confirmed completion"
+    run_async(api.run(seconds=0, first=True))
+    assert not onboarded, "A restart must not re-onboard a system whose offboard switch is on"
 
     return failed
 
@@ -3034,8 +2980,7 @@ def run_sigenergy_tests(my_predbat):
         ("offboard_retries_when_api_call_fails", test_sigenergy_offboard_retries_when_the_api_call_fails),
         ("offboard_retries_per_item_failure", test_sigenergy_offboard_retries_per_item_failure),
         ("offboard_unknown_mode_defers", test_sigenergy_offboard_unknown_mode_defers),
-        ("offboard_latches_survive_restart_safely", test_sigenergy_offboard_latches_survive_restart_safely),
-        ("visible_restored_offboard_keeps_recovery_point", test_sigenergy_visible_restored_offboard_keeps_recovery_point),
+        ("offboard_switch_survives_restart", test_sigenergy_offboard_switch_survives_restart),
         ("publish_onboard_status_sensors", test_sigenergy_publish_onboard_status_sensors),
         ("run_derives_onboard_status", test_sigenergy_run_derives_onboard_status),
         ("run_pending_publishes_before_early_exit", test_sigenergy_run_pending_publishes_before_early_exit),