diff --git a/custom_components/lock_code_manager/providers/_zwave_js_uc.py b/custom_components/lock_code_manager/providers/_zwave_js_uc.py index a0ce5a82..01ca7a08 100644 --- a/custom_components/lock_code_manager/providers/_zwave_js_uc.py +++ b/custom_components/lock_code_manager/providers/_zwave_js_uc.py @@ -133,6 +133,16 @@ def node(self) -> Node: """Return the Z-Wave JS node; the concrete provider supplies this.""" raise NotImplementedError + def _pin_state(self, data: str | bytes | None) -> SlotCredential: + """ + Project raw credential data to a SlotCredential. + + Universal masked/withheld-aware projection, implemented by the + concrete provider (``ZWaveJSLock._pin_state``) and reused here so + the UC fallback's read path matches the unified path exactly. + """ + raise NotImplementedError + def _node_supports_user_code_cc(self) -> bool: """Return whether the node's endpoint 0 advertises User Code CC.""" return any(cc.id == CommandClass.USER_CODE for cc in self.node.command_classes) @@ -173,15 +183,14 @@ async def _async_uc_fallback_active(self) -> bool: def _uc_fallback_capabilities(self) -> LockCapabilities | None: """ - Detect the fallback and build slot-only capabilities for it. + Build slot-only capabilities for a degenerate-capability lock. - Called by ``async_get_capabilities`` after the unified API - reported no usable PIN support. Only falls back when the node - advertises User Code CC -- without it the legacy utilities - cannot work either, and the lock genuinely has no PIN support - LCM can manage -- and when the User Code CC value DB walk finds - slots. Sets ``_uc_fallback`` accordingly and returns None when - no fallback is possible. + Called by ``async_get_capabilities`` only when the unified API + reports no usable PIN slots (the #1251 zero-slot variant). Falls + back to the legacy User Code CC value-DB walk for the slot count. + Returns None -- and clears ``_uc_fallback`` -- when the node has + no User Code CC, so a true U3C lock with degenerate caps (which + the legacy utilities can't help) is not mis-routed here. ``get_usercodes`` walks slot 1, 2, 3, ... in the value DB until ``NotFoundError``, so the returned list length is the lock's @@ -190,18 +199,19 @@ def _uc_fallback_capabilities(self) -> LockCapabilities | None: walking, so we let any unexpected exception surface rather than silently mis-routing the lock to "no PIN support". """ - uc_slots = ( - get_usercodes(self.node) if self._node_supports_user_code_cc() else [] - ) - if not uc_slots: + if not self._node_supports_user_code_cc(): + self._uc_fallback = False + return None + num_slots = len(get_usercodes(self.node)) + if not num_slots: self._uc_fallback = False return None - _LOGGER.warning( - "Lock %s: unified access-control API reports no usable PIN " - "capabilities but the node supports User Code CC with %s slots; " - "falling back to legacy User Code CC handling (see issue #1251)", + _LOGGER.debug( + "Lock %s: unified API reports no usable PIN slots but the node " + "supports User Code CC (%s slots); using the legacy User Code " + "CC value path (see issue #1251)", self.lock.entity_id, - len(uc_slots), + num_slots, ) self._uc_fallback = True return LockCapabilities( @@ -214,7 +224,7 @@ def _uc_fallback_capabilities(self) -> LockCapabilities | None: max_users=0, credential_types={ CredentialType.PIN: CredentialTypeCapability( - num_slots=len(uc_slots), + num_slots=num_slots, # UC spec allows 4-10 ASCII digits per User Code CC v1+. min_length=4, max_length=10, @@ -224,18 +234,20 @@ def _uc_fallback_capabilities(self) -> LockCapabilities | None: max_user_name_length=0, ) - @staticmethod - def _uc_slot_state(in_use: bool | None, usercode: str | None) -> SlotCredential: + def _uc_slot_state( + self, in_use: bool | None, usercode: str | None + ) -> SlotCredential: """ Project a User Code CC slot to a ``SlotCredential``. - Slots count as empty only when ``in_use`` is explicitly ``False``, - or when ``in_use`` is unknown (``None``) with no cached value -- - the latter matches the legacy 3.x reader. Masked codes (all - asterisks) and occupied slots without a cached value count as - unreadable; an unknown ``in_use`` with a present value is treated - as occupied so a partially populated cache cannot erase a live - PIN (mirrors the push-path rule at ``_handle_uc_value_update``). + Adds the User Code CC ``in_use`` (userIdStatus) gate on top of the + shared ``_pin_state`` projection. Slots count as empty only when + ``in_use`` is explicitly ``False``, or when ``in_use`` is unknown + (``None``) with no cached value -- the latter matches the legacy + 3.x reader. An occupied (or unknown-but-present) slot defers to + ``_pin_state`` so masked/withheld codes map to unreadable exactly + as on the unified path, and a partially populated cache cannot + erase a live PIN. """ if in_use is False: return SlotCredential.empty() @@ -245,10 +257,7 @@ def _uc_slot_state(in_use: bool | None, usercode: str | None) -> SlotCredential: if in_use is None else SlotCredential.unreadable() ) - code = str(usercode) - if code == "*" * len(code): - return SlotCredential.unreadable() - return SlotCredential.known(code) + return self._pin_state(usercode) async def _async_uc_users_from_value_db(self) -> list[User]: """ diff --git a/custom_components/lock_code_manager/providers/matter.py b/custom_components/lock_code_manager/providers/matter.py index b6ef7fa9..cece5355 100644 --- a/custom_components/lock_code_manager/providers/matter.py +++ b/custom_components/lock_code_manager/providers/matter.py @@ -14,6 +14,7 @@ from datetime import timedelta from typing import Any, Literal +from matter_server.client.exceptions import MatterClientException from matter_server.common.errors import MatterError from matter_server.common.models import EventType @@ -71,6 +72,39 @@ _DATA_OP_MODIFY = 2 +def _is_transient_credential_status(status: str) -> bool: + """ + Return True for a SetCredential status that should be retried, not rejected. + + HA's matter ``lock_helpers.set_lock_credential`` maps recognized DlStatus + values to names (``success`` / ``failure`` / ``duplicate`` / ``occupied``) + and formats anything else as ``unknown()`` -- + ``SET_CREDENTIAL_STATUS_MAP.get(status_code, f"unknown({status_code})")`` -- + discarding the raw code. So ``unknown(`` is the only signal LCM has for "the + lock returned a status the helper did not recognize", and an unmapped status + is treated as transient (route to the retry path) -- notably ``unknown(133)`` + observed while a lock is not fully ready right after startup (issue #1257) -- + rather than a definitive rejection that permanently disables the slot. + + FRAGILE COUPLING: this depends on HA's private ``unknown()`` format, + which has no stability contract. If that helper ever changes the format or + starts mapping a code we rely on (e.g. 133 -> a real name), this predicate + silently returns False and the #1257 not-ready case regresses to a permanent + suspend. ``tests/providers/matter/test_provider.py`` pins the current format. + """ + return status.startswith("unknown(") + + +def _transient_status_disconnect( + entity_id: str, slot: int, status: str +) -> LockDisconnected: + """Build the LockDisconnected raised for a transient SetCredential status.""" + return LockDisconnected( + f"Matter set_lock_credential returned a transient status " + f"'{status}' for {entity_id} slot {slot}" + ) + + def _lcm_slot_from_raw_users_by_user_index( raw_users: list[dict[str, Any]], user_index: int | None ) -> int | None: @@ -684,8 +718,12 @@ async def _send_set_credential( lock_entity_id=self.lock.entity_id, reason=str(err), ) from err - except HomeAssistantError as err: - # Transport / endpoint failure -> route to the retry path. + except (HomeAssistantError, MatterError, MatterClientException) as err: + # Transport / connectivity / server failure -> route to the retry + # path. MatterError and MatterClientException are independent of + # HomeAssistantError (e.g. ``InvalidState: Not connected`` during + # startup, issue #1257), so they must be caught explicitly or they + # escape to the generic handler and suspend the slot. raise LockDisconnected( f"Matter set_lock_credential failed for {self.lock.entity_id}: {err}" ) from err @@ -749,6 +787,15 @@ async def async_set_credential( ) except SetCredentialFailedError as err: status = (err.translation_placeholders or {}).get("status", "") + if _is_transient_credential_status(status): + # Unmapped/unknown status (e.g. ``unknown(133)`` seen while the + # lock is not fully ready at startup, issue #1257). Route to the + # retry path rather than permanently disabling the slot: a later + # tick after Matter is ready will succeed. Recognized rejections + # (occupied/failure) fall through to CodeRejectedError below. + raise _transient_status_disconnect( + self.lock.entity_id, slot, status + ) from err if status != "duplicate": raise CodeRejectedError( code_slot=slot, @@ -787,7 +834,15 @@ async def async_set_credential( f"Matter clear_lock_credential rejected input for " f"{self.lock.entity_id} during sync-duplicate retry: {clear_err}" ) from clear_err - except HomeAssistantError as clear_err: + except ( + HomeAssistantError, + MatterError, + MatterClientException, + ) as clear_err: + # Same connectivity-vs-HomeAssistantError split as the other + # clear/set sites (issue #1257): a MatterClientException here + # (e.g. ``InvalidState: Not connected`` mid-retry) must route to + # retry, not escape to the generic handler and suspend the slot. raise LockDisconnected( f"Matter clear_lock_credential failed for " f"{self.lock.entity_id} during sync-duplicate retry: {clear_err}" @@ -805,6 +860,10 @@ async def async_set_credential( code_slot=slot, lock_entity_id=self.lock.entity_id, ) from retry_err + if _is_transient_credential_status(retry_status): + raise _transient_status_disconnect( + self.lock.entity_id, slot, retry_status + ) from retry_err raise CodeRejectedError( code_slot=slot, lock_entity_id=self.lock.entity_id, @@ -846,7 +905,9 @@ async def async_delete_credential(self, ref: CredentialRef) -> bool: f"Matter clear_lock_credential rejected input for " f"{self.lock.entity_id}: {err}" ) from err - except HomeAssistantError as err: + except (HomeAssistantError, MatterError, MatterClientException) as err: + # Connectivity/server failure (incl. ``InvalidState: Not connected`` + # at startup, issue #1257) -> retry rather than suspend. raise LockDisconnected( f"Matter clear_lock_credential failed for {self.lock.entity_id}: {err}" ) from err diff --git a/custom_components/lock_code_manager/providers/zwave_js.py b/custom_components/lock_code_manager/providers/zwave_js.py index 097996a8..ae42bca6 100644 --- a/custom_components/lock_code_manager/providers/zwave_js.py +++ b/custom_components/lock_code_manager/providers/zwave_js.py @@ -157,12 +157,26 @@ def supports_native_users(self) -> bool: return True def _pin_state(self, data: str | bytes | None) -> SlotCredential: - """Project Z-Wave credential data to a SlotCredential (readable when present).""" + """ + Project Z-Wave credential data to a SlotCredential. + + Universal across both command classes (User Code CC and User + Credential CC): a present slot whose code we can read becomes + ``known``; a present slot whose code is withheld becomes + ``unreadable``. Many locks report the PIN back masked (all + asterisks) or omit it entirely for security -- those carry no + comparable value, so they are unreadable, NOT ``known`` of the + masked string (which would surface a wrong PIN and never + reconcile against the configured one). + """ if not data: return SlotCredential.unreadable() # CredentialData.data is str | bytes; decode bytes so a Personal # Identification Number is the digit string, not "b'1234'". - return SlotCredential.known(data if isinstance(data, str) else data.decode()) + code = data if isinstance(data, str) else data.decode() + if not code or code == "*" * len(code): + return SlotCredential.unreadable() + return SlotCredential.known(code) async def async_get_users(self) -> list[User]: """ @@ -228,27 +242,30 @@ async def async_get_capabilities(self) -> LockCapabilities: """ Report the lock's user/credential capabilities. - Auto-detects User Code CC (UC) vs User Credential CC (U3C) and - returns capabilities shaped so the seam routes through the right - code path. - - The unified ``access_control`` API in node-zwave-js computes its - capabilities from cached interview data; on some locks that data - is degenerate -- the PIN credential type comes back either - missing or advertising ``num_slots=0`` even though the lock - manages codes fine through the legacy User Code CC (issue - #1251, upstream fix in zwave-js/zwave-js#8873). When that - happens AND the node actually advertises User Code CC, we fall - back to reading the lock's UC slot count from the value DB and - return slot-only capabilities: ``supports_user_management=False`` - and ``max_user_name_length=0``, which the seam recognizes as a - slot-only lock and routes through the credential-only - primitives (``async_set_credential`` / ``async_delete_credential`` - / ``async_get_users``) without the user lifecycle. All - credential operations then use the User Code CC utilities - directly. Once the upstream fix ships and the unified API - reports usable PIN capabilities for these locks, the fallback - detection stops triggering on its own. + Routes the lock based on whether the unified API can express its PIN + capabilities: + + - **Usable PIN capabilities reported** (``num_slots > 0``) -> the + unified ``access_control`` API. This is the common path for both + U3C locks and healthy User Code CC-only locks. Masked-code locks + stay correct here via the universal read projection (``_pin_state`` + maps a withheld code to ``unreadable``) and tolerant write handling + (a driver ``ERROR_UNKNOWN`` from the masked read-back verification + is treated as a completed set in ``async_set_credential``, not a + rejection) -- see issue #1251. + - **Degenerate capabilities** (PIN missing or ``num_slots == 0``) -> + the legacy User Code CC fallback (slot-only), which addresses slots + directly because the unified API can't even express them. Only the + zero-slot variant needs this; a node with no User Code CC at all has + no PIN support LCM can manage. + + Note: this method does NOT route by command class -- a User Code + CC-only lock with healthy capabilities uses the unified path, relying + on the projection + tolerant-write handling above rather than the + legacy fallback. Only degenerate capabilities trigger the fallback. + Once the upstream driver fixes the masked-read verification and the + capability reporting (zwave-js/zwave-js#8873), the fallback can be + removed entirely (see ``_zwave_js_uc.py``). """ try: caps = await lock_helpers.async_get_credential_capabilities(self.node) @@ -259,7 +276,10 @@ async def async_get_capabilities(self) -> LockCapabilities: pin = caps["supported_credential_types"].get(_PIN_TYPE_STR) if pin and pin["num_slots"] > 0: - # The unified API advertises real PIN credential slots. + # The unified API advertises real PIN credential slots. Use it; + # the universal read projection (``_pin_state``) and tolerant + # write handling below keep masked-code locks working on this + # path regardless of which CC the driver dispatches to. self._uc_fallback = False return LockCapabilities( supports_user_management=caps["supports_user_management"], @@ -275,11 +295,14 @@ async def async_get_capabilities(self) -> LockCapabilities: max_user_name_length=caps.get("max_user_name_length", 0), ) - # Degenerate unified capabilities (issue #1251): try the User Code - # CC fallback layer; when it does not apply either, the lock has - # no PIN support LCM can manage. + # Degenerate unified capabilities (issue #1251 zero-slot variant): + # the unified API can't even express the lock's PIN slots, so it + # can't write through them. Fall back to the legacy User Code CC + # utilities, which address slots directly. When the node has no + # User Code CC either, the lock genuinely has no PIN support. if uc_caps := self._uc_fallback_capabilities(): return uc_caps + self._uc_fallback = False return LockCapabilities( supports_user_management=False, max_users=0, @@ -420,6 +443,17 @@ async def async_set_credential( Otherwise the write goes through HA's ``lock_helpers.async_set_credential``, whose translation-key errors are mapped to LCM's typed exceptions. + + A driver ``ERROR_UNKNOWN`` (HA key ``credential_rejected_unknown``) + is treated as a COMPLETED set rather than a rejection: the driver + returns it when its post-write verification can't confirm the + code, which notably happens for locks that report the user code + back masked/withheld -- there the write actually succeeded + (``userIdStatus`` -> Enabled). Reconciliation (the sync manager's + last-set tracking + the masked-as-unreadable read-back) verifies + it, instead of permanently disabling an accepted write (#1251). + Definitive rejections (duplicate, occupied, manufacturer rules, + validation) still surface as typed errors. """ if await self._async_uc_fallback_active(): return await self._async_uc_set_usercode(credential.slot, pin) @@ -438,11 +472,22 @@ async def async_set_credential( f"set credential slot {credential.slot} failed: {err}" ) from err except HomeAssistantError as err: - if getattr(err, "translation_key", None) == "credential_rejected_duplicate": + key = getattr(err, "translation_key", None) + if key == "credential_rejected_duplicate": raise DuplicateCodeError( code_slot=credential.slot, lock_entity_id=self.lock.entity_id, ) from err + if key == "credential_rejected_unknown": + _LOGGER.debug( + "Lock %s slot %s: driver returned ERROR_UNKNOWN; treating " + "as a completed set pending reconciliation (the lock may " + "report the code back masked -- see issue #1251): %s", + self.lock.entity_id, + credential.slot, + err, + ) + return True raise CodeRejectedError( code_slot=credential.slot, lock_entity_id=self.lock.entity_id, diff --git a/tests/providers/matter/test_provider.py b/tests/providers/matter/test_provider.py index ed2c273d..2a450919 100644 --- a/tests/providers/matter/test_provider.py +++ b/tests/providers/matter/test_provider.py @@ -8,6 +8,7 @@ from typing import Any from unittest.mock import AsyncMock, MagicMock, patch +from matter_server.client.exceptions import MatterClientException from matter_server.common.errors import UnknownError from matter_server.common.models import EventType, MatterNodeEvent import pytest @@ -1684,6 +1685,36 @@ async def test_get_users_empty( users = await matter_lock_simple.async_get_users() assert users == [] + async def test_get_users_skips_raw_user_without_user_index( + self, hass: HomeAssistant, matter_lock_simple: MatterLock + ) -> None: + """A raw lock user with no user_index is skipped (cannot be projected).""" + mock_get_lock_users = AsyncMock( + return_value={ + "max_users": 10, + "users": [ + {"user_index": None, "credentials": [{"type": "pin", "index": 1}]}, + { + "user_index": 2, + "user_name": "Bob", + "credentials": [{"type": "pin", "index": 2}], + }, + ], + } + ) + with ( + patch.object( + matter_lock_simple, "_get_matter_client", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, "_get_matter_node", return_value=MagicMock() + ), + patch(f"{_PROVIDER_MODULE}.get_lock_users", mock_get_lock_users), + ): + users = await matter_lock_simple.async_get_users() + + assert [user.user_id for user in users] == [2] + async def test_get_users_pin_credentials_become_unreadable( self, hass: HomeAssistant, matter_lock_simple: MatterLock ) -> None: @@ -3025,6 +3056,79 @@ async def test_set_credential_duplicate_sync_persistent_raises( assert mock_set_credential.call_count == 2 assert mock_clear.call_count == 1 + async def test_set_credential_duplicate_sync_retry_transient_routes_to_retry( + self, hass: HomeAssistant, matter_lock_simple: MatterLock + ) -> None: + """A transient status on the post-clear retry routes to retry (#1257). + + The sync-duplicate path clears the existing credential and retries; + if that retry returns an unmapped ``unknown(N)`` status (lock not + ready), it must route to LockDisconnected rather than permanently + disabling the slot. + """ + mock_set_credential = AsyncMock( + side_effect=[ + _make_set_credential_failed_error("duplicate"), + _make_set_credential_failed_error("unknown(133)"), + ] + ) + mock_clear = AsyncMock(return_value={}) + credential = self._make_credential(slot=1, pin="1234") + with ( + patch.object( + matter_lock_simple, "_get_matter_client", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, "_get_matter_node", return_value=MagicMock() + ), + self._patch_user_with_pin(user_id=1, credential_index=10), + patch(f"{_PROVIDER_MODULE}.set_lock_credential", mock_set_credential), + patch(f"{_PROVIDER_MODULE}.clear_lock_credential", mock_clear), + pytest.raises(LockDisconnected), + ): + await matter_lock_simple.async_set_credential( + 1, credential, "1234", name=None, source="sync" + ) + + assert mock_set_credential.call_count == 2 + assert mock_clear.call_count == 1 + + async def test_set_credential_duplicate_sync_retry_definitive_rejection_raises( + self, hass: HomeAssistant, matter_lock_simple: MatterLock + ) -> None: + """A definitive rejection on the post-clear retry raises CodeRejectedError. + + Distinct from the transient (``unknown(N)``) retry, a recognized + rejection such as ``occupied`` is permanent, so it surfaces as a + code rejection rather than routing to retry. + """ + mock_set_credential = AsyncMock( + side_effect=[ + _make_set_credential_failed_error("duplicate"), + _make_set_credential_failed_error("occupied"), + ] + ) + mock_clear = AsyncMock(return_value={}) + credential = self._make_credential(slot=1, pin="1234") + with ( + patch.object( + matter_lock_simple, "_get_matter_client", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, "_get_matter_node", return_value=MagicMock() + ), + self._patch_user_with_pin(user_id=1, credential_index=10), + patch(f"{_PROVIDER_MODULE}.set_lock_credential", mock_set_credential), + patch(f"{_PROVIDER_MODULE}.clear_lock_credential", mock_clear), + pytest.raises(CodeRejectedError), + ): + await matter_lock_simple.async_set_credential( + 1, credential, "1234", name=None, source="sync" + ) + + assert mock_set_credential.call_count == 2 + assert mock_clear.call_count == 1 + async def test_set_credential_sync_duplicate_external_surfaces_immediately( self, hass: HomeAssistant, matter_lock_simple: MatterLock ) -> None: @@ -3084,6 +3188,38 @@ async def test_set_credential_sync_duplicate_clear_disconnected( # First set raised duplicate; clear failed; no retry attempt is made. assert mock_set_credential.call_count == 1 + async def test_set_credential_sync_duplicate_clear_matter_client_exception( + self, hass: HomeAssistant, matter_lock_simple: MatterLock + ) -> None: + """A MatterClientException during the duplicate-retry clear routes to retry. + + Regression guard (#1257): this clear path is independent of the main + clear and both set paths; a MatterClientException here must map to + LockDisconnected rather than escape to the generic handler and suspend + the slot. + """ + mock_set_credential = AsyncMock( + side_effect=_make_set_credential_failed_error("duplicate") + ) + mock_clear = AsyncMock(side_effect=MatterClientException("Not connected")) + credential = self._make_credential(slot=1, pin="1234") + with ( + patch.object( + matter_lock_simple, "_get_matter_client", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, "_get_matter_node", return_value=MagicMock() + ), + self._patch_user_with_pin(user_id=1, credential_index=10), + patch(f"{_PROVIDER_MODULE}.set_lock_credential", mock_set_credential), + patch(f"{_PROVIDER_MODULE}.clear_lock_credential", mock_clear), + pytest.raises(LockDisconnected, match="sync-duplicate retry"), + ): + await matter_lock_simple.async_set_credential( + 1, credential, "1234", name=None, source="sync" + ) + assert mock_set_credential.call_count == 1 + async def test_set_credential_sync_duplicate_clear_service_validation_error( self, hass: HomeAssistant, matter_lock_simple: MatterLock ) -> None: @@ -3174,6 +3310,59 @@ async def test_set_credential_transport_error_raises_lock_disconnected( 1, credential, "1234", name=None, source="direct" ) + async def test_set_credential_unknown_status_routes_to_retry( + self, hass: HomeAssistant, matter_lock_simple: MatterLock + ) -> None: + """An unmapped ``unknown(N)`` status routes to retry, not disable (#1257). + + Seen when a lock is not fully ready right after startup: the write + must retry on a later tick rather than permanently disabling the slot. + """ + mock_set_credential = AsyncMock( + side_effect=_make_set_credential_failed_error("unknown(133)") + ) + credential = self._make_credential(slot=1, pin="1234") + with ( + patch.object( + matter_lock_simple, "_get_matter_client", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, "_get_matter_node", return_value=MagicMock() + ), + patch(f"{_PROVIDER_MODULE}.set_lock_credential", mock_set_credential), + pytest.raises(LockDisconnected), + ): + await matter_lock_simple.async_set_credential( + 1, credential, "1234", name=None, source="sync" + ) + + async def test_set_credential_matter_client_exception_routes_to_retry( + self, hass: HomeAssistant, matter_lock_simple: MatterLock + ) -> None: + """``InvalidState: Not connected`` at startup routes to retry (#1257). + + MatterClientException is independent of HomeAssistantError, so without + an explicit catch it would escape to the generic handler and suspend + the slot instead of retrying. + """ + mock_set_credential = AsyncMock( + side_effect=MatterClientException("Not connected") + ) + credential = self._make_credential(slot=1, pin="1234") + with ( + patch.object( + matter_lock_simple, "_get_matter_client", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, "_get_matter_node", return_value=MagicMock() + ), + patch(f"{_PROVIDER_MODULE}.set_lock_credential", mock_set_credential), + pytest.raises(LockDisconnected), + ): + await matter_lock_simple.async_set_credential( + 1, credential, "1234", name=None, source="sync" + ) + async def test_set_credential_create_passes_credential_index_none( self, hass: HomeAssistant, matter_lock_simple: MatterLock ) -> None: @@ -3422,6 +3611,29 @@ async def test_delete_credential_transport_error_raises_lock_disconnected( ): await matter_lock_simple.async_delete_credential(ref) + async def test_delete_credential_matter_client_exception_routes_to_retry( + self, hass: HomeAssistant, matter_lock_simple: MatterLock + ) -> None: + """``InvalidState: Not connected`` during clear routes to retry (#1257).""" + mock_clear = AsyncMock(side_effect=MatterClientException("Not connected")) + ref = CredentialRef(user_id=1, type=CredentialType.PIN, slot=1) + with ( + patch.object( + matter_lock_simple, "_get_matter_client", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, "_get_matter_node", return_value=MagicMock() + ), + patch.object( + matter_lock_simple, + "_find_pin_credential_index_for_user", + AsyncMock(return_value=5), + ), + patch(f"{_PROVIDER_MODULE}.clear_lock_credential", mock_clear), + pytest.raises(LockDisconnected), + ): + await matter_lock_simple.async_delete_credential(ref) + async def test_delete_credential_validation_error_raises_operation_failed( self, hass: HomeAssistant, matter_lock_simple: MatterLock ) -> None: diff --git a/tests/providers/zwave_js/test_provider.py b/tests/providers/zwave_js/test_provider.py index 6b4d5060..466bd535 100644 --- a/tests/providers/zwave_js/test_provider.py +++ b/tests/providers/zwave_js/test_provider.py @@ -442,6 +442,34 @@ def raise_error(self): # Credential API tests (Option B: readable PINs via node.access_control) +@pytest.mark.parametrize( + ("data", "expected"), + [ + ("1234", SlotCredential.known("1234")), + (b"1234", SlotCredential.known("1234")), + (None, SlotCredential.unreadable()), + ("", SlotCredential.unreadable()), + # Masked/withheld codes carry no usable value -> unreadable, NOT + # known("****") (which would surface a wrong PIN and never reconcile). + ("****", SlotCredential.unreadable()), + ("**********", SlotCredential.unreadable()), + ], + ids=["known_str", "known_bytes", "none", "empty", "masked", "masked_long"], +) +async def test_pin_state_projects_masked_codes_as_unreadable( + zwave_js_lock: ZWaveJSLock, + data: str | bytes | None, + expected: SlotCredential, +) -> None: + """The unified read projection maps masked/withheld codes to unreadable. + + Mirrors the UC fallback's _uc_slot_state so a masked code read through + the unified access-control path is not mistaken for a readable PIN + (issue #1251 working-capability variant). + """ + assert zwave_js_lock._pin_state(data) == expected + + async def test_async_get_users_returns_all_mappable_credential_types( zwave_js_lock: ZWaveJSLock, mock_access_control: MagicMock, @@ -870,13 +898,13 @@ async def test_async_set_credential_raises_code_rejected_error_on_other_ha_error mock_lock_helpers: dict, ) -> None: """ - Test async_set_credential raises CodeRejectedError for non-duplicate rejections. + Test async_set_credential raises CodeRejectedError for definitive rejections. - When the lock_helpers helper raises HomeAssistantError with any other - translation_key (for example "credential_rejected_unknown"), the provider - must re-raise as CodeRejectedError. + When the lock_helpers helper raises HomeAssistantError with a definitive + rejection translation_key (for example "credential_rejected_manufacturer_rules"), + the provider must re-raise as CodeRejectedError. """ - err = HomeAssistantError(translation_key="credential_rejected_unknown") + err = HomeAssistantError(translation_key="credential_rejected_manufacturer_rules") mock_lock_helpers["async_set_credential"].side_effect = err credential = Credential( @@ -895,6 +923,39 @@ async def test_async_set_credential_raises_code_rejected_error_on_other_ha_error assert not isinstance(exc_info.value, DuplicateCodeError) +async def test_async_set_credential_tolerates_error_unknown_as_completed_set( + zwave_js_lock: ZWaveJSLock, + mock_access_control: MagicMock, + mock_lock_helpers: dict, +) -> None: + """ + A driver ERROR_UNKNOWN (credential_rejected_unknown) is treated as a + completed set, not a rejection. + + The driver returns ERROR_UNKNOWN when its post-write verification can't + confirm the code -- notably for locks that report the user code back + masked, where the write actually succeeded (issue #1251). The provider + must return True and let reconciliation verify, instead of raising + CodeRejectedError (which would permanently disable an accepted write). + """ + mock_lock_helpers["async_set_credential"].side_effect = HomeAssistantError( + translation_key="credential_rejected_unknown" + ) + + credential = Credential( + type=CredentialType.PIN, slot=4, state=SlotCredential.known("2222") + ) + result = await zwave_js_lock.async_set_credential( + user_id=1, + credential=credential, + pin="2222", + name=None, + source="sync", + ) + + assert result is True + + async def test_async_set_credential_maps_failed_command_to_lock_disconnected( zwave_js_lock: ZWaveJSLock, mock_access_control: MagicMock, diff --git a/tests/providers/zwave_js/test_uc_fallback.py b/tests/providers/zwave_js/test_uc_fallback.py index be732957..b175a0cf 100644 --- a/tests/providers/zwave_js/test_uc_fallback.py +++ b/tests/providers/zwave_js/test_uc_fallback.py @@ -880,6 +880,16 @@ async def test_support_layer_node_stub_raises() -> None: ZWaveJSUserCodeFallbackSupport.node.fget(None) +async def test_support_layer_pin_state_stub_raises() -> None: + """The support layer's _pin_state must be overridden by the provider. + + It exists only so the UC fallback's read path can reuse the concrete + provider's universal masked/withheld projection. + """ + with pytest.raises(NotImplementedError): + ZWaveJSUserCodeFallbackSupport._pin_state(None, "1234") + + async def test_usercode_cc_version_defaults_to_v1_when_cc_missing( zwave_js_lock: ZWaveJSLock, ) -> None: @@ -1302,7 +1312,10 @@ async def test_uc_value_update_duplicate_value_skipped( ], ) def test_uc_slot_state_projection( - in_use: bool | None, usercode: str | None, expected: SlotCredential + zwave_js_lock: ZWaveJSLock, + in_use: bool | None, + usercode: str | None, + expected: SlotCredential, ) -> None: """_uc_slot_state must distinguish None (unknown) from False (cleared).""" - assert ZWaveJSUserCodeFallbackSupport._uc_slot_state(in_use, usercode) == expected + assert zwave_js_lock._uc_slot_state(in_use, usercode) == expected