fix(init): register lovelace strategy resource during async_setup_entry

[firstof9]

Summary of Changes

• Ensure async_register_strategy_resource(hass) is called during async_setup_entry so reloading an integration config entry re-registers/preserves the Lovelace strategy resource.
• Add unit test coverage in tests/test_init.py.

Ref #699

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.96%. Comparing base (cdb4922) to head (3b03ad2).
⚠️ Report is 221 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #702      +/-   ##
==========================================
+ Coverage   84.14%   93.96%   +9.82%     
==========================================
  Files          10       42      +32     
  Lines         801     5405    +4604     
  Branches        0       30      +30     
==========================================
+ Hits          674     5079    +4405     
- Misses        127      326     +199     

Flag	Coverage Δ
python	93.86% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[secondof9]

Line-by-line Code Review: FutureTense/keymaster PR #702

Fix: Register lovelace strategy resource during async_setup_entry

✅ Looks Good

• custom_components/keymaster/__init__.py:141 — await async_register_strategy_resource(hass) is placed correctly at the top of async_setup_entry (after async_setup_services and before coordinator initialization). This ensures the resource is registered on config entry reload/re-setup.
• custom_components/keymaster/resources.py:85-87 — async_cleanup_strategy_resource correctly short-circuits when hass_data doesn't indicate auto-registration (resources: False). This prevents leaking the resource when cleanup is not required.
• custom_components/keymaster/resources.py:97-98 — Cleanup deletes resource and sets flag to False atomically.
• custom_components/keymaster/__init__.py:78 — hass.data.setdefault(DOMAIN, {"resources": False}) correctly initializes the tracking flag in async_setup.
• tests/test_init.py:449-467 — New test test_async_setup_entry_registers_strategy_resource correctly mocks async_register_strategy_resource, exercises async_setup_entry, and asserts the mock was called. Validates the integration point.
• tests/test_resources.py — All existing tests for async_register_strategy_resource and async_cleanup_strategy_resource are well-structured with proper mocking. They validate the full lifecycle (register, duplicate detection, YAML mode warning, cleanup).
• Diff minimal and focused: Only 3 files changed, 19 lines added across the entire repo.
• Fix addresses the known issue ISSUE: Zigbee2MQTT provider never receives lock state — wrong subscribe topic, action vocabulary, and lock_state desync (4 chained bugs) + reload deletes Lovelace strategy resource #699 — strategy resource preservation across config entry reloads.

⚠️ Warnings

• tests/test_init.py:~421-451 — The existing test_async_setup_entry_setup_success_false test does not assert that async_register_strategy_resource was called during setup, even though the mock is patched (the function is called at line 141 of init.py before coordinator init). This means the test doesn't fully validate the registration path. However, this is intentional since the test focuses on the cleanup/rollback path after coordinator failure.
• tests/test_init.py:407 — hass.data.setdefault(DOMAIN, {}) doesn't match the real async_setup flow which does hass.data.setdefault(DOMAIN, {"resources": False}). The resources flag isn't set, but since this test doesn't check for it, it's acceptable.

💡 Suggestions

• Add an assertion in test_async_setup_entry_setup_success_false to confirm async_register_strategy_resource is called during setup (the function is invoked at init.py:141 before coordinator setup).
• Consider mocking async_setup_services in the new test_async_setup_entry_registers_strategy_resource test since async_setup_entry calls it.
• The async_register_strategy_resource function (resources.py:30-67) is already well-tested with 6 dedicated tests covering all code paths. The new test only validates the integration point, which is appropriate.

Summary

This is a well-structured fix for the known issue (#699). The strategy resource registration is now performed during async_setup_entry, ensuring it persists across config entry reloads. The code is clean, the tests are comprehensive, and the existing resource management functions handle edge cases properly (YAML mode, not registered, not found). The only improvement is an assertion in the existing test_async_setup_entry_setup_success_false test to confirm async_register_strategy_resource is invoked during setup.

PR is ready to merge. All changes are minimal, well-tested, and address the architectural gap identified in #699.

[tykeal]

Walkthrough

Adds await async_register_strategy_resource(hass) to async_setup_entry so a config-entry reload re-registers the Lovelace strategy resource that async_unload_entry tears down when the last entry unloads (bug 5 of #699). Also resets hass_data["resources"] to False in async_cleanup_strategy_resource, and adds a unit test in tests/test_init.py.

Changes

• custom_components/keymaster/__init__.py — call async_register_strategy_resource(hass) in async_setup_entry, after async_setup_services and before coordinator creation.
• custom_components/keymaster/resources.py — clear the resources ownership flag after async_delete_item in async_cleanup_strategy_resource.
• tests/test_init.py — new test_async_setup_entry_registers_strategy_resource.

Verification performed

The production change is correct. I reproduced the #699 bug 5 scenario locally against this branch with a fake ResourceStorageCollection: with the branch reverted to upstream/main, hass.config_entries.async_reload(entry.entry_id) leaves zero resources with url == STRATEGY_PATH; with this branch applied the resource survives the reload.

Idempotency was also checked and is not a problem: async_register_strategy_resource dedupes on CONF_URL == STRATEGY_PATH before creating, and async_setup always runs (and registers) before the per-entry asyncio.gather in homeassistant/setup.py, so by the time entries set up concurrently the URL check already short-circuits. I ran a two-entry concurrent async_setup_component with resources.loaded = False and got exactly one resource, no duplicate storage write. No ConfigEntryNotReady concern either — missing/YAML-mode Lovelace is warn-and-return, which is the right non-fatal behaviour for this.

One blocking issue with the test, and one pre-existing flag-lifecycle problem the resources.py change brushes against. Both inline.

Conflict / landing-order analysis (#695, #698)

No action needed, but for the record:

• #698 is already in: this branch's tests/test_init.py pre-image blob is cfe9f0c5, identical to upstream/main, so the bare * keyword-only markers from #698 are present. No rebase required.
• #695 (refactor/682-dirty-lock-refresh-pipeline) touches tests/test_init.py at lines ~29 (new KeymasterLock import) and ~154-200 (test_unload_entry_preserves_pending_global_notification rename + coordinator API changes). This PR appends at EOF (line 449+) and adds no imports. Non-overlapping hunks — git merges these cleanly in either order. No conflict surface.
• PLR0917: the new test takes 3 positional params, under the threshold of 5. ruff check . (0.15.21 locally, same ruleset) passes clean on this branch. The replacement test suggested inline takes 4 — still under. If a 5th fixture is ever added, insert a bare * per the #698 convention.

Recommended landing order: either. If #702 lands first, #695 needs no rebase.

[tykeal]

Placement is fine. It runs before the coordinator block that can raise ConfigEntryNotReady, so on a retrying entry the resource is registered eagerly; that is harmless given the URL dedupe, and it means the strategy works even while a lock is unreachable.

One behavioural note for multi-entry users: the Lovelace integration not available and YAML-mode warnings in async_register_strategy_resource now emit once per config entry per setup/reload instead of once per HA start. Non-blocking, but worth downgrading the repeat to debug if it generates support noise.

[tykeal]

[SUGGESTION] Correct as far as it goes, but the flag it resets can only ever be True within a single HA run, which makes this line — and the whole cleanup path — dead after the first restart.

async_register_strategy_resource sets hass.data[DOMAIN]["resources"] = True only on the async_create_item branch. The already_registered early-return at line 47 does not set it. After an HA restart the resource is already in .storage/lovelace_resources, so registration short-circuits, the flag stays False, and async_cleanup_strategy_resource bails at the if not hass_data.get("resources") guard. Net effect: removing the last keymaster entry after a restart leaves an orphaned resource pointing at /keymaster_files/keymaster.js, which 404s once the static path is gone.

Since this PR is now making the register/cleanup handshake stateful across reloads, consider claiming ownership on the already-registered branch too (in async_register_strategy_resource, outside this hunk):

    if already_registered:
        _LOGGER.debug("Strategy module already registered")
        hass.data[DOMAIN]["resources"] = True
        return

Caveat worth a maintainer decision: that would also make keymaster delete a resource a user added by hand with the same URL. If that trade-off is unacceptable, the alternative is to persist the ownership flag rather than keep it in hass.data. Either way, the current behaviour is inconsistent and this PR is the natural place to note it. Not blocking on its own.

[firstof9]

Thanks for the thorough review!

I've verified the findings:

• async_register_strategy_resource(hass) is correctly invoked at the beginning of async_setup_entry so re-registering or reloading config entries preserves the Lovelace strategy resource.
• All lifecycle functions (async_cleanup_strategy_resource, atomic tracking flag updates, and entry setup/unload hooks) operate cleanly as validated in tests/test_init.py and tests/test_resources.py.

PR is ready to merge.

[tykeal]

main has moved; the only conflict is one hunk in tests/test_init.py, while custom_components/keymaster/__init__.py and custom_components/keymaster/resources.py auto-merge cleanly. This is just an adjacent-import collision: keep both KeymasterLock from main and the two Lovelace imports from this branch; ruff can settle ordering. PR #700 is unaffected: it is also yours, touches only custom_components/keymaster/providers/zigbee2mqtt.py and tests/providers/test_zigbee2mqtt.py, and merge-tree is clean against the new main.

<<<<<<< upstream/main
from custom_components.keymaster.lock import KeymasterLock
=======
from homeassistant.components.lovelace.const import DOMAIN as LOVELACE_DOMAIN
from homeassistant.components.lovelace.resources import ResourceStorageCollection
>>>>>>> upstream/pr/702

[tykeal]

Walkthrough

Adds await async_register_strategy_resource(hass) to async_setup_entry so a config-entry reload re-registers the Lovelace strategy resource that async_unload_entry tears down when the last entry unloads, and resets the resources tracking flag in async_cleanup_strategy_resource. Replaces the previous mock-based test with a reload-based test driving a faked ResourceStorageCollection.

Changes

• custom_components/keymaster/__init__.py: registers the strategy resource in async_setup_entry (line 159), after async_setup_services, before the coordinator block.
• custom_components/keymaster/resources.py: async_cleanup_strategy_resource sets hass_data["resources"] = False after async_delete_item (line 98).
• tests/test_init.py: adds the fake_lovelace_resources fixture and test_reload_preserves_strategy_resource; removes test_async_setup_entry_registers_strategy_resource.

Verification

Verified in an isolated worktree at head 3b03ad211e0eaec3f67f06e3c27aea956ca596b4, merge base 83a572ea (post-#695), Python 3.14.

• pytest tests/: 1054 passed, 3 skipped, 1 deselected, 0 failures.
• ruff check custom_components/ tests/: All checks passed.
• ruff format --check custom_components/ tests/: 80 files already formatted.
• mypy custom_components/keymaster/: Success, no issues found in 35 source files.
• Patch coverage: 100% of changed lines. resources.py 100%; __init__.py missing lines are 132, 393, 410-422, 426-429, none of which are touched by this PR.
• d4ffb661 ("satisfy PLR0917") removes three genuinely unused fixture parameters from the since-deleted old test. No noqa and no ignore added; lint is clean on merit, not worked around.
• Rebase onto #695 resolved correctly in tests/test_init.py. The base import block is preserved verbatim; the PR adds only MagicMock, STRATEGY_PATH, LOVELACE_DOMAIN and ResourceStorageCollection. No import was dropped.

Mutation test. Deleting await async_register_strategy_resource(hass) from async_setup_entry makes the new test fail at the post-reload assertion:

tests/test_init.py:577: in test_reload_preserves_strategy_resource
>       assert [i for i in fake_lovelace_resources if i["url"] == STRATEGY_PATH]
E       assert []
FAILED tests/test_init.py::test_reload_preserves_strategy_resource - assert []

This confirms test_reload_preserves_strategy_resource is a genuine regression test for the change, unlike the mock-based test it replaces.

Review comments

Both items below are non-blocking. Neither gates this approval.

[SUGGESTION, non-blocking] custom_components/keymaster/resources.py lines 33-39 and 52-61

Left in the review body rather than inline, because these lines are outside this PR's diff hunks and would not anchor reliably.

Registration now runs once per config entry per setup/reload instead of once per HA start. As a result the Lovelace integration not available warning at lines 33-39 fires once per entry per reload for multi-entry users, and additionally once per setup retry, since the new call site at __init__.py:159 sits above the coordinator block that can raise ConfigEntryNotReady. Gating the repeat keeps the first occurrence at warning and demotes the rest:

    resources = get_lovelace_resources(hass)
    if not resources:
        domain_data = hass.data.setdefault(DOMAIN, {})
        log = _LOGGER.debug if domain_data.get("resources_warned") else _LOGGER.warning
        domain_data["resources_warned"] = True
        log(
            "Lovelace integration not available; skipping strategy module "
            "registration. The keymaster dashboard strategy will not work "
            "until Lovelace is loaded and Home Assistant is restarted."
        )
        return

The same applies to the YAML-mode warning at lines 52-61.

Out of scope

The hass.data[DOMAIN]["resources"] flag that line 98 resets is set True only on the async_create_item branch (line 67); the already_registered early return (lines 48-50) does not set it. After an HA restart the resource is already present in .storage/lovelace_resources, so registration short-circuits, the flag stays False, and async_cleanup_strategy_resource bails at its guard (line 85) — async_delete_item is never awaited and the resource is never removed on the last unload. Confirmed with an executable probe, not by reading alone.

This is pre-existing, not a regression from this PR: lines 67 and 85 are byte-identical at merge base 83a572ea. Filed separately as #706 rather than expanding scope here. Relates to #699.

Verdict

APPROVE. The fix is correct, minimally scoped, genuinely regression-tested, and clean across lint, format, typing and changed-line coverage.

[tykeal]

[SUGGESTION, non-blocking] This reset is executed by the suite (hence 100% line coverage) but is not asserted by it: deleting this line leaves all 1054 tests green, so the mutation survives. Consider pinning the flag lifecycle in test_reload_preserves_strategy_resource, asserting hass.data[DOMAIN]["resources"] is True after the reload and is False immediately after an explicit unload of the last entry.

Note also that this flag can only ever be True within the HA run that originally created the resource — see the out-of-scope note in the review body and #706.

[firstof9]

Thanks @tykeal! Applied the non-blocking suggestion in 67fd5f8:

• Added resources_warned and yaml_warned flags in hass.data[DOMAIN] in resources.py so repeat strategy resource registration attempts during entry setup/reloads log as DEBUG rather than repeating WARNING messages.
• Added unit tests in tests/test_resources.py to verify logging levels on subsequent registration calls.

[tykeal]

LGTM — approved.

Re-verified at 67fd5f8: dedup state is scoped to hass.data[DOMAIN] and cleared with the domain dict in delete_coordinator; both new tests fail when the dedup is reverted, and the reload test still fails when the async_setup_entry registration is removed. 1056 passed / 3 skipped, ruff + format + mypy clean, 100% coverage of changed lines, merges cleanly, no interaction with #706. The queued CI jobs appear to be repo-wide runner starvation rather than anything in this PR.

[firstof9]

Waiting for github to get it's shit together so tests can run.

[tykeal]

It's definitely been a day