fix(zigbee2mqtt): read PIN responses from the users payload; gate cached republications

custom_components/lock_code_manager/providers/_util.py

@@ -99,8 +99,12 @@ def parse_slot_num(value: object) -> int | None:
 
     Mirrors ``int(value)`` while collapsing the ``TypeError``/``ValueError``
     that providers otherwise catch when a lock reports a non-numeric slot key.
+    JSON booleans are rejected rather than coerced (``int(True)`` is 1, so a
+    malformed ``true`` would otherwise silently address slot 1).
     Call sites remain responsible for their own logging and skip/return flow.
     """
+    if isinstance(value, bool):
+        return None
     try:
         return int(value)  # type: ignore[call-overload]
     except TypeError, ValueError:

custom_components/lock_code_manager/providers/zigbee2mqtt.py

@@ -73,13 +73,50 @@ def _mqtt_payload_pin_has_code_value(pin_raw: Any) -> bool:
     return str(pin_raw) != ""
 
 
+def _project_z2m_user_state(user_info: dict[str, Any]) -> SlotCredential:
+    """
+    Project one Zigbee2MQTT ``users`` entry to a SlotCredential.
+
+    The status vocabulary comes from zigbee-herdsman-converters'
+    ``lockUserStatus`` map (available/enabled/disabled); statuses outside
+    it are published as ``not_supported_<n>``. Mapping traps:
+
+    - ``enabled`` without a usable PIN value is occupied-but-withheld
+      (``expose_pin`` off hides the code entirely), so it projects to
+      unreadable -- treating it as empty would make sync reprogram a slot
+      that already holds the right code.
+    - The one exception: an explicit ``pin_code: null`` on an enabled user
+      means the broker exposes the field and the device reports no code,
+      so that projects to empty.
+    - Unrecognized statuses (``not_supported_*``) project to unreadable,
+      not empty, for the same reprogramming-storm reason.
+    """
+    status = user_info.get("status")
+    pin_raw = user_info.get("pin_code")
+    if status == "enabled":
+        if _mqtt_payload_pin_has_code_value(pin_raw):
+            return SlotCredential.known(str(pin_raw))
+        if "pin_code" in user_info:
+            return SlotCredential.empty()
+        return SlotCredential.unreadable()
+    if status in ("available", "disabled"):
+        return SlotCredential.empty()
+    return SlotCredential.unreadable()
+
+
 @dataclass(repr=False, eq=False)
 class Zigbee2MQTTLock(BaseLock):
     """Class to represent Zigbee2MQTT lock."""
 
     _base_topic: str = field(init=False, default=DEFAULT_BASE_TOPIC)
     _friendly_name: str | None = field(init=False, default=None)
-    _pending_codes: dict[int, asyncio.Future[str | None]] = field(
+    _pending_codes: dict[int, asyncio.Future[SlotCredential]] = field(
+        init=False, default_factory=dict
+    )
+    # Last projected state per slot from the most recent users payload;
+    # the delta gate in _process_z2m_device_payload compares against this
+    # so full-cached-state republications don't repush stale entries.
+    _last_users_states: dict[int, SlotCredential] = field(
         init=False, default_factory=dict
     )
 
@@ -227,7 +264,7 @@ def _process_z2m_device_payload(self, payload: dict[str, Any]) -> None:
 
         users_data = payload.get("users")
         if users_data and isinstance(users_data, dict):
-            updates: dict[int, SlotCredential] = {}
+            states: dict[int, SlotCredential] = {}
             for user_id_str, user_info in users_data.items():
                 user_id = parse_slot_num(user_id_str)
                 if user_id is None:
@@ -247,31 +284,50 @@ def _process_z2m_device_payload(self, payload: dict[str, Any]) -> None:
                     )
                     continue
 
-                status = user_info.get("status")
-                pin_code_present = "pin_code" in user_info
-                pin_raw = user_info.get("pin_code")
-
-                # Zigbee2MQTT often omits pin_code when expose_pin is false (default on
-                # several Yale models). Treating that as empty makes the coordinator think
-                # the slot is cleared, so disabling the slot skips clear_usercode while the
-                # lock still holds the PIN. Only treat as empty when MQTT exposes the field.
-                if status == "enabled":
-                    if _mqtt_payload_pin_has_code_value(pin_raw):
-                        updates[user_id] = SlotCredential.known(str(pin_raw))
-                    elif pin_code_present:
-                        updates[user_id] = SlotCredential.empty()
-                    else:
-                        continue
-                else:
-                    updates[user_id] = SlotCredential.empty()
-
-            if updates and self.coordinator:
-                LOGGER.debug(
-                    "Lock %s received push update for slots: %s",
-                    self.lock.entity_id,
-                    list(updates.keys()),
-                )
-                self.coordinator.push_update(updates)
+                states[user_id] = _project_z2m_user_state(user_info)
+
+            # The converter answers GetPinCode through the users object
+            # (fz.lock_pin_code_response), not through a pin_code response
+            # payload -- resolve the pending read here or every slot read
+            # times out (issue #1335). At most one read is pending at a
+            # time (async_get_users queries slots sequentially), so cached
+            # entries for other slots cannot satisfy a future they don't
+            # belong to.
+            for user_id, state in states.items():
+                if (
+                    future := self._pending_codes.pop(user_id, None)
+                ) is not None and not future.done():
+                    future.set_result(state)
+
+            # Zigbee2MQTT republishes its full cached state on every
+            # attribute change, so most users payloads restate old entries
+            # rather than report changes. Applying them verbatim lets a
+            # stale cache entry overwrite the optimistic push from a write
+            # that the device already accepted -- the slot flips back to
+            # its pre-write state and sync reprograms it forever (issue
+            # #1335). Gate on the previous payload so only entries that
+            # actually changed reach the coordinator.
+            #
+            # The gate only records payloads once a coordinator is
+            # attached: retained/live messages can arrive between
+            # async_setup's subscription and coordinator attach, and a
+            # pre-attach snapshot would gate out the first post-attach
+            # republication that should seed the initial state.
+            if self.coordinator is not None:
+                changed = {
+                    user_id: state
+                    for user_id, state in states.items()
+                    if self._last_users_states.get(user_id) != state
+                }
+                self._last_users_states.update(states)
+                if changed:
+                    LOGGER.debug(
+                        "Lock %s received push update for slots: %s",
+                        self.lock.entity_id,
+                        list(changed),
+                    )
+                    for user_id, state in changed.items():
+                        self._confirm_slot(user_id, state)
 
         pin_code_data = payload.get("pin_code")
         if pin_code_data and isinstance(pin_code_data, dict):
@@ -297,9 +353,9 @@ def _process_z2m_device_payload(self, payload: dict[str, Any]) -> None:
                     user_enabled = pin_code_data.get("user_enabled", False)
                     pin_code = pin_code_data.get("pin_code")
                     if user_enabled and _mqtt_payload_pin_has_code_value(pin_code):
-                        future.set_result(str(pin_code))
+                        future.set_result(SlotCredential.known(str(pin_code)))
                     else:
-                        future.set_result(None)
+                        future.set_result(SlotCredential.empty())
 
     async def _async_ensure_device_subscription(self) -> None:
         """Subscribe to the Z2M device topic; idempotent."""
@@ -624,9 +680,7 @@ async def async_get_users(self) -> list[User]:
                 )
                 slot_states[slot_num] = SlotCredential.unreadable()
             else:
-                slot_states[slot_num] = (
-                    SlotCredential.known(result) if result else SlotCredential.empty()
-                )
+                slot_states[slot_num] = result
             finally:
                 self._pending_codes.pop(slot_num, None)
 

tests/providers/test_util.py

@@ -152,6 +152,10 @@ class TestParseSlotNum:
             pytest.param("five", None, id="str-non-numeric"),
             pytest.param(None, None, id="none"),
             pytest.param([], None, id="non-coercible-type"),
+            # int() coerces booleans to 0/1; a malformed JSON true must not
+            # silently address slot 1.
+            pytest.param(True, None, id="bool-true-rejected"),
+            pytest.param(False, None, id="bool-false-rejected"),
         ],
     )
     def test_parse_slot_num(self, input_value: object, expected: int | None) -> None:

tests/providers/zigbee2mqtt/test_payload.py

@@ -25,13 +25,80 @@ def test_mqtt_payload_pin_has_code_value_rejects_bool() -> None:
     assert _mqtt_payload_pin_has_code_value(True) is False
 
 
-def test_users_enabled_without_pin_key_skips_push() -> None:
-    """Do not infer EMPTY when expose_pin hides pin_code (enabled user, key absent)."""
+def test_users_enabled_without_pin_key_pushes_unreadable() -> None:
+    """An enabled user with expose_pin hiding pin_code is occupied-but-withheld."""
     lock = _minimal_lock()
     lock.coordinator = MagicMock()
     lock._process_z2m_device_payload({"users": {"5": {"status": "enabled"}}})
+    lock.coordinator.push_update.assert_called_once_with(
+        {5: SlotCredential.unreadable()}
+    )
+
+
+def test_users_not_supported_status_pushes_unreadable() -> None:
+    """An unrecognized status must not be misread as confirmed-empty."""
+    lock = _minimal_lock()
+    lock.coordinator = MagicMock()
+    lock._process_z2m_device_payload({"users": {"3": {"status": "not_supported_5"}}})
+    lock.coordinator.push_update.assert_called_once_with(
+        {3: SlotCredential.unreadable()}
+    )
+
+
+def test_users_republication_of_unchanged_state_not_repushed() -> None:
+    """Zigbee2MQTT republishes its full cached state; unchanged entries stay quiet.
+
+    Without the delta gate, a stale cached entry republished after LCM's
+    optimistic write flips the slot back and sync reprograms it forever
+    (issue #1335).
+    """
+    lock = _minimal_lock()
+    lock.coordinator = MagicMock()
+    payload = {"users": {"2": {"status": "enabled", "pin_code": "1234"}}}
+
+    lock._process_z2m_device_payload(payload)
+    lock.coordinator.push_update.assert_called_once_with(
+        {2: SlotCredential.known("1234")}
+    )
+
+    lock.coordinator.push_update.reset_mock()
+    lock._process_z2m_device_payload(payload)
     lock.coordinator.push_update.assert_not_called()
 
+    # A genuine change still gets through.
+    lock._process_z2m_device_payload({"users": {"2": {"status": "available"}}})
+    lock.coordinator.push_update.assert_called_once_with({2: SlotCredential.empty()})
+
+
+async def test_users_payload_resolves_pending_read() -> None:
+    """GetPinCode responses arrive via the users object (issue #1335).
+
+    The converter (fz.lock_pin_code_response) publishes the response inside
+    ``users``; the pending read future must resolve from it instead of
+    timing out.
+    """
+    loop = asyncio.get_running_loop()
+    lock = _minimal_lock()
+    lock.coordinator = MagicMock()
+
+    fut_known = loop.create_future()
+    lock._pending_codes[2] = fut_known
+    lock._process_z2m_device_payload(
+        {"users": {"2": {"status": "enabled", "pin_code": "1234"}}}
+    )
+    assert fut_known.done() and fut_known.result() == SlotCredential.known("1234")
+    assert 2 not in lock._pending_codes
+
+    fut_hidden = loop.create_future()
+    lock._pending_codes[3] = fut_hidden
+    lock._process_z2m_device_payload({"users": {"3": {"status": "enabled"}}})
+    assert fut_hidden.done() and fut_hidden.result() == SlotCredential.unreadable()
+
+    fut_available = loop.create_future()
+    lock._pending_codes[4] = fut_available
+    lock._process_z2m_device_payload({"users": {"4": {"status": "available"}}})
+    assert fut_available.done() and fut_available.result() == SlotCredential.empty()
+
 
 def test_users_enabled_with_numeric_zero_pin_updates() -> None:
     """Numeric zero is a valid digit; it must not be treated as a missing PIN."""
@@ -63,7 +130,7 @@ def test_users_non_numeric_slot_key_skipped() -> None:
     lock.coordinator.push_update.assert_not_called()
 
 
-async def test_pin_code_get_disabled_or_empty_pin_sets_future_none() -> None:
+async def test_pin_code_get_disabled_or_empty_pin_sets_future_empty() -> None:
     """PIN response with disabled user, empty pin, or numeric zero."""
     loop = asyncio.get_running_loop()
     lock = _minimal_lock()
@@ -73,21 +140,21 @@ async def test_pin_code_get_disabled_or_empty_pin_sets_future_none() -> None:
     lock._process_z2m_device_payload(
         {"pin_code": {"user": 7, "user_enabled": False, "pin_code": "1234"}}
     )
-    assert fut_disabled.done() and fut_disabled.result() is None
+    assert fut_disabled.done() and fut_disabled.result() == SlotCredential.empty()
 
     fut_empty = loop.create_future()
     lock._pending_codes[8] = fut_empty
     lock._process_z2m_device_payload(
         {"pin_code": {"user": 8, "user_enabled": True, "pin_code": ""}}
     )
-    assert fut_empty.done() and fut_empty.result() is None
+    assert fut_empty.done() and fut_empty.result() == SlotCredential.empty()
 
     fut_zero = loop.create_future()
     lock._pending_codes[9] = fut_zero
     lock._process_z2m_device_payload(
         {"pin_code": {"user": 9, "user_enabled": True, "pin_code": 0}}
     )
-    assert fut_zero.done() and fut_zero.result() == "0"
+    assert fut_zero.done() and fut_zero.result() == SlotCredential.known("0")
 
 
 async def test_pin_code_deleted_schedules_refresh_task(
@@ -276,3 +343,32 @@ def test_boolean_action_user_is_ignored() -> None:
     lock._process_z2m_device_payload({"action": "keypad_lock", "action_user": False})
 
     lock.async_fire_code_slot_event.assert_not_called()
+
+
+def test_users_payload_before_coordinator_does_not_poison_delta_gate() -> None:
+    """A retained payload arriving before coordinator attach must not gate out
+    the first post-attach republication (it should still seed initial state)."""
+    lock = _minimal_lock()
+    payload = {"users": {"2": {"status": "enabled", "pin_code": "1234"}}}
+
+    assert lock.coordinator is None
+    lock._process_z2m_device_payload(payload)
+
+    lock.coordinator = MagicMock()
+    lock._process_z2m_device_payload(payload)
+    lock.coordinator.push_update.assert_called_once_with(
+        {2: SlotCredential.known("1234")}
+    )
+
+
+def test_pin_code_boolean_user_does_not_resolve_slot_one() -> None:
+    """A malformed boolean ``user`` must not address slot 1 (int(True) == 1)."""
+    lock = _minimal_lock()
+    fut = MagicMock(spec=["cancel", "done", "set_result"])
+    fut.done.return_value = False
+    lock._pending_codes[1] = fut  # type: ignore[assignment]
+    lock._process_z2m_device_payload(
+        {"pin_code": {"user": True, "user_enabled": True, "pin_code": "9999"}}
+    )
+    fut.set_result.assert_not_called()
+    assert 1 in lock._pending_codes