fix(saves): re-register the device when RomM no longer has its id

docs/user-guide/troubleshooting.md

@@ -85,12 +85,17 @@ the RomM database was reset and this device's registration disappeared with it 
 **RomM offline** badge stays clear and the surface tells you what's missing instead. If you see such a message while the
 badge is clear, the fix is on the RomM side (re-sync the library, or re-check the game), not with your network.
 
+When the missing thing is this device's own registration — the RomM database was wiped or restored and the id this
+device was issued no longer exists — you don't have to do anything. The next time the plugin loads (installing an update
+counts, since that reloads it) it notices the id is gone, registers this device afresh, and save sync carries on under
+the new id. A new entry shows up in RomM's device list; the old, dead one can be ignored.
+
 The first-time save-slot setup screen is the clearest example. If RomM can't find the save data that setup needs, it
 pauses with "RomM couldn't find the save data for this setup" rather than the "server is not reachable" message, and the
 offline badge stays clear. Setup deliberately stops there instead of picking a slot for you: the plugin has no
 trustworthy view of what's on the server, and choosing a slot on that basis could overwrite real saves on the first
-sync. Re-check the game in RomM (or, after a server database reset, reconnect in the plugin's Connection settings so
-this device is registered again), then tap **Retry**.
+sync. Re-check the game in RomM (a server database reset re-registers this device on its own the next time the plugin
+loads — see above), then tap **Retry**.
 
 ### Save file not found
 

py_modules/services/saves/sync_engine/devices.py

@@ -16,7 +16,7 @@
 import contextlib
 from typing import TYPE_CHECKING, Any
 
-from lib.errors import classify_error
+from lib.errors import RommNotFoundError, classify_error
 from lib.list_result import ErrorCode
 from services.saves._settings import save_sync_enabled
 
@@ -182,6 +182,14 @@ async def ensure_device_registered(
         fingerprint sent as the RomM ``hostname`` so the server dedupes
         this device across reinstalls. When the machine id is unreadable
         (``None``) the call degrades to no-fingerprint registration.
+
+        A cached device id is trusted only as far as the best-effort
+        ``update_device`` touch below: a definitive 404 from that touch
+        means the server no longer has this device (its database was wiped
+        or restored out from under the cached id), so the dead id is
+        forgotten and a fresh one is registered in its place. Every other
+        touch failure is a transient miss that leaves the cached id in
+        force — a server blip must never churn a re-registration.
         """
         if not save_sync_enabled(self._settings):
             return {
@@ -217,22 +225,38 @@ async def ensure_device_registered(
         device_id = await loop.run_in_executor(None, self.get_device_id)
         if device_id:
             server_id_str = str(device_id)
-            # Best-effort touch of the server-side client_version. A failure here
-            # is non-fatal (the device is already registered), but log it at debug
-            # so the swallow leaves a breadcrumb instead of vanishing silently.
+            # Best-effort touch of the server-side client_version, with one
+            # exception peeled off: a definitive ``RommNotFoundError`` (404)
+            # means the server no longer has this device (its DB was wiped or
+            # restored), so the cached id is dead — forget it and fall through
+            # to the registration path below to mint a fresh one (#1560). Every
+            # OTHER failure is non-fatal (the device is still registered): keep
+            # the cached id and return it, logging at debug so the swallow
+            # leaves a breadcrumb. A transient server blip must NEVER throw away
+            # a valid id and churn a spurious re-registration.
+            dead_device = False
             try:
                 await loop.run_in_executor(
                     None,
                     lambda: self._romm_api.update_device(server_id_str, client_version=self._plugin_version),
                 )
+            except RommNotFoundError as e:
+                self._log_debug(
+                    f"ensure_device_registered: server no longer has device {server_id_str} (404), re-registering: {e}"
+                )
+                await loop.run_in_executor(None, self.forget_device)
+                dead_device = True
             except Exception as e:
                 self._log_debug(f"ensure_device_registered: update_device failed (non-fatal): {e}")
-            return {
-                "success": True,
-                "device_id": device_id,
-                "device_name": self._get_device_name() or "",
-                "server_device_id": device_id,
-            }
+            if not dead_device:
+                return {
+                    "success": True,
+                    "device_id": device_id,
+                    "device_name": self._get_device_name() or "",
+                    "server_device_id": device_id,
+                }
+            # dead_device: the cached id was forgotten above; fall through to
+            # registration so a fresh id is minted and persisted.
 
         hostname = hostname_provider.get()
         machine_id = machine_id_provider.get()

tests/services/saves/sync_engine/test_devices.py

@@ -14,7 +14,15 @@
 from fakes.fake_save_api import FakeSaveApi
 from fakes.fake_unit_of_work import FakeUnitOfWorkFactory
 
-from lib.errors import RommApiError, RommAuthError, RommConnectionError, RommForbiddenError, RommSSLError
+from lib.errors import (
+    RommApiError,
+    RommAuthError,
+    RommConnectionError,
+    RommForbiddenError,
+    RommNotFoundError,
+    RommSSLError,
+    RommTimeoutError,
+)
 from lib.list_result import ErrorCode
 from services.saves.sync_engine.devices import DeviceRegistry
 from tests.services.saves._helpers import (
@@ -303,6 +311,151 @@ async def test_update_device_failure_logs_at_debug_and_still_succeeds(self, tmp_
         assert any("update_device failed" in m and "boom" in m for m in debug_log)
 
 
+class TestEnsureDeviceRegisteredReRegistersDeadDevice:
+    """#1560: after a RomM database wipe/restore the cached device id no longer
+    exists server-side, so the best-effort ``update_device`` touch gets a
+    definitive 404. That 404 (and ONLY that) forgets the dead id and
+    re-registers a fresh one; every other touch failure stays a best-effort
+    swallow that keeps the cached id, so a server blip never churns a
+    re-registration."""
+
+    @pytest.mark.asyncio
+    async def test_live_cached_id_touch_succeeds_no_reregistration(self, tmp_path):
+        svc, fake = make_service(tmp_path)
+        _enable_sync_with_device(svc, "server-uuid")
+        # Stamp a version so the pre-register heartbeat probe is skipped — the
+        # touch lands on update_device, which succeeds against a live id.
+        fake.set_version("4.9.0")
+
+        result = await svc.ensure_device_registered()
+
+        assert result["success"] is True
+        assert result["device_id"] == "server-uuid"
+        # The live touch fired and kept the id — no re-registration, id intact.
+        assert any(c[0] == "update_device" for c in fake.call_log)
+        assert _register_call(fake) is None
+        assert _get_device_id(svc) == "server-uuid"
+
+    @pytest.mark.asyncio
+    async def test_dead_cached_id_forgets_and_reregisters(self, tmp_path):
+        debug_log: list[str] = []
+        svc, fake = make_service(tmp_path, log_debug=debug_log.append)
+        _enable_sync_with_device(svc, "server-uuid")
+        fake.set_version("4.9.0")
+        # The server no longer has this device → the touch 404s (the #1560 wedge).
+        fake.fail_on_next(RommNotFoundError("Device with ID server-uuid not found"))
+
+        result = await svc.ensure_device_registered()
+
+        # A fresh id was minted and persisted, and it is the one returned — not
+        # the stale, dead one.
+        assert result["success"] is True
+        new_id = result["device_id"]
+        assert new_id
+        assert new_id != "server-uuid"
+        # register_device ran (the forget fell through to registration)...
+        assert _register_call(fake) is not None
+        # ...and the persisted kv_config id is now the fresh id, not the dead one.
+        assert _get_device_id(svc) == new_id
+        assert result["server_device_id"] == new_id
+        # The heal took the 404 branch (naming the dead id), not the generic swallow.
+        assert any("server no longer has device server-uuid" in m for m in debug_log)
+
+    @pytest.mark.asyncio
+    async def test_transport_failure_on_touch_is_not_a_heal(self, tmp_path):
+        svc, fake = make_service(tmp_path)
+        _enable_sync_with_device(svc, "server-uuid")
+        fake.set_version("4.9.0")
+        # A transport blip on the touch — NOT a definitive 404. The device is
+        # still registered; the id must be kept, not forgotten + re-registered.
+        fake.fail_on_next(RommConnectionError("Connection refused"))
+
+        result = await svc.ensure_device_registered()
+
+        assert result["success"] is True
+        assert result["device_id"] == "server-uuid"
+        # The cached id was KEPT — no forget, no re-registration.
+        assert _get_device_id(svc) == "server-uuid"
+        assert _register_call(fake) is None
+
+    @pytest.mark.asyncio
+    async def test_timeout_on_touch_is_not_a_heal(self, tmp_path):
+        svc, fake = make_service(tmp_path)
+        _enable_sync_with_device(svc, "server-uuid")
+        fake.set_version("4.9.0")
+        fake.fail_on_next(RommTimeoutError("timed out"))
+
+        result = await svc.ensure_device_registered()
+
+        assert result["success"] is True
+        assert result["device_id"] == "server-uuid"
+        assert _get_device_id(svc) == "server-uuid"
+        assert _register_call(fake) is None
+
+    @pytest.mark.asyncio
+    async def test_generic_exception_on_touch_is_not_a_heal(self, tmp_path):
+        """Edge: only a RommNotFoundError heals — a non-RomM error stays a swallow.
+
+        Guards the discriminator: broadening the peel to ``except Exception``
+        would re-register on any error, throwing away a valid id.
+        """
+        svc, fake = make_service(tmp_path)
+        _enable_sync_with_device(svc, "server-uuid")
+        fake.set_version("4.9.0")
+        fake.fail_on_next(RuntimeError("unexpected"))
+
+        result = await svc.ensure_device_registered()
+
+        assert result["success"] is True
+        assert result["device_id"] == "server-uuid"
+        assert _get_device_id(svc) == "server-uuid"
+        assert _register_call(fake) is None
+
+    @pytest.mark.asyncio
+    async def test_heal_then_reregister_fails_leaves_clean_state(self, tmp_path):
+        svc, fake = make_service(tmp_path)
+        _enable_sync_with_device(svc, "server-uuid")
+        fake.set_version("4.9.0")
+
+        # The touch 404s (dead id) → forget → but the server goes away mid-heal,
+        # so the re-registration itself fails. The touch is monkeypatched to 404
+        # (leaving the one-shot ``fail_on_next`` for the register_device that
+        # follows), since a single ``fail_on_next`` can only arm one call.
+        def _touch_404(*_args, **_kwargs):
+            raise RommNotFoundError("Device with ID server-uuid not found")
+
+        fake.update_device = _touch_404
+        fake.fail_on_next(RommConnectionError("Connection refused"))
+
+        result = await svc.ensure_device_registered()
+
+        # Clean, recoverable state: classified failure, empty id, dead id cleared.
+        assert result["success"] is False
+        assert result["reason"] == ErrorCode.SERVER_UNREACHABLE.value
+        assert result["message"]
+        assert result["device_id"] == ""
+        # The dead id was forgotten and NOT replaced — kv_config is left cleared,
+        # so the next ensure_device_registered retries cleanly from None.
+        assert _get_device_id(svc) is None
+
+    @pytest.mark.asyncio
+    async def test_no_cached_id_goes_straight_to_registration(self, tmp_path):
+        """First-time path is unchanged: no cached id → no touch, register runs."""
+        svc, fake = make_service(tmp_path)
+        svc._config.settings["save_sync_enabled"] = True
+        fake.set_version("4.9.0")
+        # No device_id persisted → registration branch.
+
+        result = await svc.ensure_device_registered()
+
+        assert result["success"] is True
+        assert result["device_id"]
+        # The update_device touch is only for an existing id — it must not run on
+        # the first-time path.
+        assert not any(c[0] == "update_device" for c in fake.call_log)
+        assert _register_call(fake) is not None
+
+
 class TestPermissionDegradedNeverDropsDeviceId:
     """#1437 regression guard: a permission-degraded (403) registration must NEVER
     delete ``kv_config["device_id"]``. The only in-process deleter is