Fix: two autolock paths that could leave a door unlocked indefinitely

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

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])
 

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
+
+    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.
 

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()