fix(acl): make the hot-reload restore unconditional; release a deleted room's slot

Fifth review round (opencode: "the ACL design is clean"; residual nits):
- The old identity entry is put back whenever the new registration did not
  take its place, including when registration raises.
- A room whose update leaves it with no password is not hot-reloaded: it
  is not registered for logins, so its ACL would stay under the old name.
  The update waits for a restart and says so.
- delete_identity releases the room's name and hash through
  unregister_identity, so the room can be re-created live rather than
  conflicting with a room that no longer exists until a restart.
- LoginHelper.move_room_acl checks for a store before any path, the
  superseded-ACL loop again skips the ACL being registered, and the
  test daemon accepts the previous_name argument of the RoomServer
  teardown fix on feat/quirky-lamarr-dfeae2, which should land with this
  branch: the hot reload that now applies is what reaches that bug.

repeater/handler_helpers/login.py

@@ -159,7 +159,7 @@ class LoginHelper:
                 self.acls_by_name.get(name),
                 self._live_acl_for_store_key(identity_acl.store_key or ""),
             }:
-                if superseded is not None:
+                if superseded is not None and superseded is not identity_acl:
                     superseded.detach_store()
                     self._drop_acl(superseded)
             identity_acl.load()
@@ -426,6 +426,10 @@ class LoginHelper:
         from repeater.handler_helpers.acl import acl_identity_label, move_identity_acl
 
         label = acl_identity_label(new_name, "room_server")
+        if self.sqlite_handler is None and old_pubkey_hex.lower() != new_pubkey_hex.lower():
+            # A key change with nowhere to move the entries would save a key
+            # whose access list is gone.
+            raise RuntimeError("no ACL store is available to move the room's access list")
         live = self.acls_by_name.get(old_name)
         if live is None or live.store_key != old_pubkey_hex.lower():
             # The name index can lag the config (a hot reload refused), but
@@ -434,10 +438,6 @@ class LoginHelper:
         if live is not None:
             live.move_store(new_pubkey_hex, label, commit)
             return
-        if self.sqlite_handler is None and old_pubkey_hex.lower() != new_pubkey_hex.lower():
-            # A key change with nowhere to move the entries would save a key
-            # whose access list is gone.
-            raise RuntimeError("no ACL store is available to move the room's access list")
         move_identity_acl(self.sqlite_handler, old_pubkey_hex, new_pubkey_hex, label, commit)
 
     def forget_identity_acl(self, name: str, pubkey_hex: str) -> int:

repeater/web/api_endpoints.py

@@ -6870,8 +6870,15 @@ class APIEndpoints:
             registration_success = False
             # Only reload if identity_key was actually provided and not empty, or if name changed
             needs_reload = data.get("identity_key") or "new_name" in data
+            # A room with no password is not registered for logins, so a live
+            # reload would leave its login ACL under the old name; it waits
+            # for a restart instead.
+            room_settings = identity.get("settings") or {}
+            can_log_in = bool(
+                room_settings.get("admin_password") or room_settings.get("guest_password")
+            )
 
-            if needs_reload and self.daemon_instance:
+            if needs_reload and can_log_in and self.daemon_instance:
                 try:
                     from openhop_core import LocalIdentity
 
@@ -6913,16 +6920,20 @@ class APIEndpoints:
                             and hasattr(identity_manager, "unregister_identity")
                             else None
                         )
-                        registration_success = self.daemon_instance._register_identity_everywhere(
-                            name=final_name,
-                            identity=room_identity,
-                            config=identity,
-                            identity_type="room_server",
-                        )
-                        if not registration_success and previous is not None:
-                            identity_manager.register_identity(
-                                old_name, previous[0], previous[1], previous[2]
+                        try:
+                            registration_success = (
+                                self.daemon_instance._register_identity_everywhere(
+                                    name=final_name,
+                                    identity=room_identity,
+                                    config=identity,
+                                    identity_type="room_server",
+                                )
                             )
+                        finally:
+                            if not registration_success and previous is not None:
+                                identity_manager.register_identity(
+                                    old_name, previous[0], previous[1], previous[2]
+                                )
                         if registration_success:
                             logger.info(
                                 f"Hot reload: Re-registered identity '{final_name}' with all systems"
@@ -7066,17 +7077,12 @@ class APIEndpoints:
                     if hasattr(self.daemon_instance, "identity_manager"):
                         identity_manager = self.daemon_instance.identity_manager
 
-                        # Remove from named_identities dict
-                        if name_s in identity_manager.named_identities:
-                            del identity_manager.named_identities[name_s]
-                            logger.info(f"Removed identity {name_s} from named_identities")
+                        # Release its name and hash slot, so a room re-created with
+                        # this name or key registers live rather than after a restart.
+                        if identity_manager.unregister_identity(name_s) is not None:
+                            logger.info(f"Unregistered identity {name_s}")
                             unregister_success = True
 
-                        # Note: We don't remove from identities dict (keyed by hash)
-                        # because we'd need to look up the hash first, and there could
-                        # be multiple identities with the same hash
-                        # Full cleanup happens on restart
-
                 except Exception as unreg_error:
                     logger.error(
                         f"Failed to unregister identity {name_s}: {unreg_error}", exc_info=True
@@ -7336,6 +7342,8 @@ class APIEndpoints:
         login_helper = getattr(daemon, "login_helper", None)
         if login_helper is None:
             return []
+        # LoginHelper.get_acl_by_name is required: the hash lookup it replaced
+        # attributes one identity's ACL to another that shares its hash byte.
 
         owners = []
         if getattr(daemon, "local_identity", None):

tests/test_acl_api.py

@@ -439,7 +439,9 @@ class _LiveDaemon:
         )
         self.config = {"identities": {"room_servers": [cfg]}}
 
-    def _register_identity_everywhere(self, name, identity, config, identity_type):
+    def _register_identity_everywhere(
+        self, name, identity, config, identity_type, previous_name=None
+    ):
         if not self.identity_manager.register_identity(
             name=name, identity=identity, config=config, identity_type=identity_type
         ):
@@ -510,3 +512,45 @@ def test_a_refused_hot_reload_keeps_the_room_registered(db, request_ctx):
     assert [n for n, *_ in daemon.identity_manager.get_identities_by_type("room_server")] == [
         "room-a"
     ]
+
+
+def test_a_hot_reload_that_raises_keeps_the_room_registered(db, request_ctx):
+    daemon = _LiveDaemon(db, "room-a", "11" * 32)
+    api = _api(daemon, daemon.config)
+    daemon._register_identity_everywhere = MagicMock(side_effect=RuntimeError("boom"))
+
+    request_ctx.method = "PUT"
+    request_ctx.json = {"name": "room-a", "new_name": "room-b"}
+    assert "Restart required" in api.update_identity()["message"]
+    assert [n for n, *_ in daemon.identity_manager.get_identities_by_type("room_server")] == [
+        "room-a"
+    ]
+
+
+def test_a_rename_that_clears_the_passwords_waits_for_a_restart(db, request_ctx):
+    daemon = _LiveDaemon(db, "room-a", "11" * 32)
+    api = _api(daemon, daemon.config)
+
+    request_ctx.method = "PUT"
+    request_ctx.json = {
+        "name": "room-a",
+        "new_name": "room-b",
+        "settings": {"admin_password": "", "guest_password": ""},
+    }
+    assert "Restart required" in api.update_identity()["message"]
+    # Nothing half-applied: the running room keeps its registration and ACL.
+    assert [n for n, *_ in daemon.identity_manager.get_identities_by_type("room_server")] == [
+        "room-a"
+    ]
+    assert daemon.login_helper.get_acl_by_name("room-a") is not None
+
+
+def test_delete_identity_frees_the_name_and_hash(db, request_ctx):
+    daemon = _LiveDaemon(db, "room-a", "11" * 32)
+    api = _api(daemon, daemon.config)
+
+    request_ctx.method = "DELETE"
+    assert api.delete_identity(name="room-a", type="room_server")["success"] is True
+    identity = LocalIdentity(seed=bytes.fromhex("11" * 32))
+    # Re-creating the room registers at once instead of conflicting until a restart.
+    assert daemon.identity_manager.registration_error("room-a", identity, "room_server") is None