Conversation
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* origin/main: feat: add_user and delete_user take lists (#1509) # Conflicts: # custom_components/lock_code_manager/__init__.py
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
python
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1514 +/- ##
=======================================
Coverage 99.22% 99.22%
=======================================
Files 66 66
Lines 8739 8814 +75
Branches 522 524 +2
=======================================
+ Hits 8671 8746 +75
Misses 68 68
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
* origin/main: fix: editing an unmanaged code left every PIN on the card in cleartext (#1519) build(deps-dev): bump aioesphomeapi in the homeassistant group (#1512) build(deps-dev): bump @types/node from 26.2.0 to 26.3.0 (#1513) # Conflicts: # custom_components/lock_code_manager/www/generated/lock-code-manager.js
* origin/main: fix: a deleted user no longer leaves a ghost card behind (#1524) # Conflicts: # custom_components/lock_code_manager/www/generated/lock-code-manager.js
#1524 fixed the subscription on main, where a user is removed through the delete_user action. v6 adds a second route with no LCM code in it at all -- Home Assistant's native delete on the entry's page, which is where the add-user card's 'Manage users in settings' link sends people. Same guard covers it, because it is re-checked on every update rather than keyed on how the removal happened. This pins that. Entire-Checkpoint: 6c08a5ce1ddb
* origin/main: fix: a stalled provider operation is reported instead of staying silent (#1525) # Conflicts: # tests/providers/test_base.py
Review of #1514. One service call is several entry writes -- a subentry per user, then the entry -- and Home Assistant schedules an update listener task per write. They overlap. Only the earliest task finds a diff, because it caches the new configuration before its first await; the rest see the cached view, return immediately, and set `settled` on the way out. So the waiter in _async_write_and_settle was released by a pass that did nothing while the one building the entities was still going -- the exact return-before-ready its docstring exists to prevent. Count passes in flight and settle only when the count returns to zero. The downstream symptom is a race a test cannot pin: which pass wins depends on scheduling, and the assertion passes either way. Verified the overlap with a trace (enter 1, enter 2, exit 2, exit 1) and tested the guarantee directly: with a slow pass and a fast one overlapping, settled stays clear until the slow one finishes. Both mutants that break the count are killed by that test. Entire-Checkpoint: e28f6f376382
Review of #1514. EntryConfig.from_entry took users from subentries only. Migration runs when an entry is set up, so an entry that is disabled -- or has not loaded yet -- keeps its pre-v5 shape, with users and their numbers still on the entry and no subentries at all. Read that way it holds nothing, and it is still asked: get_managed_slots consults every entry on a lock for the numbers it holds, so a new entry could be issued the same numbers the disabled one already has, and the unmanaged-code sweep would raise a fixable "delete this code" repair for each of them. Fall back to reading the entry mapping, through the same converter the migration uses, so the two cannot disagree about what a slot-keyed entry means. Told apart by shape rather than entry.version. Shape is what this reads and the version is a proxy for it -- one the test fixtures do not keep, since they build subentries onto entries left at version 1 (see user_subentries in tests/common.py, which says as much). The shape is unambiguous because migration strips the legacy keys as it moves users out: an entry with none of them has nothing to fall back to, and an entry that still has them has not been migrated. Entire-Checkpoint: 2a25d0d52b56
Review of #1514. When users moved off the options form into their own subentries, the form lost the capacity check along with them. It kept the unclaimed-MQTT check and nothing else, so a lock that cannot hold the entry's existing slot numbers was accepted here and refused later, at write time, one slot at a time, as a connectivity warning. The users are not on this form, but the numbers they hold are exactly what a lock added here has to be able to hold. Reauth already checks this when it swaps a lock in; the options flow now does the same for the locks it is given, and reports the refusal the same way. Entire-Checkpoint: 90f73c003cbe
|
Ran a full code review of this branch and pushed fixes for the three findings, each as its own commit with a test that fails without it and passes with it (mutation-checked, not just covered).
|
Fifteen commits from main, including the deadline, queue and allocation trio (#1551, #1553, #1554), the coordinator-owned pending writes (#1545, #1547, #1550) and the move to probatio (#1540). Where the two sides met in the flows, the allocation progress step from #1554 lands on the setup paths (guided count and YAML block), which still allocate in bulk; the YAML path creates its entry through the subentry helper the way v6 does. The options flow stays locks-only and gets no progress step, since it no longer allocates. The user subentry flow picks up #1554's tenure rule: the users already here keep their numbers and are not read for, so adding one user reads one index per lock. Its refusal for an unforeseen read failure joins the subentry strings, and the contract test knows about it. Tests main wrote against users in the options flow are re-expressed as subentry flow tests; the #1550 zwave_js test builds its entry with subentries; the zwave-js-ui push tests keep v6's direct assertion on the event call and main's observe_push. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 0435effbae00
Adding a user opened straight onto a single-user form, so putting five people on an entry meant five dialogs, five lock reads and five entry settles. Setup has always taken a pasted block; after it, the only bulk route left was the add_users action, which means writing a script. "Add user" now opens on a choice. One user is the form that was there. Several takes the same dictionary the setup flow's YAML path takes, validated by the same function, so what a person pastes to create an entry is what they paste to add to it. One allocation covers the block and one write lands it, and the numbers already held are held by tenure, so a block of two on an entry of one asks the lock about two numbers. Adding is all it does. A block naming somebody already here is refused rather than merged into them, by the same identity rule the rest of the integration uses, so Bob and "bob " cannot become one person carrying two credentials. Editing and removing stay one user at a time, which keeps exactly one way to change a user who exists -- the property the subentry move was for. The block is written through async_write_entry_config rather than by completing the flow: a finished subentry flow creates exactly one subentry, and this creates as many as were pasted. The shared block validator now takes the names the submission is being added alongside, empty from setup where the block is the whole configuration, and sizes its capacity check to everyone the entry will hold rather than to the block alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: db2fedcc8879
Breaking change
Four actions removed. They addressed a lock slot or a device instead of a person:
lock_code_manager.set_usercodeset_credentiallock_code_manager.clear_usercodeclear_credentiallock_code_manager.set_slot_conditionset_conditionlock_code_manager.clear_slot_conditionclear_conditionThe
lock_code_manager_lock_state_changedevent is removed. Uselock_code_manager_credential_used, which names the person rather than the slot number they occupy. Field-by-field mapping is on the wiki.Users are now config subentries. A v4 → v5 migration moves them on load. Nothing is renamed and nobody is renumbered, so every entity keeps its ID, its settings and its history. The Configure screen now edits only the locks; each user gets their own add, edit and delete dialog on the entry's page, with their entities and device grouped under them. Add user opens on a choice of one user or a pasted block, so bulk adding survives the move.
The Condition Linker blueprint takes a Name instead of a Slot number. An automation already created from it must be reconfigured, or simply recreated — it is a one-shot.
Proposed change
The through-line is that a person is the unit Lock Code Manager works in, and the slot number is internal bookkeeping. Every surface that leaked that number is gone: the four slot-addressed actions, the lock-shaped event, and the
slot_assignmentside table.Users move into config subentries, so each person's slot number lives in their own record and the configuration can no longer disagree with itself about who holds which.
async_write_entry_configis the single write path, matching users by slot number — not by name, which is what a rename changes.Verified against a real instance
Deployed to a production instance holding two v4 entries and restored from a full backup afterwards. Both migrated to v5, users landed in subentries with their numbers intact, and all 31 entities plus all 3 slot devices came up bound to the right person — no duplicates, no orphans, no errors.
That run caught two things the suite could not, both fixed here:
async_add_entitiesdrag an existing device into a subentry. Every upgrading instance is in that position, since its devices predate subentries; the suite creates devices fresh during setup, already bound, so only a real upgrade reproduces it. The migration now moves each one withasync_update_device.dialog-data-entry-flowships only in lazily-loaded chunks, and no URL starts a subentry flow. The add-user card keeps its own dialog and links to the page that hosts the native ones.Adding users in bulk
Setup has always taken a block of users keyed by name, and it still does; after this branch it creates one subentry per user and everything afterwards is per person. The Add user dialog offers the same block, so adding five people is one dialog rather than five, and the earlier options form that could bulk-add is not missed.
It only adds. A block naming somebody already on the entry is refused rather than merged into them, by the same identity rule the rest of the integration uses, so
Bobandbobcannot become one person holding two credentials. Editing and removing stay one user at a time, which is what keeps exactly one way to change a user who exists.The saving is per lock, not per keystroke: one allocation covers the block and one write lands it, and numbers already held are held by tenure and are not read for. A block of two on an entry of one asks each lock about two numbers, where two separate adds would ask twice and settle the entry twice. The block is written through
async_write_entry_configrather than by completing the flow, because a finished subentry flow creates exactly one subentry and this creates as many as were pasted.Defects this exposed
Five bugs that were latent before this branch:
data, so it saw zero managed slots and would have raised a repair for every one of its own codes on any v3 upgrade.from_entryreads the entry's locks options-first, so writing onlydatawas invisible whenever options were staged — and, showing no diff, never woke the listener that clears them.condition_linkerwent on callingset_slot_conditionwith a green suite, because nothing type-checks a blueprint. There is now a test that every action a blueprint calls exists inservices.yamlwith its required fields supplied.Type of change
Additional information
set_credential/clear_credentialshipped in 5.x; this removes theset_usercode/clear_usercodepair they replace, which is the half that had to wait for a major.lock_code_manager_lock_state_changedevent is gone.prekclean.mainis merged in as of 41ee870, so this carries the deadline, queue and allocation work (fix: waiting for the lock's turn is not the lock failing to answer (#1535) #1551, fix: the operation deadline scales by what the call walks, not by the entry (#1528) #1553, fix: a long lock read shows progress, and numbers held by tenure are not read (#1536) #1554) and the progress step it added to the setup flow.