fix(zwave_js,matter): tolerate ambiguous credential writes; universal masked read projection (#1251, #1257)#1258
Merged
Conversation
… masked read projection Two distinct variants of the "slots disabled after a working write" regression (#1251 working-capability locks, #1257 Matter), unified by one principle: 4.x turned ambiguous/transient write results into permanent slot disables that 3.x tolerated. The correct mechanism differs by whether the write reached the lock. zwave_js (#1251, Schlage BE469 family): - The driver's unified User Code CC setCredential verifies by reading the code back and comparing it to what was written. Locks that report the code masked return ERROR_UNKNOWN for an *accepted* write (userIdStatus -> Enabled). HA surfaces credential_rejected_unknown; LCM disabled the slot. - Treat credential_rejected_unknown as a COMPLETED set (the write landed; the sync manager's last-set tracking + read-back reconcile it), not a rejection. Definitive rejections (duplicate/occupied/manufacturer/validation) unchanged. - _pin_state now maps masked/withheld codes (empty or all-asterisks) to unreadable on the unified path, matching the UC fallback's _uc_slot_state, which now delegates to it. A masked code is no longer mistaken for a readable (wrong) PIN that never reconciles. - Reverted the UC write-routing broadening: with the universal projection + tolerant writes, healthy-capability UC-only locks work on the unified path. The UC fallback is retained only for the zero-slot capability variant. matter (#1257, Aqara U200): - MatterClientException (e.g. InvalidState: Not connected at startup) is independent of HomeAssistantError and escaped to the generic handler, which suspended the slot. Catch it (and MatterError) on set/clear -> LockDisconnected. - An unmapped SetCredential status (unknown(N), seen while a lock is not ready at startup) routes to retry (LockDisconnected), not a permanent disable. Recognized rejections (occupied) still surface as CodeRejectedError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 74f4540b17c1
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1258 +/- ##
==========================================
+ Coverage 97.11% 97.15% +0.03%
==========================================
Files 54 54
Lines 6416 6434 +18
Branches 461 461
==========================================
+ Hits 6231 6251 +20
+ Misses 185 183 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…branches Brings the three touched provider modules to 100% line coverage: - zwave _pin_state masked/withheld -> unreadable projection (parametrized) - zwave credential_rejected_unknown -> completed set (not CodeRejectedError) - _zwave_js_uc _pin_state base stub raises (override contract) - matter unknown(N) status and MatterClientException -> retry, on set and clear - matter sync-duplicate retry: transient -> retry, definitive -> CodeRejectedError - matter async_get_users skips a raw user with no user_index Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 7c8ddfb4056a
…eview Code-review follow-ups on PR #1258: - #1 (bug): the sync-duplicate-retry clear_lock_credential inside async_set_credential still caught only HomeAssistantError, while the three sibling set/clear sites were broadened. A MatterClientException (e.g. InvalidState: Not connected) there escaped to the generic handler and suspended the slot -- the exact #1257 failure. Broaden it to (HomeAssistantError, MatterError, MatterClientException) and add a regression test. - #2: rewrite the async_get_capabilities docstring, which described the reverted UC write-routing broadening. It now matches the code: healthy capabilities -> unified path (masked-code locks kept correct by the read projection + tolerant writes); degenerate caps -> legacy fallback. - #4: document the fragile coupling of _is_transient_credential_status to HA's private unknown(<code>) status format. - #5: dedupe the byte-identical transient-status LockDisconnected raise into a _transient_status_disconnect helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 4653715368a2
This was referenced Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Two distinct variants of the same regression — 4.x permanently disables lock slots on ambiguous/transient credential-write results that 3.x tolerated — unified by one principle, with the correct mechanism differing by whether the write actually reached the lock.
zwave_js (#1251, working-capability variant — e.g. Schlage BE469/BE469ZP)
Separate from the already-fixed zero-slot-capability variant. Here the lock's capabilities are healthy, but the driver's unified User Code CC
setCredentialverifies a write by reading the code back and comparing it to what was written. Locks that report the user code back masked/withheld returnERROR_UNKNOWNfor a write the lock actually accepted (userIdStatus→ Enabled). HA surfacescredential_rejected_unknown; LCM mapped it toCodeRejectedErrorand disabled the slot. Evidence from a reporter's Z-Wave logs: the code lands (userIdStatus[N]: 1) and is removed ~96 ms later when LCM disables+clears it; on 3.2.1 the same codes persist. (Upstream driver bug reported separately; the verify-by-code-equality should be verify-by-status.)credential_rejected_unknownas a completed set (the write landed; the sync manager's last-set tracking + read-back reconcile it) instead of a rejection. Definitive rejections (duplicate / occupied / manufacturer-rules / validation) are unchanged._pin_statenow maps masked/withheld codes (empty or all-asterisks) to unreadable on the unified path, matching the UC fallback's_uc_slot_state(which now delegates to it). A masked code is no longer mistaken for a readable (wrong) PIN that can never reconcile against the configured one.matter (#1257 — e.g. Aqara U200)
The same "don't permanently disable a transient failure" principle, but the failure is "not reached" rather than "reached but unverifiable," so the fix routes to retry, not completed-set (retry converges once Matter is ready; treating a not-reached write as completed would risk a silent no-code lockout).
MatterClientException(e.g.InvalidState: Not connectedduring startup) is independent ofHomeAssistantErrorand was escaping to the generic handler, which suspended the slot. It (andMatterError) are now caught on set/clear →LockDisconnected(retry).unknown(N), observed while a lock isn't ready right after startup) routes to retry (LockDisconnected) rather than a permanent disable. Recognized rejections (occupied) still surface asCodeRejectedError.Not included (deliberately)
A push-as-commit / pending-confirmation model that would also close a narrow, pre-existing (3.x-equivalent) silent-failure window for genuine unsupervised rejections. It interacts with shared
calculate_in_sync/ coordinator-merge semantics that other providers depend on (Schlage's eventual-consistency convergence relies on the empty-branch last-set trust; no-downgrade conflicts with out-of-band change detection), so it needs its own per-provider-aware design and PR. Tracked as a follow-up.Type of change
Additional information
_pin_statemasked-projection parametrization; zwavecredential_rejected_unknown→ completed set; matterunknown(N)andMatterClientException→ retry on set and clear.🤖 Generated with Claude Code