danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Wire automation engine through next_run_at + register monthly_time (PR 2/4)

Browse source 
PR 1 (commit 6ad85e27) shipped the ``next_run_at`` pure function as
foundation plumbing. PR 2 wires the engine through it and adds
``monthly_time`` as a real registered trigger type. After this PR
``core/automation_engine.py`` no longer has its own datetime
arithmetic for daily / weekly schedules — every next-run computation
flows through one function with one set of defensive fallbacks.

Net user-visible change: zero (no UI surface for monthly_time yet —
that's PR 3). New ``monthly_time`` trigger is reachable only via
direct API for now.

**Engine refactor:**

- ``_finish_run`` — collapsed three inline branches (daily_time
  arithmetic, weekly_time arithmetic, fallback schedule arithmetic)
  into a single ``next_run_at(...)`` call with ``_dt_to_db_str``
  normalising the aware-UTC result to the engine's naive-UTC string
  convention. Retry-delay short-circuit preserved. Exception
  swallowing preserved (logged at debug, writes None next_run).

- ``_setup_daily_time_trigger`` + ``_setup_weekly_time_trigger`` +
  new ``_setup_monthly_time_trigger`` — three near-identical methods
  collapsed into one ``_setup_timed_trigger`` skeleton. Each public
  method is now a one-line dispatch passing trigger_type to the
  shared helper with a human-readable label for the debug log.

- Existing ``_next_weekly_occurrence`` deleted — its logic now lives
  in ``core/automation/schedule.py:_next_weekly`` (lifted in PR 1).

- New ``_dt_to_db_str(dt)`` module-level helper normalises aware-UTC
  → naive-UTC string. Centralised so a tz mistake here surfaces in
  one place. Aware non-UTC datetimes converted to UTC first
  (defensive against a future bug that passes the wrong tz).

- New ``_resolve_system_default_tz()`` reads the server's local IANA
  tz via ``tzlocal``. Cached at module import (the host's tz doesn't
  change while the process runs). Falls back to UTC when ``tzlocal``
  is missing — defensive for minimal Docker images.

- New ``self._default_tz`` engine attribute reads from
  ``automation.default_timezone`` config first, falls back to the
  system-detected IANA name. Override path lets users on weird
  setups pin a specific tz without touching env vars.

**Convergence fix (intentional behaviour change):**

Old ``_setup_daily_time_trigger`` / ``_setup_weekly_time_trigger``
didn't check the DB for an existing future ``next_run`` — they'd
recompute from scratch on every engine startup, overwriting manual
edits or pending retries. The interval path (``_setup_schedule_trigger``)
already had this check. The new shared ``_setup_timed_trigger``
brings daily / weekly in line: existing-future next_run wins over
freshly-computed delay. Treat this as a correctness fix, not a
breaking change — the old behaviour was an inconsistency, not a
deliberate choice.

**Backward-compat:**

- Existing ``schedule`` / ``daily_time`` / ``weekly_time`` rows
  continue to work unchanged. The ``_trigger_handlers`` registry
  keeps every historic key.

- Existing rows without an explicit ``tz`` field use
  ``self._default_tz`` (server-local IANA via ``tzlocal``) —
  preserves "every Monday 09:00 server-local" behaviour on
  non-UTC servers. Pre-fix the engine used naive
  ``datetime.now()`` which is also server-local; net effect is
  identical wall-clock time, just routed through a tz-aware
  pipeline that handles DST correctly (the May 2026 "next in 8h"
  bug fix class).

- Engine boots even when ``tzlocal`` is missing — the resolver
  falls back to UTC silently. Existing tests would catch a hard
  dependency on tzlocal here.

**``tzlocal>=5.0`` added to requirements.txt** alongside
``tzdata>=2024.1`` from PR 1. Both libraries are small and stable;
``tzlocal`` returns a clean IANA name across Windows / Linux /
Docker, sidestepping the platform-specific tz detection mess.

**Tests:** 20 new in ``tests/automation/test_engine_schedule_integration.py``:
- ``_dt_to_db_str`` x3 (aware UTC, aware non-UTC converted to UTC,
  naive assumed UTC)
- ``_resolve_system_default_tz`` x2 (returns IANA string, falls back
  to UTC without tzlocal)
- ``_finish_run`` dispatch through next_run_at for each trigger type
  (schedule, daily_time, weekly_time, monthly_time)
- Retry-delay short-circuits next_run_at
- next_run_at returns None → DB next_run cleared
- next_run_at raises → engine swallows + writes None
- Event triggers skipped (no scheduled next-run)
- ``self._default_tz`` passed through to next_run_at
- monthly_time registered in _trigger_handlers
- All historic trigger types kept registered
- ``_setup_monthly_time_trigger`` arms timer + writes DB
- ``_setup_timed_trigger`` honours existing future DB next_run
- Skip-with-log when next_run_at returns None
- End-to-end no-mock smoke for monthly_time

260 automation suite tests pass; the 240 from PR 1's branch plus 20
new integration tests. Ruff clean.

No WHATS_NEW entry — UI doesn't expose monthly_time yet (PR 3),
and the backward-compat path preserves existing daily/weekly
schedule timing.
This commit is contained in:
Broque Thomas 2026-05-27 12:03:41 -07:00
parent 6ad85e2733
commit 62ef39c4b7
3 changed files with 545 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

201
core/automation_engine.py
View file

@ -18,10 +18,14 @@ import time
import threading
import requests
from datetime import datetime, timedelta, timezone
from typing import Optional
from utils.logging_config import get_logger
from core.automation.schedule import next_run_at
logger = get_logger("automation_engine")
def _utcnow():
"""Return current UTC time as timezone-aware datetime."""
return datetime.now(timezone.utc)
@ -34,6 +38,42 @@ def _utc_after(seconds):
"""Return UTC time N seconds from now as naive string for DB storage."""
return (datetime.now(timezone.utc) + timedelta(seconds=seconds)).strftime('%Y-%m-%d %H:%M:%S')
def _dt_to_db_str(dt: datetime) -> str:
"""Convert an aware-UTC datetime to the naive-UTC string the DB
``next_run`` column stores. Centralised so a tz mistake here
surfaces in one place, not scattered through every caller of
``next_run_at``."""
if dt.tzinfo is None:
return dt.strftime('%Y-%m-%d %H:%M:%S')
return dt.astimezone(timezone.utc).strftime('%Y-%m-%d %H:%M:%S')
def _resolve_system_default_tz() -> str:
"""Return the IANA tz name the engine uses when a schedule's
trigger config doesn't carry an explicit ``tz`` field.
Existing daily / weekly rows pre-date the ``tz`` field — the
historic engine computed delays from naive ``datetime.now()``,
which is implicitly the server's local timezone. Falling back to
that same tz here preserves "every Monday at 09:00" running at
09:00 server local for rows that already exist in the DB.
Without ``tzlocal`` installed (or a system without a discoverable
tz), falls back to UTC."""
try:
import tzlocal
return tzlocal.get_localzone_name() or 'UTC'
except Exception:
return 'UTC'
# Server-local tz cached at import time. Re-reading per-call is
# pointless: the host's timezone doesn't change while the process is
# running. Tests that need a different default tz inject it through
# the engine's ``_default_tz`` attribute or via the
# ``automation.default_timezone`` config key.
_SYSTEM_DEFAULT_TZ = _resolve_system_default_tz()
SYSTEM_AUTOMATIONS = [
{
'name': 'Auto-Process Wishlist',
@ -134,11 +174,25 @@ class AutomationEngine:
self._max_chain_depth = 5
self._signal_cooldown_seconds = 10
# Default tz used when a schedule's ``trigger_config`` doesn't
# carry an explicit ``tz`` field — preserves historic behaviour
# for daily / weekly rows created before the field existed
# (engine used naive ``datetime.now()`` = server local). Reads
# from the ``automation.default_timezone`` config key first to
# let users override without touching env vars; falls back to
# the system-detected local tz.
try:
from config.settings import config_manager
self._default_tz = (config_manager.get('automation.default_timezone', '') or _SYSTEM_DEFAULT_TZ)
except Exception:
self._default_tz = _SYSTEM_DEFAULT_TZ
# Trigger registry: type → setup function (schedule only — events use emit())
self._trigger_handlers = {
'schedule': self._setup_schedule_trigger,
'daily_time': self._setup_daily_time_trigger,
'weekly_time': self._setup_weekly_time_trigger,
'monthly_time': self._setup_monthly_time_trigger,
}
# --- Action Handler Registration ---
@ -674,23 +728,21 @@ class AutomationEngine:
trigger_config = json.loads(auto.get('trigger_config') or '{}')
if retry_delay_seconds:
next_run_str = _utc_after(retry_delay_seconds)
elif trigger_type == 'daily_time':
# Next run is tomorrow at the configured time (compute delay from local time, store as UTC)
time_str = trigger_config.get('time', '00:00')
hour, minute = map(int, time_str.split(':'))
now_local = datetime.now()
target = now_local.replace(hour=hour, minute=minute, second=0, microsecond=0) + timedelta(days=1)
next_run_str = _utc_after((target - now_local).total_seconds())
elif trigger_type == 'weekly_time':
time_str = trigger_config.get('time', '00:00')
hour, minute = map(int, time_str.split(':'))
now_local = datetime.now()
target = self._next_weekly_occurrence(hour, minute, trigger_config.get('days', []))
next_run_str = _utc_after((target - now_local).total_seconds())
else:
delay = self._calc_delay_seconds(trigger_config)
if delay:
next_run_str = _utc_after(delay)
# Single integration point with ``next_run_at``. The
# helper handles every trigger type the engine
# supports (interval / daily / weekly / monthly) and
# returns aware-UTC; ``_dt_to_db_str`` normalises to
# the naive-UTC string the DB column stores. Tests
# injecting a different ``now_utc`` patch this same
# path — no scattered ``datetime.now()`` calls left.
next_run_dt = next_run_at(
trigger_type, trigger_config,
now_utc=_utcnow(),
default_tz=self._default_tz,
)
if next_run_dt is not None:
next_run_str = _dt_to_db_str(next_run_dt)
except Exception as e:
logger.debug("next run calc failed: %s", e)
@ -764,45 +816,72 @@ class AutomationEngine:
logger.debug(f"Scheduled automation {automation_id} in {delay:.0f}s")
def _setup_daily_time_trigger(self, automation_id, config):
"""Config: {"time": "03:00"} — runs daily at the specified local time."""
time_str = config.get('time', '00:00')
try:
hour, minute = map(int, time_str.split(':'))
except (ValueError, AttributeError):
hour, minute = 0, 0
now_local = datetime.now()
target = now_local.replace(hour=hour, minute=minute, second=0, microsecond=0)
if target <= now_local:
target += timedelta(days=1)
delay = (target - now_local).total_seconds()
next_run_str = _utc_after(delay)
self.db.update_automation(automation_id, next_run=next_run_str)
timer = threading.Timer(delay, self.run_automation, args=(automation_id,))
timer.daemon = True
timer.start()
with self._lock:
self._timers[automation_id] = timer
logger.debug(f"Daily automation {automation_id} scheduled for {time_str} (in {delay:.0f}s)")
"""Config: ``{"time": "03:00", "tz": "<IANA>"}`` — runs daily
at the specified local time. Tz defaults to ``self._default_tz``
when absent."""
self._setup_timed_trigger(automation_id, 'daily_time', config,
label=f"Daily at {config.get('time', '00:00')}")
def _setup_weekly_time_trigger(self, automation_id, config):
"""Config: {"time": "03:00", "days": ["mon", "wed", "fri"]}"""
time_str = config.get('time', '00:00')
try:
hour, minute = map(int, time_str.split(':'))
except (ValueError, AttributeError):
hour, minute = 0, 0
"""Config: ``{"time": "03:00", "days": ["mon","wed","fri"], "tz": "<IANA>"}``."""
day_names = ', '.join(config.get('days') or []) or 'every day'
self._setup_timed_trigger(automation_id, 'weekly_time', config,
label=f"Weekly {config.get('time', '00:00')} on {day_names}")
target = self._next_weekly_occurrence(hour, minute, config.get('days', []))
delay = (target - datetime.now()).total_seconds()
def _setup_monthly_time_trigger(self, automation_id, config):
"""Config: ``{"time": "09:00", "day_of_month": 15, "tz": "<IANA>"}``.
next_run_str = _utc_after(delay)
self.db.update_automation(automation_id, next_run=next_run_str)
Day clamped to [1, 31]; months too short for the target day
clamp to the last valid day (Feb 31 → Feb 28 / Feb 29 leap
year) per standard cron convention — see
``core.automation.schedule._next_monthly`` for the rule."""
day = config.get('day_of_month', 1)
self._setup_timed_trigger(automation_id, 'monthly_time', config,
label=f"Monthly {config.get('time', '00:00')} on day {day}")
def _setup_timed_trigger(self, automation_id, trigger_type, config, *, label):
"""Shared setup for daily / weekly / monthly time triggers.
All three flow through the same skeleton: compute next-run
via ``next_run_at``, persist to DB, arm a ``threading.Timer``
that fires the automation when the delay elapses. Lifting
these out of three near-identical methods means there's one
place to fix when (e.g.) timer rearm semantics need a tweak.
Honours an existing future ``next_run`` row in the DB —
prevents losing a hand-edited next_run when the engine
reschedules at startup. Same guard as the interval path."""
target_dt = next_run_at(
trigger_type, config or {},
now_utc=_utcnow(),
default_tz=self._default_tz,
)
if target_dt is None:
logger.warning(
f"Skip scheduling automation {automation_id}: next_run_at returned "
f"None for {trigger_type!r}",
)
return
delay = max(0.0, (target_dt - _utcnow()).total_seconds())
# If the DB already carries a future next_run, prefer it —
# matches the interval-path behaviour and lets manual edits
# or pending retries survive a process restart.
auto = self.db.get_automation(automation_id)
if auto and auto.get('next_run'):
try:
existing = datetime.strptime(
auto['next_run'], '%Y-%m-%d %H:%M:%S',
).replace(tzinfo=timezone.utc)
remaining = (existing - _utcnow()).total_seconds()
if remaining > 0:
delay = remaining
target_dt = existing
except (ValueError, TypeError):
pass
self.db.update_automation(automation_id, next_run=_dt_to_db_str(target_dt))
timer = threading.Timer(delay, self.run_automation, args=(automation_id,))
timer.daemon = True
@ -811,25 +890,7 @@ class AutomationEngine:
with self._lock:
self._timers[automation_id] = timer
day_names = ', '.join(config.get('days', [])) or 'every day'
logger.debug(f"Weekly automation {automation_id} scheduled for {time_str} on {day_names} (in {delay:.0f}s)")
def _next_weekly_occurrence(self, hour, minute, days):
"""Find the next datetime matching one of the given weekday abbreviations."""
day_map = {'mon': 0, 'tue': 1, 'wed': 2, 'thu': 3, 'fri': 4, 'sat': 5, 'sun': 6}
allowed = {day_map[d] for d in days if d in day_map}
if not allowed:
allowed = set(range(7)) # no days selected = every day
now = datetime.now()
for offset in range(8): # check today + next 7 days
candidate = now + timedelta(days=offset)
if candidate.weekday() in allowed:
target = candidate.replace(hour=hour, minute=minute, second=0, microsecond=0)
if target > now:
return target
# Fallback: tomorrow (shouldn't happen with 8-day scan)
return now.replace(hour=hour, minute=minute, second=0, microsecond=0) + timedelta(days=1)
logger.debug(f"{label} automation {automation_id} scheduled (in {delay:.0f}s)")
# --- Then Actions (notifications + signals) ---

9
requirements.txt
View file

@ -37,6 +37,15 @@ psutil==7.2.2
# the app's tz knowledge to the build date.
tzdata>=2024.1
# Cross-platform IANA timezone detection — returns the server's local
# tz as a string ('America/Los_Angeles', not 'PDT'). Consumed by the
# automation engine to preserve historic behaviour for daily / weekly
# trigger rows that don't carry an explicit ``tz`` field: the old
# engine computed delays from naive ``datetime.now()``, which is
# implicitly the server's local tz, so falling back to the same tz
# keeps existing schedules running at the same wall-clock time.
tzlocal>=5.0
# YouTube support -- unpinned; yt-dlp must track upstream releases to stay functional
yt-dlp>=2026.3.17

405
tests/automation/test_engine_schedule_integration.py Normal file
View file

@ -0,0 +1,405 @@
"""Engine-level integration tests for the next_run_at refactor (PR 2).
PR 1 lifted next-run computation into ``core/automation/schedule.py``
as a pure function. PR 2 wires the engine through it — three setup
methods (daily / weekly / monthly) collapse to one ``_setup_timed_trigger``
helper, ``_finish_run`` drops its inline daily / weekly arithmetic,
and ``monthly_time`` becomes a real registered trigger type.
These tests pin the integration surface:
- ``_finish_run`` dispatches through ``next_run_at`` for every trigger
type with the right args (trigger_type, trigger_config, now_utc,
default_tz) and serialises the result into the DB ``next_run`` column.
- Retry-delay short-circuit bypasses ``next_run_at`` (immediate
reschedule on transient failure, not on the regular cadence).
- Error path swallows + writes None next_run instead of crashing.
- Backward-compat: existing daily / weekly rows without an explicit
``tz`` field use the engine's ``_default_tz`` (server-local IANA),
preserving "every Monday 09:00 server-local" behaviour.
- New ``monthly_time`` trigger registers in ``_trigger_handlers`` and
arms a timer correctly.
- ``_setup_timed_trigger`` honours an existing future ``next_run`` in
the DB (lets manual edits / restart-resume survive).
- ``_dt_to_db_str`` correctly normalises aware + naive datetimes to
the engine's naive-UTC string convention.
"""
from __future__ import annotations
import json
from datetime import datetime, timedelta, timezone
from unittest.mock import MagicMock, patch
import pytest
from core.automation_engine import (
AutomationEngine,
_dt_to_db_str,
_resolve_system_default_tz,
)
def _utc(year, month, day, hour=0, minute=0, second=0):
"""Aware UTC datetime for test clarity — matches the convention
used in tests/automation/test_schedule.py."""
return datetime(year, month, day, hour, minute, second, tzinfo=timezone.utc)
# ---------------------------------------------------------------------------
# _dt_to_db_str — engine-side serialiser for ``next_run_at`` results.
# ---------------------------------------------------------------------------
def test_dt_to_db_str_normalises_aware_utc():
"""Aware-UTC datetime → naive-UTC string the DB column expects.
Format matches the engine's existing ``_utc_after``."""
dt = _utc(2026, 5, 27, 14, 30, 0)
assert _dt_to_db_str(dt) == '2026-05-27 14:30:00'
def test_dt_to_db_str_converts_aware_non_utc_to_utc_first():
"""An aware datetime in a non-UTC tz must be converted to UTC
before stringifying — otherwise the DB column would silently
hold a tz-shifted instant. This is the bug class the PR 1
tests already cover at the next_run_at layer; pin it here so a
future refactor that drops the ``.astimezone(UTC)`` step fails
fast."""
from zoneinfo import ZoneInfo
la = ZoneInfo('America/Los_Angeles')
dt = datetime(2026, 5, 27, 9, 0, 0, tzinfo=la) # 09:00 PDT
# 09:00 PDT (UTC-7) → 16:00 UTC.
assert _dt_to_db_str(dt) == '2026-05-27 16:00:00'
def test_dt_to_db_str_assumes_naive_is_utc():
"""Defensive — naive inputs are assumed UTC (matches the engine's
convention when parsing the DB column back out)."""
dt = datetime(2026, 5, 27, 14, 30, 0) # naive
assert _dt_to_db_str(dt) == '2026-05-27 14:30:00'
# ---------------------------------------------------------------------------
# _resolve_system_default_tz — engine's tz fallback chain.
# ---------------------------------------------------------------------------
def test_resolve_system_default_tz_returns_iana_string():
"""The engine caches this at import time; the result must be a
string (not a ZoneInfo object) so it can flow into next_run_at's
``default_tz`` param."""
result = _resolve_system_default_tz()
assert isinstance(result, str)
assert len(result) > 0
def test_resolve_system_default_tz_falls_back_to_utc_when_tzlocal_missing():
"""tzlocal is in requirements but the engine should still boot
without it — minimal Docker images / dev environments where
tzlocal didn't install. Defensive fallback to UTC instead of
crashing the engine."""
with patch.dict('sys.modules', {'tzlocal': None}):
# Force ImportError on the in-function import.
import importlib
import core.automation_engine as engine_mod
importlib.reload(engine_mod)
result = engine_mod._resolve_system_default_tz()
assert result == 'UTC'
# Reload again to restore normal state for subsequent tests.
importlib.reload(engine_mod)
# ---------------------------------------------------------------------------
# Engine fixture — minimal AutomationEngine with mocked DB.
# ---------------------------------------------------------------------------
@pytest.fixture
def engine_with_db():
"""AutomationEngine wired to a mock DB. Used across the
integration tests below — each test sets ``trigger_type`` and
``trigger_config`` on the mock's ``get_automation`` return value."""
db_mock = MagicMock()
db_mock.update_automation_run = MagicMock(return_value=True)
db_mock.update_automation = MagicMock(return_value=True)
db_mock.get_automation.return_value = None # tests override
engine = AutomationEngine(db_mock)
engine._running = True
return engine, db_mock
# ---------------------------------------------------------------------------
# _finish_run — single integration point with next_run_at.
# ---------------------------------------------------------------------------
def test_finish_run_dispatches_interval_trigger_through_next_run_at(engine_with_db):
"""Interval trigger flows through the same next_run_at call as
daily/weekly/monthly — no special-case branch left in the engine
for the legacy ``schedule`` type."""
engine, db_mock = engine_with_db
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'schedule',
'trigger_config': json.dumps({'interval': 6, 'unit': 'hours'}),
}
with patch('core.automation_engine.next_run_at') as mock_nra:
mock_nra.return_value = _utc(2026, 5, 27, 18, 0)
engine._finish_run(auto, 1, {'status': 'completed'}, error=None)
assert mock_nra.called
call = mock_nra.call_args
assert call.args[0] == 'schedule'
assert call.args[1] == {'interval': 6, 'unit': 'hours'}
assert call.kwargs['default_tz'] == engine._default_tz
def test_finish_run_dispatches_daily_time_through_next_run_at(engine_with_db):
"""Daily trigger no longer has its own inline arithmetic — the
refactor must route through next_run_at with the unmodified
trigger_config so tz / time fields flow through cleanly."""
engine, db_mock = engine_with_db
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'daily_time',
'trigger_config': json.dumps({'time': '09:00', 'tz': 'America/Los_Angeles'}),
}
with patch('core.automation_engine.next_run_at') as mock_nra:
mock_nra.return_value = _utc(2026, 5, 27, 16, 0)
engine._finish_run(auto, 1, {}, error=None)
assert mock_nra.call_args.args[0] == 'daily_time'
assert mock_nra.call_args.args[1] == {'time': '09:00', 'tz': 'America/Los_Angeles'}
def test_finish_run_dispatches_weekly_time_through_next_run_at(engine_with_db):
"""Weekly trigger same as daily — single integration point."""
engine, db_mock = engine_with_db
cfg = {'time': '09:00', 'days': ['mon', 'wed', 'fri'], 'tz': 'America/Los_Angeles'}
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'weekly_time',
'trigger_config': json.dumps(cfg),
}
with patch('core.automation_engine.next_run_at') as mock_nra:
mock_nra.return_value = _utc(2026, 5, 27, 16, 0)
engine._finish_run(auto, 1, {}, error=None)
assert mock_nra.call_args.args[0] == 'weekly_time'
assert mock_nra.call_args.args[1] == cfg
def test_finish_run_dispatches_monthly_time_through_next_run_at(engine_with_db):
"""New monthly_time trigger — added to _trigger_handlers in PR 2.
Without this entry, the if-trigger_type-in-handlers gate above
skips computation entirely and the DB next_run stays stale."""
engine, db_mock = engine_with_db
cfg = {'time': '09:00', 'day_of_month': 15, 'tz': 'America/Los_Angeles'}
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'monthly_time',
'trigger_config': json.dumps(cfg),
}
with patch('core.automation_engine.next_run_at') as mock_nra:
mock_nra.return_value = _utc(2026, 6, 15, 16, 0)
engine._finish_run(auto, 1, {}, error=None)
assert mock_nra.call_args.args[0] == 'monthly_time'
def test_finish_run_retry_delay_short_circuits_next_run_at(engine_with_db):
"""When a transient failure asks for a retry-delay reschedule
(e.g. action handler returns ``status='retry'``), the next_run
is just now+delay — NOT the regular schedule cadence. The
refactor must preserve this short-circuit path."""
engine, db_mock = engine_with_db
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'daily_time',
'trigger_config': json.dumps({'time': '09:00'}),
}
with patch('core.automation_engine.next_run_at') as mock_nra:
engine._finish_run(auto, 1, {}, error='boom', retry_delay_seconds=120)
# next_run_at NOT called — we used the retry delay instead.
mock_nra.assert_not_called()
# DB write happened (with a next_run computed from now + 120s).
assert db_mock.update_automation_run.called
written = db_mock.update_automation_run.call_args.kwargs.get('next_run')
assert written is not None
def test_finish_run_writes_none_when_next_run_at_returns_none(engine_with_db):
"""Defensive — next_run_at can return None for unknown trigger
types or completely broken configs. The engine must write
None to the DB rather than skip the update (which would leave
a stale next_run sitting in the row forever)."""
engine, db_mock = engine_with_db
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'daily_time',
'trigger_config': json.dumps({'time': '09:00'}),
}
with patch('core.automation_engine.next_run_at', return_value=None):
engine._finish_run(auto, 1, {}, error=None)
assert db_mock.update_automation_run.called
assert db_mock.update_automation_run.call_args.kwargs.get('next_run') is None
def test_finish_run_swallows_next_run_at_exception(engine_with_db):
"""next_run_at is pure so it shouldn't raise — but if it does
(programmer error in the helper, weird tz lookup), the engine
must not crash the finish-run cycle. Existing behaviour
swallows + logs at debug; the refactor preserves that."""
engine, db_mock = engine_with_db
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'daily_time',
'trigger_config': json.dumps({'time': '09:00'}),
}
with patch('core.automation_engine.next_run_at', side_effect=RuntimeError('boom')):
engine._finish_run(auto, 1, {}, error=None)
# DB write still happens, just with None next_run.
assert db_mock.update_automation_run.called
def test_finish_run_skips_next_run_for_event_triggers(engine_with_db):
"""Event-based triggers (not in _trigger_handlers) have no
scheduled next-run — the existing gate must still skip them
after the refactor."""
engine, db_mock = engine_with_db
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'event',
'trigger_config': json.dumps({}),
}
with patch('core.automation_engine.next_run_at') as mock_nra:
engine._finish_run(auto, 1, {}, error=None)
mock_nra.assert_not_called()
# update_automation_run still fires but with next_run=None.
assert db_mock.update_automation_run.call_args.kwargs.get('next_run') is None
def test_finish_run_passes_engine_default_tz(engine_with_db):
"""Backward-compat: existing daily/weekly rows without ``tz`` in
their config must use the engine's ``_default_tz`` (server-local
IANA via tzlocal). Pre-fix, the engine implicitly used naive
``datetime.now()`` = server local; post-fix the explicit
default_tz preserves that behaviour."""
engine, db_mock = engine_with_db
engine._default_tz = 'America/Los_Angeles' # simulate server-local
auto = {
'id': 1, 'enabled': True,
'trigger_type': 'daily_time',
'trigger_config': json.dumps({'time': '09:00'}), # NO tz field
}
with patch('core.automation_engine.next_run_at') as mock_nra:
mock_nra.return_value = _utc(2026, 5, 27, 16, 0)
engine._finish_run(auto, 1, {}, error=None)
assert mock_nra.call_args.kwargs['default_tz'] == 'America/Los_Angeles'
# ---------------------------------------------------------------------------
# Trigger handler registration.
# ---------------------------------------------------------------------------
def test_engine_registers_monthly_time_trigger(engine_with_db):
"""``monthly_time`` joins schedule / daily_time / weekly_time in
the _trigger_handlers registry — without this, calling
``schedule_automation`` on a monthly row falls through the
``trigger_type in self._trigger_handlers`` gate and the
automation never gets armed."""
engine, _ = engine_with_db
assert 'monthly_time' in engine._trigger_handlers
assert callable(engine._trigger_handlers['monthly_time'])
def test_engine_keeps_existing_trigger_registrations(engine_with_db):
"""Backward-compat: the refactor must not drop the historic
trigger types. schedule / daily_time / weekly_time stay
registered alongside the new monthly_time."""
engine, _ = engine_with_db
assert 'schedule' in engine._trigger_handlers
assert 'daily_time' in engine._trigger_handlers
assert 'weekly_time' in engine._trigger_handlers
# ---------------------------------------------------------------------------
# _setup_timed_trigger — shared skeleton for daily / weekly / monthly.
# ---------------------------------------------------------------------------
def test_setup_monthly_time_trigger_writes_next_run_and_arms_timer(engine_with_db):
"""Sanity check that the new monthly handler actually wires up
a timer (it's the new-shaped trigger so a "no timer armed"
regression would otherwise be silent — the automation just
never fires)."""
engine, db_mock = engine_with_db
db_mock.get_automation.return_value = {'id': 1, 'next_run': None}
with patch('core.automation_engine.threading.Timer') as mock_timer_cls:
mock_timer = MagicMock()
mock_timer_cls.return_value = mock_timer
engine._setup_monthly_time_trigger(
1, {'time': '09:00', 'day_of_month': 15, 'tz': 'UTC'},
)
# Timer armed.
assert mock_timer.start.called
# next_run written to DB.
assert db_mock.update_automation.called
written = db_mock.update_automation.call_args.kwargs.get('next_run')
assert written is not None and isinstance(written, str)
def test_setup_timed_trigger_honours_future_db_next_run(engine_with_db):
"""If the DB row already has a future ``next_run`` (e.g. a
manual edit, or a process restart picking up where it left
off), the setup must keep that instant — not recompute from
scratch. Matches the existing interval-path behaviour and
prevents losing pending retries."""
engine, db_mock = engine_with_db
# Far-future next_run in the DB.
future = (datetime.now(timezone.utc) + timedelta(hours=24)).strftime('%Y-%m-%d %H:%M:%S')
db_mock.get_automation.return_value = {'id': 1, 'next_run': future}
with patch('core.automation_engine.threading.Timer') as mock_timer_cls:
mock_timer_cls.return_value = MagicMock()
engine._setup_daily_time_trigger(1, {'time': '09:00', 'tz': 'UTC'})
# Engine writes the EXISTING next_run back (the if-future-in-DB
# branch overrides the freshly-computed delay).
written = db_mock.update_automation.call_args.kwargs.get('next_run')
assert written == future
def test_setup_timed_trigger_skips_when_next_run_at_returns_none(engine_with_db):
"""If next_run_at can't compute a valid next-run (e.g. broken
config that defeats every defensive fallback in the helper),
the setup must NOT arm a timer with bogus delay. Skip-with-log
is safer than scheduling-for-the-past or scheduling-immediately."""
engine, db_mock = engine_with_db
db_mock.get_automation.return_value = {'id': 1, 'next_run': None}
with patch('core.automation_engine.next_run_at', return_value=None), \
patch('core.automation_engine.threading.Timer') as mock_timer_cls:
engine._setup_monthly_time_trigger(1, {})
# No timer armed.
mock_timer_cls.assert_not_called()
# ---------------------------------------------------------------------------
# End-to-end: real next_run_at + engine wiring (no mocks).
# ---------------------------------------------------------------------------
def test_end_to_end_monthly_schedule_produces_valid_db_string(engine_with_db):
"""No-mock smoke: monthly_time config flows from engine through
the real next_run_at into a valid DB string. Catches any
serialisation drift between PR 1 (helper returns aware UTC) and
PR 2 (engine writes naive UTC string)."""
engine, db_mock = engine_with_db
engine._default_tz = 'UTC'
db_mock.get_automation.return_value = {'id': 1, 'next_run': None}
with patch('core.automation_engine.threading.Timer') as mock_timer_cls:
mock_timer_cls.return_value = MagicMock()
engine._setup_monthly_time_trigger(
1, {'time': '09:00', 'day_of_month': 15},
)
written = db_mock.update_automation.call_args.kwargs['next_run']
# Format matches the engine's existing _utcnow_str / _utc_after.
parsed = datetime.strptime(written, '%Y-%m-%d %H:%M:%S')
# Day-of-month is 15 in the user's tz (UTC here).
assert parsed.day == 15
assert parsed.hour == 9
assert parsed.minute == 0