fix(zwave_js): tolerate transient SetValueResult statuses on UC writes (#1251)

custom_components/lock_code_manager/providers/_zwave_js_uc.py

@@ -82,6 +82,21 @@
     SetValueStatus.WORKING,
 )
 
+# SetValueResult statuses that are structural impossibilities for this
+# lock/endpoint -- the operation cannot succeed no matter how many times
+# we retry. Surface these as a real rejection. Any *other* non-OK status
+# (notably ``FAIL``) is treated as transient: 3.x routed sets through the
+# HA service ``zwave_js.set_lock_usercode``, which never inspected the
+# status at all, so those locks worked fine on flaky firmware that
+# returned ``FAIL`` intermittently. Raising on transient ``FAIL`` is
+# exactly what disabled TheMegamind's slots on 4.0.3-4.0.4 (#1251).
+_UC_SET_VALUE_FATAL = (
+    SetValueStatus.NO_DEVICE_SUPPORT,
+    SetValueStatus.ENDPOINT_NOT_FOUND,
+    SetValueStatus.NOT_IMPLEMENTED,
+    SetValueStatus.INVALID_VALUE,
+)
+
 
 @dataclass(repr=False, eq=False)
 class ZWaveJSUserCodeFallbackSupport(BaseLock):
@@ -333,11 +348,22 @@ async def _async_uc_set_usercode(self, code_slot: int, usercode: str) -> bool:
                 f"set usercode slot {code_slot} failed: {err}"
             ) from err
         if result is not None and result.status not in _UC_SET_VALUE_OK:
-            self._set_in_progress_code_slot = None
-            raise CodeRejectedError(
-                code_slot=code_slot,
-                lock_entity_id=self.lock.entity_id,
-                reason=f"set value returned {result.status.name}",
+            if result.status in _UC_SET_VALUE_FATAL:
+                self._set_in_progress_code_slot = None
+                raise CodeRejectedError(
+                    code_slot=code_slot,
+                    lock_entity_id=self.lock.entity_id,
+                    reason=f"set value returned {result.status.name}",
+                )
+            # Transient non-OK (canonically ``FAIL``): match the 3.x
+            # behavior of the HA service we used to call -- log and
+            # let the optimistic push + next sync tick converge.
+            _LOGGER.info(
+                "Lock %s slot %s: set returned %s; "
+                "trusting optimistic push and continuing",
+                self.lock.entity_id,
+                code_slot,
+                result.status.name,
             )
         await self._async_uc_verify_write(code_slot, "set")
         # Optimistic update: the value cache updates asynchronously via push
@@ -379,9 +405,18 @@ async def _async_uc_clear_usercode(self, code_slot: int) -> bool:
                 f"clear usercode slot {code_slot} failed: {err}"
             ) from err
         if result is not None and result.status not in _UC_SET_VALUE_OK:
-            raise LockOperationFailed(
-                f"clear usercode slot {code_slot} failed: "
-                f"set value returned {result.status.name}"
+            if result.status in _UC_SET_VALUE_FATAL:
+                raise LockOperationFailed(
+                    f"clear usercode slot {code_slot} failed: "
+                    f"set value returned {result.status.name}"
+                )
+            # Transient non-OK: see _async_uc_set_usercode for rationale.
+            _LOGGER.info(
+                "Lock %s slot %s: clear returned %s; "
+                "trusting optimistic push and continuing",
+                self.lock.entity_id,
+                code_slot,
+                result.status.name,
             )
         await self._async_uc_verify_write(code_slot, "clear")
         # Optimistic update: see _async_uc_set_usercode for rationale.

tests/providers/zwave_js/test_uc_fallback.py

@@ -228,19 +228,69 @@ async def test_uc_set_credential_proceeds_when_masked(
     mock_uc_utils["set_usercode"].assert_awaited_once()
 
 
-async def test_uc_set_credential_failure_status_raises_code_rejected(
+async def test_uc_set_fail_status_logs_and_continues(
     uc_fallback_lock: ZWaveJSLock,
     mock_uc_utils: dict,
+    caplog: pytest.LogCaptureFixture,
 ) -> None:
-    """A FAIL set-value result maps to CodeRejectedError."""
+    """A transient ``FAIL`` set-value status is tolerated, not a rejection.
+
+    3.3.0 routed sets through the HA service ``zwave_js.set_lock_usercode``,
+    which never inspected ``SetValueResult.status`` -- transient ``FAIL``
+    responses (the canonical case on flaky-interview Schlage UC v1 locks,
+    #1251) were silently swallowed and the slot stayed enabled. The
+    structural statuses (``NO_DEVICE_SUPPORT`` etc.) still escalate to
+    ``CodeRejectedError`` because they are real impossibilities.
+    """
+    mock_coordinator = MagicMock()
+    mock_coordinator.data = {}
+    uc_fallback_lock.coordinator = mock_coordinator
+
     mock_uc_utils["set_usercode"].return_value = SetValueResult(
         {"status": SetValueStatus.FAIL}
     )
 
     credential = Credential(
         type=CredentialType.PIN, slot=5, state=SlotCredential.known("4321")
     )
-    with pytest.raises(CodeRejectedError, match="FAIL"):
+    with caplog.at_level(logging.INFO):
+        result = await uc_fallback_lock.async_set_credential(
+            user_id=5, credential=credential, pin="4321", name=None, source="sync"
+        )
+
+    assert result is True
+    mock_coordinator.push_update.assert_called_once_with(
+        {5: SlotCredential.known("4321")}
+    )
+    assert "set returned FAIL" in caplog.text
+
+
+@pytest.mark.parametrize(
+    "status",
+    [
+        SetValueStatus.NO_DEVICE_SUPPORT,
+        SetValueStatus.ENDPOINT_NOT_FOUND,
+        SetValueStatus.NOT_IMPLEMENTED,
+        SetValueStatus.INVALID_VALUE,
+    ],
+    ids=lambda s: s.name,
+)
+async def test_uc_set_fatal_status_raises_code_rejected(
+    uc_fallback_lock: ZWaveJSLock,
+    mock_uc_utils: dict,
+    status: SetValueStatus,
+) -> None:
+    """Structural-impossibility statuses still raise ``CodeRejectedError``.
+
+    These mean the operation cannot succeed on this device/endpoint --
+    not transient -- so the slot should be rejected loudly.
+    """
+    mock_uc_utils["set_usercode"].return_value = SetValueResult({"status": status})
+
+    credential = Credential(
+        type=CredentialType.PIN, slot=5, state=SlotCredential.known("4321")
+    )
+    with pytest.raises(CodeRejectedError, match=status.name):
         await uc_fallback_lock.async_set_credential(
             user_id=5, credential=credential, pin="4321", name=None, source="sync"
         )
@@ -982,17 +1032,49 @@ async def test_uc_clear_credential_transport_error_raises_lock_disconnected(
         await uc_fallback_lock.async_delete_credential(ref)
 
 
-async def test_uc_clear_credential_failure_status_raises_operation_failed(
+async def test_uc_clear_fail_status_logs_and_continues(
     uc_fallback_lock: ZWaveJSLock,
     mock_uc_utils: dict,
+    caplog: pytest.LogCaptureFixture,
 ) -> None:
-    """A FAIL set-value result during clear maps to LockOperationFailed."""
+    """A transient ``FAIL`` clear status is tolerated; mirrors the set path."""
+    mock_coordinator = MagicMock()
+    mock_coordinator.data = {}
+    uc_fallback_lock.coordinator = mock_coordinator
+
     mock_uc_utils["clear_usercode"].return_value = SetValueResult(
         {"status": SetValueStatus.FAIL}
     )
 
     ref = CredentialRef(user_id=3, type=CredentialType.PIN, slot=3)
-    with pytest.raises(LockOperationFailed, match="FAIL"):
+    with caplog.at_level(logging.INFO):
+        result = await uc_fallback_lock.async_delete_credential(ref)
+
+    assert result is True
+    mock_coordinator.push_update.assert_called_once_with({3: SlotCredential.empty()})
+    assert "clear returned FAIL" in caplog.text
+
+
+@pytest.mark.parametrize(
+    "status",
+    [
+        SetValueStatus.NO_DEVICE_SUPPORT,
+        SetValueStatus.ENDPOINT_NOT_FOUND,
+        SetValueStatus.NOT_IMPLEMENTED,
+        SetValueStatus.INVALID_VALUE,
+    ],
+    ids=lambda s: s.name,
+)
+async def test_uc_clear_fatal_status_raises_operation_failed(
+    uc_fallback_lock: ZWaveJSLock,
+    mock_uc_utils: dict,
+    status: SetValueStatus,
+) -> None:
+    """Structural-impossibility clear statuses still raise ``LockOperationFailed``."""
+    mock_uc_utils["clear_usercode"].return_value = SetValueResult({"status": status})
+
+    ref = CredentialRef(user_id=3, type=CredentialType.PIN, slot=3)
+    with pytest.raises(LockOperationFailed, match=status.name):
         await uc_fallback_lock.async_delete_credential(ref)