From 90c19443a8eaf09e2fa89ad4891f32b0e54ca782 Mon Sep 17 00:00:00 2001 From: ygelfand Date: Mon, 3 Aug 2026 08:35:19 -0400 Subject: [PATCH 1/5] Fix: don't orphan an autolock fire re-armed during cancel --- custom_components/keymaster/autolock/timer.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/custom_components/keymaster/autolock/timer.py b/custom_components/keymaster/autolock/timer.py index 11037c34..d6c90a39 100644 --- a/custom_components/keymaster/autolock/timer.py +++ b/custom_components/keymaster/autolock/timer.py @@ -131,7 +131,11 @@ async def start(self, duration: int) -> None: async def cancel(self) -> None: """Cancel the timer. Idempotent. Awaits in-flight callback.""" if self._scheduled is not None: - await self._scheduled.cancel() + scheduled = self._scheduled + await scheduled.cancel() + if self._scheduled is not scheduled: + # A start() interleaved with the await and owns the timer now. + return self._scheduled = None if self._state == TimerState.ACTIVE: self._entry = None From 92cd5fe370084920beef2addf14ecd17b9b22608 Mon Sep 17 00:00:00 2001 From: ygelfand Date: Mon, 3 Aug 2026 08:36:41 -0400 Subject: [PATCH 2/5] Test: cancel racing start must not orphan the new fire --- tests/autolock/test_timer.py | 46 ++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/autolock/test_timer.py b/tests/autolock/test_timer.py index 09a20951..81514551 100644 --- a/tests/autolock/test_timer.py +++ b/tests/autolock/test_timer.py @@ -303,6 +303,52 @@ async def test_fire_closure_bails_when_entry_cleared_race(hass, store, kmlock): await cleanup() +async def test_cancel_does_not_orphan_fire_armed_during_await(hass, store, kmlock): + """A start() that interleaves with cancel() keeps its armed fire. + + cancel() awaits the in-flight ScheduledFire. If a start() installs a + replacement during that await, clearing `_scheduled` unconditionally + drops the new fire without cancelling it, and the entry cancel() then + clears belongs to the timer that fire was meant to run — so it wakes + up, finds no entry, bails, and the lock never engages. + """ + timer, action, _, cleanup = make_timer(hass, store, kmlock=kmlock) + await timer.recover() + await timer.start(duration=300) + + first = timer._scheduled + assert first is not None + original_cancel = first.cancel + gate = asyncio.Event() + calls: list[int] = [] + + async def gated_cancel() -> None: + """Hold only the first cancel() inside its await.""" + calls.append(1) + if len(calls) == 1: + await gate.wait() + await original_cancel() + + first.cancel = gated_cancel # type: ignore[method-assign] + + cancel_task = asyncio.create_task(timer.cancel()) + await asyncio.sleep(0) # let cancel() reach the await + + await timer.start(duration=600) # re-arm while cancel() is suspended + second = timer._scheduled + + gate.set() + await cancel_task + + assert timer._scheduled is second + assert timer.state == TimerState.ACTIVE + assert timer.is_running + assert await store.read("t1") is not None + assert action.await_count == 0 + + await cleanup() + + async def test_action_failure_preserves_entry_for_replay(hass, store, kmlock): """Preserve store entry on action failure for replay on next restart. From 09e5493632081e3268bcc523f971070e236ff0b2 Mon Sep 17 00:00:00 2001 From: ygelfand Date: Mon, 3 Aug 2026 08:39:00 -0400 Subject: [PATCH 3/5] Fix: arm autolock for locks already unlocked at startup --- custom_components/keymaster/coordinator.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/custom_components/keymaster/coordinator.py b/custom_components/keymaster/coordinator.py index 7d4479b9..eadcb0c9 100644 --- a/custom_components/keymaster/coordinator.py +++ b/custom_components/keymaster/coordinator.py @@ -1662,6 +1662,14 @@ def get_kmlock(eid: str = entry_id) -> KeymasterLock | None: action=self._timer_triggered, ) await kmlock.autolock_timer.recover() + if ( + kmlock.lock_state == LockState.UNLOCKED + and kmlock.autolock_enabled + and not kmlock.autolock_timer.is_running + ): + # Already unlocked at startup: no unlocked transition will arrive + # to arm the timer, so arm it from the state we adopted. + await kmlock.autolock_timer.start(duration=self.autolock_duration_seconds(kmlock)) if kmlock.autolock_timer.is_running: self.async_schedule_keymaster_notifications([kmlock.keymaster_config_entry_id]) From 3b66590f96a5357b5d556d3248f34979f243ac28 Mon Sep 17 00:00:00 2001 From: ygelfand Date: Mon, 3 Aug 2026 08:54:33 -0400 Subject: [PATCH 4/5] Test: arm autolock for locks already unlocked at startup --- tests/test_coordinator_lifecycle.py | 82 +++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/test_coordinator_lifecycle.py b/tests/test_coordinator_lifecycle.py index e1690551..d8551182 100644 --- a/tests/test_coordinator_lifecycle.py +++ b/tests/test_coordinator_lifecycle.py @@ -10,6 +10,7 @@ import pytest from pytest_homeassistant_custom_component.common import MockConfigEntry +from custom_components.keymaster.autolock.store import TimerEntry from custom_components.keymaster.const import ( CONF_ADVANCED_DATE_RANGE, CONF_ADVANCED_DAY_OF_WEEK, @@ -25,10 +26,12 @@ from custom_components.keymaster.coordinator import KeymasterCoordinator, KeymasterLockCoordinator from custom_components.keymaster.lock import KeymasterCodeSlot, KeymasterLock from custom_components.keymaster.providers._base import BaseLockProvider, CodeSlot +from homeassistant.components.lock.const import LockState from homeassistant.config_entries import ConfigEntryState from homeassistant.const import EVENT_HOMEASSISTANT_STOP, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr, entity_registry as er +from homeassistant.util import dt as dt_util _LOGGER = logging.getLogger(__name__) @@ -2221,3 +2224,82 @@ async def test_update_lock_rebuilds_relationships_when_parent_changes(hass): coordinator._rebuild_lock_relationships.assert_awaited_once() await coordinator.async_shutdown() + + +def _autolock_kmlock( + *, + lock_state: str, + autolock_enabled: bool, + entry_id: str = "entry_1", +) -> KeymasterLock: + """Build a kmlock with equal day/night autolock so sun position is moot.""" + return KeymasterLock( + lock_name="test_lock", + lock_entity_id="lock.test", + keymaster_config_entry_id=entry_id, + lock_state=lock_state, + autolock_enabled=autolock_enabled, + autolock_min_day=5, + autolock_min_night=5, + ) + + +async def test_setup_timer_arms_lock_already_unlocked_at_startup(hass): + """Arm autolock for a lock that was already unlocked when HA started. + + A door open before startup never produces an unlocked transition, so + without arming from the adopted state nothing schedules the autolock + and the lock stays unlocked indefinitely. + """ + coordinator = KeymasterCoordinator(hass) + kmlock = _autolock_kmlock(lock_state=LockState.UNLOCKED, autolock_enabled=True) + + await coordinator._setup_timer(kmlock) + + assert kmlock.autolock_timer is not None + assert kmlock.autolock_timer.is_running + assert kmlock.autolock_timer.duration == 300 + await kmlock.autolock_timer.cancel() + + +async def test_setup_timer_does_not_arm_when_autolock_disabled(hass): + """A lock with autolock off is left alone, unlocked or not.""" + coordinator = KeymasterCoordinator(hass) + kmlock = _autolock_kmlock(lock_state=LockState.UNLOCKED, autolock_enabled=False) + + await coordinator._setup_timer(kmlock) + + assert kmlock.autolock_timer is not None + assert not kmlock.autolock_timer.is_running + + +async def test_setup_timer_does_not_arm_locked_lock(hass): + """A lock that is locked at startup gets no timer.""" + coordinator = KeymasterCoordinator(hass) + kmlock = _autolock_kmlock(lock_state=LockState.LOCKED, autolock_enabled=True) + + await coordinator._setup_timer(kmlock) + + assert kmlock.autolock_timer is not None + assert not kmlock.autolock_timer.is_running + + +async def test_setup_timer_keeps_recovered_timer_over_startup_arm(hass): + """A timer restored from the store wins over the startup arm. + + Otherwise a restart mid-countdown would restart the clock instead of + honoring the remaining time. + """ + coordinator = KeymasterCoordinator(hass) + await coordinator._timer_store.write( + "entry_1_autolock", + TimerEntry(end_time=dt_util.utcnow() + timedelta(seconds=900), duration=900), + ) + kmlock = _autolock_kmlock(lock_state=LockState.UNLOCKED, autolock_enabled=True) + + await coordinator._setup_timer(kmlock) + + assert kmlock.autolock_timer is not None + assert kmlock.autolock_timer.is_running + assert kmlock.autolock_timer.duration == 900 + await kmlock.autolock_timer.cancel() From 00bcff807fe22ab16453eb76130160d0f5891ad8 Mon Sep 17 00:00:00 2001 From: ygelfand Date: Mon, 3 Aug 2026 09:09:51 -0400 Subject: [PATCH 5/5] Test: drop unused type: ignore --- tests/autolock/test_timer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/autolock/test_timer.py b/tests/autolock/test_timer.py index 81514551..0f087ab7 100644 --- a/tests/autolock/test_timer.py +++ b/tests/autolock/test_timer.py @@ -329,7 +329,7 @@ async def gated_cancel() -> None: await gate.wait() await original_cancel() - first.cancel = gated_cancel # type: ignore[method-assign] + first.cancel = gated_cancel cancel_task = asyncio.create_task(timer.cancel()) await asyncio.sleep(0) # let cancel() reach the await