fix: parent/child lock sync resolution

[ps851112]

Summary

Fixes child-lock sync not propagating from the parent lock due to parent relationship resolution/runtime linkage issues.

Root Cause

• CONF_PARENT is stored as the config entry title (e.g. Front Door), but setup logic compared it only with CONF_LOCK_NAME (e.g. front_door), so parent_entry_id could remain unresolved.
• During setup, normalized config data was updated, but runtime lock objects could still be created from stale entry data in the same pass.
• Existing lock objects loaded from storage were not force-refreshed with updated relationship data during setup.
• Relationship rebuild favored name matching and could miss valid links when parent_config_entry_id was already known.

Changes

• Resolve parent by matching CONF_PARENT against entry.title with fallback to CONF_LOCK_NAME.
• Use normalized setup data consistently during runtime lock setup.
• Force coordinator lock refresh with add_lock(..., update=True) so existing locks are updated on startup/reload.
• In relationship rebuild, prioritize explicit parent_config_entry_id before fallback name matching.

Validation

• Reproduced with node8 parent and node9/14/15 children.
• After restart and slot updates, child locks began syncing correctly from parent.

[tykeal]

Thanks for the contribution! The problem you've identified is real — CONF_PARENT stores the entry title but the resolution only compared against CONF_LOCK_NAME, so parent/child relationships could silently fail to resolve. The fix approach is sound.

However, there are several items that need to be addressed before this can be merged:

CI Failures

Lint (ruff format): custom_components/keymaster/__init__.py needs reformatting. The project uses ruff format for code formatting.

Recommendation: Install pre-commit in your local dev environment. The repo has a .pre-commit-config.yaml that will catch formatting and linting issues before they reach CI:

pip install pre-commit
pre-commit install

This will automatically run ruff format, ruff check, and other hooks on every commit so you don't have to manually track these.

Pytest failures: 12 tests are failing with "Lingering timer" errors — likely related to the update=True change on add_lock() causing timer lifecycle issues during setup/reload. See the line comment for details.

Missing Test Coverage

This project enforces 100% patch coverage via Codecov. All new/changed lines must be covered by tests. Currently this PR has no test changes — you'll need to add tests covering:

• Parent resolution matching against entry.title (the new path)
• The setup_data flow ensuring resolved CONF_PARENT_ENTRY_ID is used consistently
• The _rebuild_lock_relationships priority change (explicit ID before name fallback)
• The update=True behavior on add_lock

See tests/test_init.py and tests/test_coordinator.py for existing patterns.

[tykeal]

Line-level comments on specific areas of concern.

[tykeal]

This change from add_lock(kmlock=kmlock) to add_lock(kmlock=kmlock, update=True) makes every setup/reload unconditionally force-update the lock object via _update_lock().

This is the likely cause of the 12 "Lingering timer" test failures in CI — during setup, if a lock already exists in the coordinator (e.g. reload scenario), update=True triggers _update_lock() which may not properly clean up existing timers before replacing the lock's state.

Previously, add_lock without update=True would skip re-adding an already-known lock (unless it was pending_delete). Forcing update=True unconditionally changes the reload behavior for all locks, not just those with parent relationships.

Suggestion: Consider scoping this to only use update=True when the parent relationship has actually changed, or verify that _update_lock() properly handles timer cleanup. The broad unconditional update is risky for locks that don't need it.

[tykeal]

Follow-up: I see you've added test_setup_entry_calls_add_lock_with_update_true which validates the update=True call is made, and the mock_async_call_later fixture change in conftest.py likely resolves the lingering timer test failures.

However, my architectural concern remains unanswered: why does every lock need update=True unconditionally on every setup/reload? The previous behavior only skipped re-adding known locks (unless pending_delete). Now every reload forces _update_lock() on all locks regardless of whether their parent relationship changed.

Could you explain the rationale? Is this needed to handle a specific scenario, or would it be possible to scope this to only locks whose parent relationship has actually changed? Understanding your reasoning here would help move this forward.

[tykeal]

Good improvement — prioritizing an already-resolved parent_config_entry_id before falling back to name matching is the correct order of operations. This avoids redundant lookups and prevents stale name matches from overriding explicit relationships.

One edge case to consider: if parent_config_entry_id points to a config entry that was deleted/unloaded (i.e., it's not in self.kmlocks), the code correctly falls through to the elif branch for name-based matching. Worth adding a test for this scenario.

[tykeal]

Follow-up: Thanks for adding test_rebuild_prefers_explicit_parent_id_over_parent_name — that covers the happy path well and validates the reordering logic.

The edge case I mentioned is still untested though: what happens when parent_config_entry_id is set but points to a config entry that has been deleted or unloaded (i.e., it's not present in self.kmlocks)? The code should fall through to name-based matching in that scenario. A test for this would give confidence that the fallback works correctly.

Could you add that test case, or explain why you think it's unnecessary?

[tykeal]

Nit: setup_data = updated_config creates an alias to the same dict object, not a copy. After async_update_entry is called above, config_entry.data already contains the updated values, so config_entry.data and updated_config point to the same data.

This means all the config_entry.data → setup_data substitutions below are functionally no-ops — they read the same values either way. The variable name setup_data implies it might be a separate snapshot, but it isn't.

This isn't a bug, but it adds 20+ line changes that don't change behavior, which makes the diff harder to review and inflates patch coverage requirements. Consider either:

1. Removing the alias and keeping config_entry.data (fewer changes, same behavior), or
2. If the intent is defensive (guard against async_update_entry not updating in-place in the future), add a comment explaining why

[tykeal]

Follow-up: This is still unaddressed. The setup_data = updated_config alias adds ~20 line changes with no behavioral change since async_update_entry updates config_entry.data in-place.

I'd like to understand the intent here — is this a defensive pattern against future HA API changes, a readability preference, or something else? If there's a good reason, a brief comment in the code explaining why would be helpful. Otherwise, removing it would simplify the diff significantly.

Please respond so we can resolve this thread one way or the other.

[tykeal]

@ps851112 Please see my comments above. I would strongly encourage you to rebase onto the latest main to pick up several linting fixes that have recently been merged.

[tykeal]

CI Status Update

CI has run and Prek (pre-commit) is failing with a mypy error:

tests/test_init.py:233: error: Item "None" of "Any | _Call | None" has no attribute "kwargs"  [union-attr]

This blocks Pytest and Coverage from running, so we can't verify the tests pass yet.

It looks like commit 60398a15 ("test: fix mypy await_args type guard") was intended to fix this, but the type narrowing isn't sufficient for mypy. You'll likely need a more explicit type guard — for example:

assert call_args is not None

or use isinstance / explicit None check before accessing .kwargs.

Recommendation: Install prek or pre-commit locally so you can catch these issues before pushing. The project's .pre-commit-config.yaml has all the hooks configured.

---

Additionally, all 3 review threads from the previous review are still unresolved with no responses. I've posted follow-up comments on each. Even if you believe the code changes address the concerns, please respond to the threads so we can have a productive discussion and move toward resolution. Thanks!