From 4013f9cd9f0ea29d831df5e443140829071a3476 Mon Sep 17 00:00:00 2001 From: tempus2016 Date: Wed, 4 Nov 2026 10:29:10 +0000 Subject: [PATCH] fix(timed): a timer reaching its cap no longer breaks the refresh, and a locked one isn't paid out unchecked --- custom_components/taskmate/coord_timed.py | 27 +++-- tests/test_hardening_2026_10_25_logic.py | 1 + tests/test_timed_cap_refresh.py | 126 ++++++++++++++++++++++ tests/test_timed_task.py | 3 + wiki/Timed-Tasks.md | 3 + 5 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 tests/test_timed_cap_refresh.py diff --git a/custom_components/taskmate/coord_timed.py b/custom_components/taskmate/coord_timed.py index 8c180482..02c5d6be 100644 --- a/custom_components/taskmate/coord_timed.py +++ b/custom_components/taskmate/coord_timed.py @@ -101,8 +101,14 @@ async def async_pause_timed_task(self, chore_id: str, child_id: str) -> None: await self.storage.async_save() await self.async_refresh() - async def async_stop_timed_task(self, chore_id: str, child_id: str) -> None: - """Stop a timed task session and create a completion.""" + async def async_stop_timed_task(self, chore_id: str, child_id: str, *, refresh: bool = True) -> None: + """Stop a timed task session and create a completion. + + ``refresh=False`` is for callers already inside the coordinator's + refresh (the cap auto-stop in _async_update_data): refreshing from + there re-enters the refresh debouncer, which raises (#771), and the + running update picks the change up anyway. + """ session = self.storage.get_active_timed_session(chore_id, child_id) if not session: raise ValueError("No active timer to stop") @@ -138,6 +144,12 @@ async def async_stop_timed_task(self, chore_id: str, child_id: str) -> None: # A grown-up's own session never waits for approval (#1061). needs_approval = chore.requires_approval and not child.is_adult + # A chore the child can't do right now (a prerequisite added after the + # timer started, a cooldown, a rota or vacation change…) isn't paid out + # automatically: completing it would be refused, so the time goes to a + # parent to approve instead of being lost or paid unchecked (#771). + if not self._is_chore_completable_by_child(chore, child_id): + needs_approval = True completion = ChoreCompletion( chore_id=chore_id, child_id=child_id, @@ -176,7 +188,8 @@ async def async_stop_timed_task(self, chore_id: str, child_id: str) -> None: completion_id=completion.id, ) - await self.async_refresh() + if refresh: + await self.async_refresh() def _timed_start_allowed(self, chore, child_id: str) -> bool: """Assignment/enabled/schedule eligibility for starting a timed task. @@ -247,7 +260,7 @@ def _calc_session_seconds(self, session: TimedSession) -> int: continue return total - async def _async_stop_stale_timed_sessions(self) -> None: + async def _async_stop_stale_timed_sessions(self, refresh: bool = True) -> None: """Auto-stop any timed sessions from a previous day (midnight cleanup).""" today = dt_util.as_local(dt_util.now()).date().isoformat() sessions = self.storage.get_timed_sessions() @@ -295,7 +308,8 @@ async def _async_stop_stale_timed_sessions(self) -> None: if stale: await self.storage.async_save() - await self.async_refresh() + if refresh: + await self.async_refresh() async def _async_auto_stop_capped_sessions(self) -> None: """Check running sessions against daily cap and auto-stop if exceeded.""" @@ -308,4 +322,5 @@ async def _async_auto_stop_capped_sessions(self) -> None: continue elapsed = self._calc_session_seconds(session) if elapsed >= chore.timed_max_daily_minutes * 60: - await self.async_stop_timed_task(session.chore_id, session.child_id) + # Called from _async_update_data: no nested refresh (#771). + await self.async_stop_timed_task(session.chore_id, session.child_id, refresh=False) diff --git a/tests/test_hardening_2026_10_25_logic.py b/tests/test_hardening_2026_10_25_logic.py index 30865988..b31236f9 100644 --- a/tests/test_hardening_2026_10_25_logic.py +++ b/tests/test_hardening_2026_10_25_logic.py @@ -325,6 +325,7 @@ def test_two_stops_landing_together_pay_once(self): storage.async_save = AsyncMock() coord.storage = storage coord.async_refresh = AsyncMock() + coord._is_chore_completable_by_child = lambda _chore, _child_id: True # #771 availability gate coord._async_notify_pending_approval = AsyncMock() async def slow_award(child, pts, **_kw): diff --git a/tests/test_timed_cap_refresh.py b/tests/test_timed_cap_refresh.py new file mode 100644 index 00000000..e22c2434 --- /dev/null +++ b/tests/test_timed_cap_refresh.py @@ -0,0 +1,126 @@ +"""A timer reaching its daily cap mid-refresh, and a locked timer at its cap (#771). + +1. The cap auto-stop runs inside the coordinator's own refresh + (_async_update_data). It stopped the session through async_stop_timed_task, + which ended with ``await self.async_refresh()`` — re-entering the refresh + debouncer, which raises "Debouncer lock is not re-entrant" in Home + Assistant and broke the refresh. The stop now takes ``refresh=False`` from + there; the running update picks the change up in the same tick. + +2. A running timer whose chore the child can no longer do (a prerequisite + added after it started, say) still auto-completed at its cap and paid out. + Completing that chore is refused, so the stopped time now goes to a parent + to approve instead of being paid automatically. +""" + +from __future__ import annotations + +import datetime as dt +from unittest.mock import patch + +from custom_components.taskmate.models import TimedSession + +from .test_completion_eligibility import _make_system, _now, run + +TIMED_NOW = "custom_components.taskmate.coord_timed.dt_util.now" + + +class ReentrantRefresh(RuntimeError): + pass + + +def _system(*, requires_approval=False, cap=10): + coord, storage, mod = _make_system() + with patch.object(mod.dt_util, "now", return_value=_now()): + kid = run(coord.async_add_child("Vaiha")) + chore = run( + coord.async_add_chore( + "Reading", points=0, requires_approval=requires_approval, schedule_mode="specific_days" + ) + ) + chore.task_type = "timed" + chore.timed_rate_points = 2 + chore.timed_rate_minutes = 5 + chore.timed_max_daily_minutes = cap + storage.update_chore(chore) + started = _now() - dt.timedelta(minutes=cap + 1) + storage.save_timed_session( + TimedSession( + chore_id=chore.id, + child_id=kid.id, + state="running", + segments=[{"start": started.isoformat(), "end": None}], + session_date=_now().date().isoformat(), + ) + ) + return coord, storage, mod, kid, chore + + +def _reentrancy_guard(coord): + """Fail the way HA's debouncer does when the refresh is re-entered.""" + + async def refresh(): + raise ReentrantRefresh("Debouncer lock is not re-entrant") + + coord.async_refresh = refresh + + +def _update(coord, mod): + with patch.object(mod.dt_util, "now", return_value=_now()), patch(TIMED_NOW, return_value=_now()): + return run(coord._async_update_data()) + + +def test_a_capped_timer_stops_inside_the_refresh_without_re_entering_it(): + coord, storage, mod, kid, chore = _system() + _reentrancy_guard(coord) + data = _update(coord, mod) # raised ReentrantRefresh before the fix + assert storage.get_timed_sessions() == [] + done = [c for c in storage.get_completions() if c.chore_id == chore.id] + assert len(done) == 1 and done[0].approved and done[0].points_awarded == 4 # 10 min at 2 per 5 min + # The running update returns the post-stop state: no stale session in it. + assert data["timed_sessions"] == [] + + +def test_a_parent_or_child_stop_outside_the_refresh_still_refreshes(): + coord, storage, mod, kid, chore = _system() + calls = [] + + async def refresh(): + calls.append(1) + + coord.async_refresh = refresh + with patch.object(mod.dt_util, "now", return_value=_now()), patch(TIMED_NOW, return_value=_now()): + run(coord.async_stop_timed_task(chore.id, kid.id)) + assert calls == [1] + + +def test_a_locked_timer_at_its_cap_goes_to_a_parent_instead_of_paying_out(): + coord, storage, mod, kid, chore = _system() + with patch.object(mod.dt_util, "now", return_value=_now()): + prereq = run(coord.async_add_chore("Brush teeth", points=1, schedule_mode="specific_days")) + chore.depends_on = [prereq.id] # added after the timer started + storage.update_chore(chore) + _reentrancy_guard(coord) + _update(coord, mod) + done = [c for c in storage.get_completions() if c.chore_id == chore.id] + assert len(done) == 1 + assert done[0].approved is False and done[0].points_awarded == 0 + assert done[0].submitted_points == 4, "the child's time is kept for the parent to approve" + assert storage.get_child(kid.id).points == 0 + coord._async_notify_pending_approval.assert_called_once() + + +def test_an_open_timer_at_its_cap_pays_as_before(): + coord, storage, mod, kid, chore = _system() + _reentrancy_guard(coord) + _update(coord, mod) + assert storage.get_child(kid.id).points == 4 + + +def test_the_stale_session_sweep_can_skip_its_refresh(): + coord, storage, mod, kid, chore = _system() + _reentrancy_guard(coord) + tomorrow = _now() + dt.timedelta(days=1) + with patch.object(mod.dt_util, "now", return_value=tomorrow), patch(TIMED_NOW, return_value=tomorrow): + run(coord._async_stop_stale_timed_sessions(refresh=False)) + assert storage.get_timed_sessions() == [] diff --git a/tests/test_timed_task.py b/tests/test_timed_task.py index e4ad2e58..50cb7191 100644 --- a/tests/test_timed_task.py +++ b/tests/test_timed_task.py @@ -41,6 +41,9 @@ def _coord(chore=None, child=None, active=None, dated=None): storage.async_save = AsyncMock() coord.storage = storage coord.async_refresh = AsyncMock() + # The stop asks whether the child can do the chore now (#771); these + # stand-ins have no availability data, and the chore is theirs to do. + coord._is_chore_completable_by_child = lambda _chore, _child_id: True return coord diff --git a/wiki/Timed-Tasks.md b/wiki/Timed-Tasks.md index 6da63f28..5ee36e4d 100644 --- a/wiki/Timed-Tasks.md +++ b/wiki/Timed-Tasks.md @@ -60,8 +60,11 @@ When `timed_max_daily_minutes` is set (non-zero): - The timer cannot be started once the cap is reached - If the session would exceed the cap, the duration is clamped to the remaining allowance +- A running timer that reaches the cap stops by itself and is submitted - The daily cap resets at midnight +If the chore can't be done right now when its timer stops — a prerequisite was added after the timer started, it's in a recurrence cooldown, the child is away — the time is not paid out automatically. It goes to a parent to approve instead, even for a chore that normally needs no approval, so the child's time isn't lost. + --- ## Session Model