fix: settle staged options on every exit, and stop double-counting a repaired user

custom_components/lock_code_manager/__init__.py

@@ -128,7 +128,7 @@
     MIN_PIN_LENGTH,
     generate_pin,
 )
-from .domain.queries import get_entry_config
+from .domain.queries import get_entry_config, subentry_id_for_slot
 from .domain.references import async_notify_moved
 from .domain.services import (
     async_add_users,
@@ -1359,8 +1359,14 @@ def _is_ours(device: dr.DeviceEntry) -> bool:
         slot_num = parse_slot_unique_id(entry_id, entity.unique_id)
         if slot_num is None:
             continue
+        # Created under the user who holds the slot, like every other
+        # device this integration makes. Created bare, the first
+        # `async_add_entities` would pull it into that subentry implicitly --
+        # which Home Assistant deprecates, and on the very upgrade path this
+        # function exists to serve.
         slot_device = dev_reg.async_get_or_create(
             config_entry_id=entry_id,
+            config_subentry_id=subentry_id_for_slot(config_entry, slot_num),
             **build_slot_device_info(config_entry, slot_num),
         )
         _LOGGER.debug(
@@ -1828,6 +1834,36 @@ async def async_update_listener(
         config_entry.runtime_data.settled.set()
 
 
+@callback
+def _async_settle_options(
+    hass: HomeAssistant,
+    config_entry: LockCodeManagerConfigEntry,
+    config: EntryConfig,
+) -> None:
+    """
+    Fold a staged options submission into the entry's own data.
+
+    `data` is where the configuration lives; `options` is a staging area,
+    because an options flow cannot write `data` itself. Settling is what keeps
+    that true, so it happens on every path out of the update listener.
+
+    Only when something is actually staged. Settling unconditionally would
+    corrupt the direct writes: `async_write_entry_config` reconciles the
+    subentries and the entry in separate calls, each of which wakes the
+    listener, and a pass that woke between them would write back the half of
+    the configuration it had read -- undoing the half that had already landed.
+    Those callers clear `options` themselves, so this is a no-op for them.
+
+    ``to_dict()`` is what makes the stored data plain dicts rather than the
+    read-only ``MappingProxyType`` wrappers ``EntryConfig`` uses internally,
+    which Home Assistant's storage layer cannot serialize.
+    """
+    if config_entry.options:
+        hass.config_entries.async_update_entry(
+            config_entry, data=config.to_dict(), options={}
+        )
+
+
 async def _async_apply_entry_update(
     hass: HomeAssistant,
     config_entry: LockCodeManagerConfigEntry,
@@ -1867,6 +1903,11 @@ async def _async_apply_entry_update(
     # produces no diff when it re-enters.
     diff = EntryConfigDiff(old=old_config, new=new_config)
     if not diff.has_changes:
+        # Settled here too. An options submission that changes nothing reaches
+        # this return, and leaving it staged would strand it in `options` for
+        # good -- read in preference to `data` for as long as the entry
+        # exists, then cleared by the next write that touches the entry.
+        _async_settle_options(hass, config_entry, new_config)
         return
 
     ent_reg = er.async_get(hass)
@@ -1940,11 +1981,16 @@ async def _async_apply_entry_update(
     # anchored the slot; slot-only providers leave the default no-op in
     # place. This runs before ``locks_to_remove`` processing so providers
     # in ``runtime_data.locks`` are still usable.
-    # Drained, not read: a hand-off applies to the write that requested it,
-    # and leaving the pair behind would spare the next occupant of that slot
-    # number the cleanup it does need.
+    # Consumed pair by pair, not drained wholesale. A hand-off applies to the
+    # write that requested it and must not outlive it -- leaving a pair behind
+    # would spare the next occupant of that number the cleanup it does need --
+    # but one `delete_user` call is several entry writes, one per departing
+    # user, and each wakes this listener. Taking the whole set on the first
+    # pass left every departure after the first one unprotected, and their
+    # credential was wiped off every lock despite the caller asking for it to
+    # be left programmed.
     retained_pairs = runtime_data.retained_pairs
-    runtime_data.retained_pairs = set()
+    runtime_data.retained_pairs = retained_pairs - diff.pairs_removed
     for lock_entity_id, slot_num in diff.pairs_removed:
         release_lock = runtime_data.locks.get(lock_entity_id)
         if release_lock is None:
@@ -2021,31 +2067,7 @@ async def _async_apply_entry_update(
     _LOGGER.info(
         "%s (%s): Done creating and/or updating entities", entry_id, entry_title
     )
-    # Only when there is something staged to settle. The options flow leaves
-    # its submission in `options` for this listener to fold into `data`;
-    # everything else writes `data` directly and clears `options` itself.
-    #
-    # An options submission that changes nothing does not reach here at all --
-    # the diff above is empty and returns first -- so `options` is left
-    # standing. Harmless while it holds what the options flow writes, which is
-    # `data`'s own contents; a field that lived in `options` WITHOUT also being
-    # in `data` would go stale there, so do not add one.
-    #
-    # Settling unconditionally corrupts those direct writes.
-    # `async_write_entry_config` reconciles the subentries and the entry in
-    # separate calls, each of which wakes this listener -- and a pass that
-    # woke between them would write back the half of the configuration it
-    # read, undoing the half that had already landed.
-    #
-    # to_dict() is what makes the stored data plain dicts rather than the
-    # read-only MappingProxyType wrappers EntryConfig uses internally, which
-    # Home Assistant's storage layer cannot serialize.
-    if config_entry.options:
-        hass.config_entries.async_update_entry(
-            config_entry, data=new_config.to_dict(), options={}
-        )
-        # The async_update_entry above re-triggers this listener, which
-        # refreshes runtime_data.config at the top before the early-return.
+    _async_settle_options(hass, config_entry, new_config)
 
     # Notify Lovelace dashboards to re-render when structure changes
     # (slots or locks added/removed), so strategy-generated cards update

custom_components/lock_code_manager/config_flow.py

@@ -795,11 +795,17 @@ async def async_step_reconfigure(
         description_placeholders: dict[str, Any] = {}
         held = normalize_name(subentry.title)
 
+        # Everybody except the person being edited. Both the name check and,
+        # below, the count allocation is asked for are about the OTHERS: the
+        # edited user is already in `config.users`, so counting them again
+        # refuses on a lock with exactly enough room, and reconciling against
+        # a dict that still holds their old name issues their new one a
+        # second number.
+        others = {other: user for other, user in config.users.items() if other != held}
+
         if user_input is not None:
             name, fields, errors, placeholders = _validate_user_form(
-                self.hass,
-                user_input,
-                [other for other in config.users if other != held],
+                self.hass, user_input, others
             )
             description_placeholders.update(placeholders)
 
@@ -822,7 +828,7 @@ async def async_step_reconfigure(
                         allocation_errors,
                         allocation_placeholders,
                     ) = await _allocate_for(
-                        self.hass, entry, config.locks, len(config.users) + 1
+                        self.hass, entry, config.locks, len(others) + 1
                     )
                     if unavailable is None:
                         errors.update(allocation_errors)
@@ -831,7 +837,7 @@ async def async_step_reconfigure(
                             subentry, user_input, errors, description_placeholders
                         )
                     held_slot = config.assignment.reconcile(
-                        {**config.users, name: fields},
+                        {**others, name: fields},
                         start=1,
                         unavailable=unavailable,
                     ).slot(name)

custom_components/lock_code_manager/providers/zha.py

@@ -54,14 +54,6 @@
     DoorLock.OperationEvent.ScheduleUnlock: False,
 }
 
-OPERATION_SOURCE_NAMES: dict[int, str] = {
-    DoorLock.OperationEventSource.Keypad: "Keypad",
-    DoorLock.OperationEventSource.RF: "RF",
-    DoorLock.OperationEventSource.Manual: "Manual",
-    DoorLock.OperationEventSource.RFID: "RFID",
-    DoorLock.OperationEventSource.Indeterminate: "Unknown",
-}
-
 
 @dataclass(repr=False, eq=False)
 class ZHALock(BaseLock):

tests/test_config_flow.py

@@ -806,6 +806,85 @@ async def test_editing_a_user_who_has_no_number_issues_one(
     assert entry.subentries[subentry.subentry_id].data[CONF_SLOT] == 1
 
 
+async def test_repairing_an_unnumbered_user_fits_on_a_lock_with_exactly_enough_room(
+    hass: HomeAssistant, mock_lock_config_entry
+):
+    """
+    The repair asks for room for the OTHERS plus this one, not for one extra.
+
+    The user being edited is already in the entry's users, so counting them
+    again refuses on a lock with exactly enough room -- naming a user count
+    that does not exist -- and the dialog that exists to repair the record
+    becomes one that can only fail.
+    """
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="test",
+        data={CONF_LOCKS: [LOCK_1_ENTITY_ID]},
+        subentries_data=[
+            *user_subentries({1: {CONF_NAME: "Alice", CONF_PIN: "1111"}}),
+            unnumbered_user_subentry("Nomad", **{CONF_ENABLED: False}),
+        ],
+        unique_id="unnumbered-exact-fit",
+    )
+    entry.add_to_hass(hass)
+    subentry = next(s for s in entry.subentries.values() if s.title == "Nomad")
+
+    result = await hass.config_entries.subentries.async_init(
+        (entry.entry_id, SUBENTRY_TYPE_USER),
+        context={"source": "reconfigure", "subentry_id": subentry.subentry_id},
+    )
+
+    # Two slots on the lock, Alice holds one, and two users all told.
+    with (
+        patch.object(MockLCMLock, "async_get_max_slot", AsyncMock(return_value=2)),
+        _holding(),
+    ):
+        result = await hass.config_entries.subentries.async_configure(
+            result["flow_id"],
+            {CONF_NAME: "Nomad", CONF_ENABLED: True, CONF_PIN: "4242"},
+        )
+
+    assert result["type"] == "abort"
+    assert entry.subentries[subentry.subentry_id].data[CONF_SLOT] == 2
+
+
+async def test_repairing_an_unnumbered_user_while_renaming_them_wastes_no_slot(
+    hass: HomeAssistant, mock_lock_config_entry
+):
+    """
+    A repair that also renames issues one number, from the bottom.
+
+    Reconciling against a dict that still holds the old name puts the same
+    person in twice under two identities, so the new one is numbered around
+    the old one and the lowest free position is burned for good.
+    """
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="test",
+        data={CONF_LOCKS: [LOCK_1_ENTITY_ID]},
+        subentries_data=[unnumbered_user_subentry("Nomad", **{CONF_ENABLED: False})],
+        unique_id="unnumbered-renamed",
+    )
+    entry.add_to_hass(hass)
+    subentry = next(iter(entry.subentries.values()))
+
+    result = await hass.config_entries.subentries.async_init(
+        (entry.entry_id, SUBENTRY_TYPE_USER),
+        context={"source": "reconfigure", "subentry_id": subentry.subentry_id},
+    )
+
+    with _holding():
+        result = await hass.config_entries.subentries.async_configure(
+            result["flow_id"],
+            {CONF_NAME: "Zed", CONF_ENABLED: True, CONF_PIN: "4242"},
+        )
+
+    assert result["type"] == "abort"
+    assert entry.subentries[subentry.subentry_id].title == "Zed"
+    assert entry.subentries[subentry.subentry_id].data[CONF_SLOT] == 1
+
+
 async def test_editing_an_unnumbered_user_refuses_when_the_lock_cannot_be_read(
     hass: HomeAssistant, mock_lock_config_entry
 ):

tests/test_init.py

@@ -2113,6 +2113,33 @@ async def _fail_lock_2(self: MockLCMLock, config_entry) -> None:
     await hass.config_entries.async_unload(entry.entry_id)
 
 
+async def test_an_options_submission_that_changes_nothing_still_settles(
+    hass: HomeAssistant, mock_lock_config_entry, lock_code_manager_config_entry
+) -> None:
+    """
+    `options` is a staging area, and it is emptied even when nothing changed.
+
+    The update listener returns early when the submission produces no diff,
+    and that was the one path out of it that skipped settling. What was left
+    behind read fine -- `options` holds what `data` holds -- right up until
+    the next write cleared it, which would take anything that lived only
+    there with it.
+    """
+    entry = lock_code_manager_config_entry
+    locks = list(get_entry_config(entry).locks)
+    assert entry.options == {}
+
+    result = await hass.config_entries.options.async_init(entry.entry_id)
+    result = await hass.config_entries.options.async_configure(
+        result["flow_id"], user_input={CONF_LOCKS: locks}
+    )
+    await hass.async_block_till_done()
+
+    assert result["type"] == "create_entry"
+    assert entry.options == {}
+    assert entry.data[CONF_LOCKS] == locks
+
+
 async def test_migration_v5_moves_an_existing_slot_device_to_its_user(
     hass: HomeAssistant, mock_lock_config_entry
 ) -> None:

tests/test_services.py

@@ -596,6 +596,43 @@ async def test_delete_user_service_can_hand_the_credential_over(
     assert not entry.runtime_data.retained_pairs
 
 
+async def test_delete_user_hands_over_every_credential_not_just_the_first(
+    hass: HomeAssistant,
+    mock_lock_config_entry,
+    lock_code_manager_config_entry,
+) -> None:
+    """
+    A batch hand-off applies to everybody in it.
+
+    One `delete_user` call is several entry writes -- a subentry removal per
+    departing user -- and each one wakes the update listener. The listener
+    drained the whole hand-off set on its first pass, so from the second user
+    onward the credential the caller explicitly asked to leave programmed was
+    wiped off every lock instead.
+    """
+    entry = lock_code_manager_config_entry
+    for lock in entry.runtime_data.locks.values():
+        lock.async_release_managed_slot = AsyncMock()
+
+    await hass.services.async_call(
+        DOMAIN,
+        SERVICE_DELETE_USER,
+        {
+            "config_entry_id": entry.entry_id,
+            CONF_NAME: ["test1", "test2"],
+            ATTR_CLEAR_CREDENTIALS: False,
+        },
+        blocking=True,
+    )
+    await hass.async_block_till_done()
+
+    config = get_entry_config(hass.config_entries.async_get_entry(entry.entry_id))
+    assert not config.users
+    for lock in entry.runtime_data.locks.values():
+        lock.async_release_managed_slot.assert_not_called()
+    assert not entry.runtime_data.retained_pairs
+
+
 async def test_delete_user_service_unknown_name(
     hass: HomeAssistant,
     mock_lock_config_entry,