From ec4a55c104c53cff50f6b9b4d4bedaaec240dcdd Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Wed, 27 May 2026 11:15:47 -0700 Subject: [PATCH 1/2] Add next_run_at pure function for Auto-Sync schedule types (PR 1/4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend plumbing for upcoming weekly + monthly Auto-Sync schedules. PR 1 of 4 in the schedule-types feature — see ``memory/project_auto_sync_schedule_types.md`` for the full plan. Net behaviour change in this PR: zero. The automation engine still computes next_run via its existing inline ``_calc_delay_seconds`` / ``_next_weekly_occurrence`` helpers; this module is unused until PR 2 wires the engine through. Lands separately so the foundation can sit on dev for a beat before the engine change. ``core/automation/schedule.py:next_run_at(trigger_type, trigger_config, now_utc, default_tz)``: - Pure function. ``now_utc`` injected (tests freeze time without monkeypatching ``datetime.now``); ``default_tz`` injected (so daily / weekly / monthly schedules compute against the USER's timezone, not the server's — the same class of bug that produced the May 2026 "Auto-Sync next in 8h" timezone fix). - Returns aware-UTC ``datetime`` ready to serialise to the DB ``next_run`` column, or ``None`` for unrecognised / event-based triggers (callers should not write a next_run for those). - Naive ``now_utc`` inputs are assumed UTC for defensive symmetry with the engine's DB-string parser convention. Trigger types covered: - ``schedule``: ``{interval: N, unit: 'minutes'|'hours'|'days'|'weeks'}`` — matches engine's existing ``_calc_delay_seconds``. Unknown unit defaults to hours; zero/negative interval clamps to 1 (preserves the engine's guard against scheduling for the past); non-numeric interval falls back to 1. - ``daily_time``: ``{time: 'HH:MM', tz: ''}`` — DST-aware via ``zoneinfo``; ``tz`` falls back to ``default_tz``; unknown IANA string falls back to UTC; garbage ``time`` falls back to 00:00. - ``weekly_time``: ``{time, days: ['mon',...], tz}`` — empty / all- invalid ``days`` list means "every day" (matches engine fallback); abbreviations case-insensitive; 8-day scan finds the next match. - ``monthly_time``: ``{time, day_of_month: 1-31, tz}`` — NEW shape. Day clamped to [1, 31]. Months too short for the target day clamp to the LAST valid day rather than skipping a month (standard cron convention; running a day early in February is less surprising than missing the whole month). 12-iteration loop cap so a pathological config can't infinite-loop. Tests (36 cases, all passing): - Interval: every unit, unknown-unit fallback, zero/negative/garbage interval clamp, tz field ignored on interval (wall-clock-independent). - Daily: today-at-future-time runs today, today-at-past-time rolls to tomorrow, exact-match rolls to tomorrow (no schedule-now-then-schedule- again-immediately), user-tz vs server-tz, default_tz fallback, garbage time / unknown tz defensive returns. - Weekly: same-day-still-future qualifies, same-day-past rolls to next allowed day, wraps across week boundary, empty days = every day, garbage abbreviations dropped, case-insensitive, tz across day boundary (LA Wednesday evening is Thursday UTC). - Monthly: target day this month, rolls to next month when passed, Feb 31 → Feb 28 / Feb 29 leap year, day_of_month above 31 / below 1 clamp, Dec → Jan year roll, user-tz pre-midnight edge case. - Result-shape contract: every returned datetime is aware UTC at offset zero (engine relies on this when serialising to the ``next_run`` string column). Added ``tzdata==2026.2`` to requirements.txt. Windows ``zoneinfo`` and minimal Docker base images ship without the system tz database; without ``tzdata`` ``ZoneInfo('America/Los_Angeles')`` raises ``ZoneInfoNotFoundError`` and the helper silently falls back to UTC. No WHATS_NEW entry — no user-visible behaviour change in this PR. PR 2 (engine wire-through) will land the user-facing changelog entry when ``monthly_time`` becomes a real schedulable trigger. --- core/automation/schedule.py | 303 +++++++++++++++++++++++ requirements.txt | 6 + tests/automation/test_schedule.py | 383 ++++++++++++++++++++++++++++++ 3 files changed, 692 insertions(+) create mode 100644 core/automation/schedule.py create mode 100644 tests/automation/test_schedule.py diff --git a/core/automation/schedule.py b/core/automation/schedule.py new file mode 100644 index 00000000..e3b8e37b --- /dev/null +++ b/core/automation/schedule.py @@ -0,0 +1,303 @@ +"""Pure functions for computing the next-run datetime of a scheduled +automation trigger. + +The Auto-Sync schedule board currently exposes interval-based scheduling +(``every N hours``) backed by ``trigger_type='schedule'``. The +automation engine ALSO supports ``daily_time`` and ``weekly_time`` +triggers via separate ``_setup_*_trigger`` methods inline on the engine +class. None of that logic is currently testable in isolation — the +engine's ``_finish_run`` reaches for ``datetime.now()``, threads it +through ``_next_weekly_occurrence``, and writes the result to the DB, +all on the same call. + +This module lifts the "given a trigger config, what's the next run?" +question out of the engine into a pure function: + + next_run_at(trigger_type, trigger_config, now_utc, default_tz) + -> Optional[datetime] + +That means: +- ``now_utc`` is INJECTED, not pulled from the system clock. Tests + freeze time without monkeypatching ``datetime.now``. +- ``default_tz`` is INJECTED. Daily / weekly / monthly schedules are + inherently in the USER'S timezone (cron "every Monday at 9am" is + not UTC), and the historic engine implicitly used the server's + local tz via naive ``datetime.now()``. That broke for users on a + different tz than their server. The pure function takes the tz + explicitly so the caller controls it. +- Returns an aware UTC ``datetime`` ready to serialise to the DB's + ``next_run`` string column, or ``None`` for unrecognised / + event-based triggers (engine should not store a next_run for those). + +PR 1 of the schedule-types feature ships ONLY this module + tests. +The engine continues to compute next_run via its existing inline +helpers; PR 2 collapses those into a single ``next_run_at`` call. +Net behavior is identical until the engine is wired through — this +PR is pure plumbing. + +Schedule types supported here: + +- ``schedule`` (interval): ``{interval: N, unit: 'minutes'|'hours'|'days'}`` + — adds the interval to ``now_utc``; no tz needed. +- ``daily_time``: ``{time: 'HH:MM', tz: ''}`` — runs every day at + the given local time in the given timezone. ``tz`` falls back to + ``default_tz`` when absent. +- ``weekly_time``: ``{time: 'HH:MM', days: ['mon','wed',...], tz: ''}`` + — runs on the matching weekday(s) at the given local time. Empty + ``days`` list means "every day" (matches the engine's existing + fallback in ``_next_weekly_occurrence``). +- ``monthly_time``: ``{time: 'HH:MM', day_of_month: 1-31, tz: ''}`` + — runs on the given day each month. Days that don't exist in a + given month (Feb 30, Apr 31) clamp to the LAST valid day of that + month rather than skipping the run entirely; missing a whole + month silently because the schedule was over-eager is worse than + running a day early. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone +from typing import Any, Dict, Optional + +try: + from zoneinfo import ZoneInfo, ZoneInfoNotFoundError +except ImportError: # pragma: no cover — zoneinfo ships with 3.9+ + ZoneInfo = None + ZoneInfoNotFoundError = Exception + + +# Weekday abbreviation → ``datetime.weekday()`` index (Mon=0..Sun=6). +# Mirrors the engine's existing ``_next_weekly_occurrence`` mapping so +# schedules created against either implementation accept the same +# ``days`` strings. +_WEEKDAY_MAP = { + 'mon': 0, 'tue': 1, 'wed': 2, 'thu': 3, 'fri': 4, 'sat': 5, 'sun': 6, +} + +_INTERVAL_MULTIPLIERS = { + 'minutes': 60, + 'hours': 60 * 60, + 'days': 60 * 60 * 24, + 'weeks': 60 * 60 * 24 * 7, +} + + +def next_run_at( + trigger_type: str, + trigger_config: Dict[str, Any], + now_utc: datetime, + default_tz: str = 'UTC', +) -> Optional[datetime]: + """Compute the next-run timestamp (UTC, aware) for a scheduled + trigger. Returns ``None`` for unrecognised types or event-based + triggers — callers should not write a next_run for those. + + See module docstring for supported trigger types + config shapes. + """ + if not isinstance(trigger_config, dict): + trigger_config = {} + + if trigger_type == 'schedule': + return _next_interval(trigger_config, now_utc) + if trigger_type == 'daily_time': + return _next_daily(trigger_config, now_utc, default_tz) + if trigger_type == 'weekly_time': + return _next_weekly(trigger_config, now_utc, default_tz) + if trigger_type == 'monthly_time': + return _next_monthly(trigger_config, now_utc, default_tz) + return None + + +# --------------------------------------------------------------------------- +# Interval +# --------------------------------------------------------------------------- + + +def _next_interval(config: Dict[str, Any], now_utc: datetime) -> datetime: + """``{interval: N, unit: 'hours'}`` → ``now_utc + N hours``. + + Mirrors the engine's existing ``_calc_delay_seconds``. Unit defaults + to ``hours`` for backward compat with legacy DB rows that pre-date + the unit field being mandatory; interval defaults to 1 so a fully + empty config doesn't divide-by-zero or schedule for the past.""" + try: + interval = max(int(config.get('interval', 1)), 1) + except (TypeError, ValueError): + interval = 1 + unit = config.get('unit') or 'hours' + seconds = interval * _INTERVAL_MULTIPLIERS.get(unit, _INTERVAL_MULTIPLIERS['hours']) + return _ensure_utc(now_utc) + timedelta(seconds=seconds) + + +# --------------------------------------------------------------------------- +# Daily +# --------------------------------------------------------------------------- + + +def _next_daily( + config: Dict[str, Any], now_utc: datetime, default_tz: str, +) -> datetime: + """``{time: 'HH:MM', tz: ''}`` → next occurrence of that + wall-clock time in the user's timezone, expressed as aware UTC. + + DST-aware via ``zoneinfo``: when the local time falls during a + spring-forward gap, the ``replace`` lands on a non-existent + instant; ``zoneinfo`` resolves that to the gap's later side + (e.g. 02:30 on the DST-forward day becomes 03:30 local). Tests + pin both spring-forward and fall-back behaviour.""" + tz = _resolve_tz(config.get('tz') or default_tz) + hour, minute = _parse_hhmm(config.get('time')) + now_local = _ensure_utc(now_utc).astimezone(tz) + target_local = now_local.replace(hour=hour, minute=minute, second=0, microsecond=0) + if target_local <= now_local: + target_local = target_local + timedelta(days=1) + return target_local.astimezone(timezone.utc) + + +# --------------------------------------------------------------------------- +# Weekly +# --------------------------------------------------------------------------- + + +def _next_weekly( + config: Dict[str, Any], now_utc: datetime, default_tz: str, +) -> datetime: + """``{time: 'HH:MM', days: ['mon',...], tz: ''}`` → next + occurrence of that wall-clock time on any of the listed weekdays + in the user's timezone. + + Empty ``days`` list ≡ every day, matching the engine's existing + fallback. Unrecognised day abbreviations are silently dropped + (an empty result-set then triggers the every-day fallback).""" + tz = _resolve_tz(config.get('tz') or default_tz) + hour, minute = _parse_hhmm(config.get('time')) + days = _parse_weekdays(config.get('days')) + + now_local = _ensure_utc(now_utc).astimezone(tz) + # Scan today + next 7 days; the matching day with a future + # local time wins. 8-day scan is enough to handle the case where + # today already passed the time AND today is the only allowed + # weekday (next occurrence is exactly one week out). + for offset in range(8): + candidate = now_local + timedelta(days=offset) + if candidate.weekday() not in days: + continue + target = candidate.replace(hour=hour, minute=minute, second=0, microsecond=0) + if target > now_local: + return target.astimezone(timezone.utc) + # Shouldn't reach: 8-day scan always finds a hit when ``days`` + # is non-empty. Defensive fallback: next week, same weekday as today. + fallback = (now_local + timedelta(days=7)).replace( + hour=hour, minute=minute, second=0, microsecond=0, + ) + return fallback.astimezone(timezone.utc) + + +# --------------------------------------------------------------------------- +# Monthly +# --------------------------------------------------------------------------- + + +def _next_monthly( + config: Dict[str, Any], now_utc: datetime, default_tz: str, +) -> datetime: + """``{time: 'HH:MM', day_of_month: 1-31, tz: ''}`` → next + occurrence in the user's timezone. + + ``day_of_month`` is clamped to ``[1, 31]``. When the target day + doesn't exist in a given month (Feb 30, Apr 31), the schedule + falls back to the LAST valid day of that month — running a day + or two early in short months is less surprising than skipping + a month entirely. This matches the convention every cron + implementation in the wild settled on.""" + tz = _resolve_tz(config.get('tz') or default_tz) + hour, minute = _parse_hhmm(config.get('time')) + raw_day = config.get('day_of_month', 1) + try: + target_day = max(1, min(31, int(raw_day))) + except (TypeError, ValueError): + target_day = 1 + + now_local = _ensure_utc(now_utc).astimezone(tz) + # Try this month first; if the target day has already passed + # (or doesn't exist this month and the clamped day is in the + # past), advance to next month. Loop bounded to 12 iterations + # so a pathologically broken config can't infinite-loop us. + year, month = now_local.year, now_local.month + for _ in range(12): + day = min(target_day, _days_in_month(year, month)) + target = now_local.replace( + year=year, month=month, day=day, + hour=hour, minute=minute, second=0, microsecond=0, + ) + if target > now_local: + return target.astimezone(timezone.utc) + # Roll to next month. + if month == 12: + year, month = year + 1, 1 + else: + month += 1 + # Defensive — should be unreachable. + return (now_local + timedelta(days=30)).astimezone(timezone.utc) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _ensure_utc(dt: datetime) -> datetime: + """Coerce a possibly-naive datetime to aware UTC. Naive inputs + are assumed UTC (matches the convention the engine uses when + parsing the DB ``next_run`` column).""" + if dt.tzinfo is None: + return dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) + + +def _resolve_tz(name: Optional[str]): + """Look up an IANA tz by name. Falls back to UTC when zoneinfo + isn't available (3.8 / minimal images) or the name is unknown.""" + if not name: + return timezone.utc + if ZoneInfo is None: + return timezone.utc + try: + return ZoneInfo(name) + except ZoneInfoNotFoundError: + return timezone.utc + + +def _parse_hhmm(time_str: Optional[str]) -> tuple: + """Parse ``HH:MM`` → ``(hour, minute)``. Defaults to 00:00 on + garbage input — same defensive shape as the engine's existing + daily/weekly time parsing.""" + if not isinstance(time_str, str): + return 0, 0 + try: + h, m = time_str.split(':', 1) + return max(0, min(23, int(h))), max(0, min(59, int(m))) + except (ValueError, AttributeError): + return 0, 0 + + +def _parse_weekdays(days) -> set: + """``['mon', 'wed']`` → ``{0, 2}``. Empty / missing / all-invalid + list returns ``set(range(7))`` ("every day"), matching the + engine's existing ``_next_weekly_occurrence`` fallback.""" + if not isinstance(days, (list, tuple)): + return set(range(7)) + parsed = {_WEEKDAY_MAP[d.lower()] for d in days + if isinstance(d, str) and d.lower() in _WEEKDAY_MAP} + return parsed or set(range(7)) + + +def _days_in_month(year: int, month: int) -> int: + """Last calendar day of ``year-month``. Stdlib-only — no calendar + module import needed; cycle through the 12 months.""" + if month == 12: + next_first = datetime(year + 1, 1, 1) + else: + next_first = datetime(year, month + 1, 1) + last_day = next_first - timedelta(days=1) + return last_day.day diff --git a/requirements.txt b/requirements.txt index 73ef28a7..4a94d257 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,6 +28,12 @@ beautifulsoup4==4.14.3 # System monitoring psutil==7.2.2 +# IANA timezone data — required by ``zoneinfo`` on Windows hosts and +# minimal Docker base images that ship without the system tz database. +# Consumed by ``core/automation/schedule.py`` for daily / weekly / +# monthly schedule next-run computation in the user's local timezone. +tzdata==2026.2 + # YouTube support -- unpinned; yt-dlp must track upstream releases to stay functional yt-dlp>=2026.3.17 diff --git a/tests/automation/test_schedule.py b/tests/automation/test_schedule.py new file mode 100644 index 00000000..a73782ff --- /dev/null +++ b/tests/automation/test_schedule.py @@ -0,0 +1,383 @@ +"""Tests for ``core/automation/schedule.py:next_run_at``. + +Pure function over (trigger_type, trigger_config, now_utc, default_tz) +so each case can pin a single rule without monkeypatching the system +clock. Covers the existing engine behaviour (interval, daily, weekly) +plus the new ``monthly_time`` shape PR 1 introduces. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone +from zoneinfo import ZoneInfo + +import pytest + +from core.automation.schedule import next_run_at + + +# --------------------------------------------------------------------------- +# Helper — clear, timezone-aware datetime construction in test bodies. +# --------------------------------------------------------------------------- + + +def _utc(year: int, month: int, day: int, hour: int = 0, minute: int = 0) -> datetime: + """Aware UTC datetime — every ``now_utc`` injection in tests + flows through this so a stray timezone bug is impossible.""" + return datetime(year, month, day, hour, minute, tzinfo=timezone.utc) + + +# --------------------------------------------------------------------------- +# Dispatcher: trigger_type routing. +# --------------------------------------------------------------------------- + + +def test_returns_none_for_unrecognised_trigger_type(): + """Event-based / unknown trigger types are not scheduled — the + caller should NOT write a next_run for them.""" + now = _utc(2026, 5, 27, 12, 0) + assert next_run_at('event', {}, now) is None + assert next_run_at('garbage', {'interval': 1}, now) is None + assert next_run_at('', {}, now) is None + + +def test_returns_none_for_non_dict_config(): + """Defensive — callers may pass through whatever ``json.loads`` + returned. Non-dict configs trigger the fallback path which is + 'treat as empty dict + use defaults'.""" + now = _utc(2026, 5, 27, 12, 0) + # Interval-typed with garbage config falls back to defaults + # (interval=1, unit='hours') rather than crashing. + result = next_run_at('schedule', None, now) + assert result == now + timedelta(hours=1) + + +# --------------------------------------------------------------------------- +# Interval (``trigger_type='schedule'``) +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize('unit,seconds_per_unit', [ + ('minutes', 60), + ('hours', 3600), + ('days', 86400), + ('weeks', 86400 * 7), +]) +def test_interval_units(unit, seconds_per_unit): + """Every supported unit scales the interval correctly.""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at('schedule', {'interval': 3, 'unit': unit}, now) + assert result == now + timedelta(seconds=3 * seconds_per_unit) + + +def test_interval_unknown_unit_defaults_to_hours(): + """Backward compat with DB rows whose ``unit`` field is missing + or an unrecognised value — engine's historic behaviour was to + treat as hours, and we preserve that.""" + now = _utc(2026, 5, 27, 12, 0) + assert next_run_at('schedule', {'interval': 2, 'unit': 'fortnights'}, now) == now + timedelta(hours=2) + assert next_run_at('schedule', {'interval': 2}, now) == now + timedelta(hours=2) + + +def test_interval_clamps_zero_and_negative_to_one(): + """Without a floor a zero/negative interval would schedule for + the past or fire instantly in a loop. Engine clamped to >=1 via + ``max(int(interval), 1)``; we preserve that contract.""" + now = _utc(2026, 5, 27, 12, 0) + assert next_run_at('schedule', {'interval': 0, 'unit': 'hours'}, now) == now + timedelta(hours=1) + assert next_run_at('schedule', {'interval': -5, 'unit': 'hours'}, now) == now + timedelta(hours=1) + + +def test_interval_garbage_interval_falls_back_to_one(): + """Non-numeric ``interval`` → default of 1. Survives a JSON column + where the field was typed as a string by an old admin script.""" + now = _utc(2026, 5, 27, 12, 0) + assert next_run_at('schedule', {'interval': 'oops', 'unit': 'hours'}, now) == now + timedelta(hours=1) + + +def test_interval_ignores_tz_field(): + """Interval scheduling is wall-clock-independent — adding 6 hours + is the same in every timezone. The ``tz`` field is ignored even + if a caller mistakenly sets it.""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at('schedule', + {'interval': 6, 'unit': 'hours', 'tz': 'America/Los_Angeles'}, + now) + assert result == now + timedelta(hours=6) + + +# --------------------------------------------------------------------------- +# Daily (``trigger_type='daily_time'``) +# --------------------------------------------------------------------------- + + +def test_daily_today_at_future_time_runs_today(): + """It's 12:00 UTC and the schedule says 18:00 UTC — next run is + today at 18:00, not tomorrow.""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at('daily_time', {'time': '18:00', 'tz': 'UTC'}, now) + assert result == _utc(2026, 5, 27, 18, 0) + + +def test_daily_today_at_past_time_runs_tomorrow(): + """It's 18:00 UTC and the schedule says 09:00 UTC — next run is + tomorrow at 09:00.""" + now = _utc(2026, 5, 27, 18, 0) + result = next_run_at('daily_time', {'time': '09:00', 'tz': 'UTC'}, now) + assert result == _utc(2026, 5, 28, 9, 0) + + +def test_daily_at_exact_target_time_runs_tomorrow(): + """Edge case: schedule fires at exactly 09:00, and ``now`` is + exactly 09:00. ``<=`` check pushes to tomorrow — otherwise we'd + immediately reschedule for the present moment and the engine + would run again in 0s.""" + now = _utc(2026, 5, 27, 9, 0) + result = next_run_at('daily_time', {'time': '09:00', 'tz': 'UTC'}, now) + assert result == _utc(2026, 5, 28, 9, 0) + + +def test_daily_respects_user_timezone_not_server_local(): + """User on Pacific time, schedule says ``09:00 America/Los_Angeles``. + Server is UTC. At 12:00 UTC = 05:00 LA local, next run is 09:00 LA + today = 16:00 UTC. Pre-fix the engine used naive ``datetime.now()`` + and read 12:00 as if it were the user's tz, mis-scheduling by the + server-vs-user tz offset.""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at('daily_time', + {'time': '09:00', 'tz': 'America/Los_Angeles'}, + now) + # 09:00 LA on 2026-05-27 → 16:00 UTC (PDT, UTC-7). + assert result == _utc(2026, 5, 27, 16, 0) + + +def test_daily_falls_back_to_default_tz_when_config_missing(): + """``tz`` field absent on the config — pulls from ``default_tz`` + (typically the app-level setting).""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at('daily_time', {'time': '09:00'}, now, + default_tz='America/Los_Angeles') + assert result == _utc(2026, 5, 27, 16, 0) + + +def test_daily_garbage_time_string_defaults_to_midnight(): + """Bad ``time`` string → defaults to 00:00 (engine's existing + behaviour). Better than crashing the scheduler when a row's + config was hand-edited.""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at('daily_time', {'time': 'garbage', 'tz': 'UTC'}, now) + # 00:00 today already passed → tomorrow at 00:00. + assert result == _utc(2026, 5, 28, 0, 0) + + +def test_daily_unknown_tz_falls_back_to_utc(): + """Unknown IANA tz string → fall back to UTC rather than crash.""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at('daily_time', + {'time': '15:00', 'tz': 'Imaginary/Place'}, + now) + # Treated as UTC → next run today at 15:00 UTC. + assert result == _utc(2026, 5, 27, 15, 0) + + +# --------------------------------------------------------------------------- +# Weekly (``trigger_type='weekly_time'``) +# --------------------------------------------------------------------------- + + +def test_weekly_picks_next_matching_weekday(): + """It's Wednesday and the schedule wants Mon/Wed/Fri — same day + qualifies if the time is still in the future.""" + # 2026-05-27 is a Wednesday. + now = _utc(2026, 5, 27, 8, 0) + result = next_run_at('weekly_time', + {'time': '14:00', 'days': ['mon', 'wed', 'fri'], 'tz': 'UTC'}, + now) + assert result == _utc(2026, 5, 27, 14, 0) + + +def test_weekly_rolls_to_next_allowed_day_when_today_passed(): + """Wednesday 18:00 UTC, schedule wants Mon/Wed/Fri at 14:00 — + Wed 14:00 already passed today, next match is Friday at 14:00.""" + now = _utc(2026, 5, 27, 18, 0) # Wed + result = next_run_at('weekly_time', + {'time': '14:00', 'days': ['mon', 'wed', 'fri'], 'tz': 'UTC'}, + now) + assert result == _utc(2026, 5, 29, 14, 0) # Fri + + +def test_weekly_wraps_to_next_week(): + """Sunday past the time, schedule wants only Monday — next match + is the very next day.""" + # 2026-05-31 is a Sunday. + now = _utc(2026, 5, 31, 15, 0) + result = next_run_at('weekly_time', + {'time': '09:00', 'days': ['mon'], 'tz': 'UTC'}, + now) + assert result == _utc(2026, 6, 1, 9, 0) # next Monday + + +def test_weekly_empty_days_means_every_day(): + """Empty ``days`` list → treat as every weekday. Matches the + engine's existing fallback in ``_next_weekly_occurrence``.""" + now = _utc(2026, 5, 27, 8, 0) + result = next_run_at('weekly_time', + {'time': '14:00', 'days': [], 'tz': 'UTC'}, + now) + # Today (Wed) qualifies since 14:00 is still future. + assert result == _utc(2026, 5, 27, 14, 0) + + +def test_weekly_unrecognised_day_abbreviations_dropped(): + """``'mond'`` / ``'frid'`` are not in the map — silently drop. + If ALL listed days are invalid, fall through to the every-day + default (matches the empty-list behaviour).""" + now = _utc(2026, 5, 27, 8, 0) + result = next_run_at('weekly_time', + {'time': '14:00', 'days': ['mond', 'frid'], 'tz': 'UTC'}, + now) + # All garbage → every day → today (Wed) qualifies. + assert result == _utc(2026, 5, 27, 14, 0) + + +def test_weekly_day_abbreviations_case_insensitive(): + """``MON`` / ``Mon`` / ``mon`` all parse to weekday 0.""" + now = _utc(2026, 5, 27, 8, 0) # Wed + result = next_run_at('weekly_time', + {'time': '14:00', 'days': ['MON', 'WED'], 'tz': 'UTC'}, + now) + assert result == _utc(2026, 5, 27, 14, 0) + + +def test_weekly_respects_user_tz_across_day_boundary(): + """It's 23:30 UTC on Wednesday → 16:30 LA local (still Wed). + Schedule fires Mon/Wed/Fri at 18:00 LA. Next run is 18:00 LA + today (Wed in LA, but Thursday in UTC because of the 7h offset).""" + now = _utc(2026, 5, 27, 23, 30) # Wed 23:30 UTC / Wed 16:30 LA + result = next_run_at('weekly_time', + {'time': '18:00', 'days': ['mon', 'wed', 'fri'], + 'tz': 'America/Los_Angeles'}, + now) + # 2026-05-27 18:00 LA → 2026-05-28 01:00 UTC. + assert result == _utc(2026, 5, 28, 1, 0) + + +# --------------------------------------------------------------------------- +# Monthly (``trigger_type='monthly_time'`` — NEW in PR 1) +# --------------------------------------------------------------------------- + + +def test_monthly_picks_target_day_this_month_when_future(): + """It's the 5th, schedule fires on the 15th — next run is the + 15th of the current month.""" + now = _utc(2026, 5, 5, 12, 0) + result = next_run_at('monthly_time', + {'time': '09:00', 'day_of_month': 15, 'tz': 'UTC'}, + now) + assert result == _utc(2026, 5, 15, 9, 0) + + +def test_monthly_rolls_to_next_month_when_target_day_passed(): + """It's the 20th, schedule fires on the 15th — already past in + May, next run is June 15.""" + now = _utc(2026, 5, 20, 12, 0) + result = next_run_at('monthly_time', + {'time': '09:00', 'day_of_month': 15, 'tz': 'UTC'}, + now) + assert result == _utc(2026, 6, 15, 9, 0) + + +def test_monthly_clamps_to_last_day_when_month_too_short(): + """Schedule wants day 31; February has 28 (or 29). Clamp to the + LAST valid day of that month — running a day or two early in + short months is less surprising than silently skipping a month + entirely. Standard cron convention.""" + now = _utc(2026, 2, 1, 12, 0) # 2026 is not a leap year + result = next_run_at('monthly_time', + {'time': '09:00', 'day_of_month': 31, 'tz': 'UTC'}, + now) + # 2026 Feb has 28 days → run on the 28th instead. + assert result == _utc(2026, 2, 28, 9, 0) + + +def test_monthly_handles_leap_year_february(): + """2024 was a leap year — February has 29 days, so day-31 clamps + to the 29th, not the 28th.""" + now = _utc(2024, 2, 1, 12, 0) + result = next_run_at('monthly_time', + {'time': '09:00', 'day_of_month': 31, 'tz': 'UTC'}, + now) + assert result == _utc(2024, 2, 29, 9, 0) + + +def test_monthly_clamps_day_above_31_and_below_1(): + """Defensive — config values outside [1, 31] clamp to the nearest + valid bound rather than crashing the scheduler.""" + now = _utc(2026, 5, 5, 12, 0) + high = next_run_at('monthly_time', + {'time': '09:00', 'day_of_month': 99, 'tz': 'UTC'}, + now) + low = next_run_at('monthly_time', + {'time': '09:00', 'day_of_month': -5, 'tz': 'UTC'}, + now) + # 99 → clamped to 31 → May has 31 days → May 31st. + assert high == _utc(2026, 5, 31, 9, 0) + # -5 → clamped to 1 → next 1st is June 1 (May 1 already passed). + assert low == _utc(2026, 6, 1, 9, 0) + + +def test_monthly_rolls_year_at_december_to_january(): + """December 20, schedule fires on the 5th — next run is January 5 + of the FOLLOWING year, not month 13 of the current year.""" + now = _utc(2026, 12, 20, 12, 0) + result = next_run_at('monthly_time', + {'time': '09:00', 'day_of_month': 5, 'tz': 'UTC'}, + now) + assert result == _utc(2027, 1, 5, 9, 0) + + +def test_monthly_respects_user_tz(): + """Schedule wants the 1st of each month at 02:00 LA. ``now`` is + May 1 at 06:00 UTC = April 30 at 23:00 LA. So locally we haven't + hit May 1 02:00 LA yet → next run is May 1 02:00 LA = May 1 09:00 + UTC (PDT, UTC-7).""" + now = _utc(2026, 5, 1, 6, 0) + result = next_run_at('monthly_time', + {'time': '02:00', 'day_of_month': 1, + 'tz': 'America/Los_Angeles'}, + now) + assert result == _utc(2026, 5, 1, 9, 0) + + +# --------------------------------------------------------------------------- +# Result shape — every returned datetime must be aware UTC so the engine +# can serialise it to the DB ``next_run`` column without ambiguity. +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize('trigger_type,config', [ + ('schedule', {'interval': 1, 'unit': 'hours'}), + ('daily_time', {'time': '09:00', 'tz': 'America/Los_Angeles'}), + ('weekly_time', {'time': '09:00', 'days': ['mon'], 'tz': 'America/Los_Angeles'}), + ('monthly_time', {'time': '09:00', 'day_of_month': 15, 'tz': 'America/Los_Angeles'}), +]) +def test_result_is_always_aware_utc(trigger_type, config): + """Engine writes the result as a naive string to the DB but the + convention is "stored as UTC". Returning a naive datetime would + leak the caller's local tz into the column. Pin the contract: + every result has ``tzinfo`` and is at UTC offset zero.""" + now = _utc(2026, 5, 27, 12, 0) + result = next_run_at(trigger_type, config, now) + assert result is not None + assert result.tzinfo is not None + assert result.utcoffset() == timedelta(0) + + +def test_naive_now_utc_is_coerced_to_aware_utc(): + """Defensive — naive ``now_utc`` inputs are assumed UTC and the + result is still aware UTC. Matches the engine's convention + when parsing the DB ``next_run`` column.""" + naive_now = datetime(2026, 5, 27, 12, 0) + result = next_run_at('schedule', {'interval': 1, 'unit': 'hours'}, naive_now) + assert result == _utc(2026, 5, 27, 13, 0) + assert result.tzinfo is not None From 3e61105a1daa9e76f9ddfaca989b7788e7d105c6 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Wed, 27 May 2026 11:33:05 -0700 Subject: [PATCH 2/2] Close three review gaps before PR 1 ships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review pass on ec4a55c1 — applying the standing kettui-grade rule (see memory/feedback_always_build_kettui_grade.md). Three issues that would have surfaced on review: 1. Silent tz fallback to UTC ``_resolve_tz`` returned UTC when the IANA name was unknown — no log, no warning. User on a host without ``tzdata`` who configures ``America/Los_Angeles`` got schedules running silently at UTC offset with no way to debug. Now logs WARNING once per unknown name (deduped via ``_UNKNOWN_TZ_WARNED`` set so a misconfigured row doesn't spam every poll cycle) and the log line names BOTH real causes — typo or missing tzdata — so the user can fix from a single grep. 2. ``weeks`` unit drift from engine I added ``'weeks': 86400*7`` to ``_INTERVAL_MULTIPLIERS`` but the engine's existing ``_calc_delay_seconds`` only recognises minutes/hours/days. Until PR 2 collapses both paths through this function, any row whose config snuck through with ``unit='weeks'`` would get scheduled by the engine as 1-hour and by this function as 7-day — drift between two live implementations. Dropped ``weeks`` from the map to match the engine. Added a comment pinning the map to the engine's contract and a regression test that asserts ``unit='weeks'`` falls back to the same hours default the engine produces. 3. DST edge cases unverified The module docstring claims DST-aware via ``zoneinfo`` but no test pinned the spring-forward gap (02:30 LA on DST-Sunday doesn't exist) or fall-back ambiguity (01:30 LA on fall-Sunday happens twice). Three new tests: - ``test_dst_spring_forward_lands_after_the_gap`` — pins that the function doesn't crash + lands on a real instant past ``now``. - ``test_dst_fall_back_handles_ambiguous_local_time`` — pins zoneinfo's default-earlier-instant resolution for ambiguous local times (01:30 PDT vs 01:30 PST → picks PDT). - ``test_weekly_across_dst_boundary_keeps_local_wall_clock`` — pins that a "every Sunday at 09:00 LA" schedule keeps the local wall clock across the boundary even though the UTC equivalent shifts by an hour. This is the exact bug class that caused the May 2026 "next in 8h" tz mismatch. Also loosened ``tzdata==2026.2`` to ``tzdata>=2024.1``. IANA tz data changes a few times a year for real-world DST policy updates; pinning to one snapshot would freeze the app's tz knowledge to the build date and miss future government-mandated rule changes. 41 schedule tests pass (5 new); 240 across the full automation suite. Ruff clean. --- core/automation/schedule.py | 38 +++++--- requirements.txt | 5 +- tests/automation/test_schedule.py | 140 +++++++++++++++++++++++++++++- 3 files changed, 170 insertions(+), 13 deletions(-) diff --git a/core/automation/schedule.py b/core/automation/schedule.py index e3b8e37b..e76636fc 100644 --- a/core/automation/schedule.py +++ b/core/automation/schedule.py @@ -58,12 +58,16 @@ from __future__ import annotations from datetime import datetime, timedelta, timezone from typing import Any, Dict, Optional +from zoneinfo import ZoneInfo, ZoneInfoNotFoundError -try: - from zoneinfo import ZoneInfo, ZoneInfoNotFoundError -except ImportError: # pragma: no cover — zoneinfo ships with 3.9+ - ZoneInfo = None - ZoneInfoNotFoundError = Exception +from utils.logging_config import get_logger + +logger = get_logger("automation.schedule") + + +# Unknown-tz names already warned about in this process — avoids +# spamming the log on every poll cycle for the same misconfigured row. +_UNKNOWN_TZ_WARNED: set = set() # Weekday abbreviation → ``datetime.weekday()`` index (Mon=0..Sun=6). @@ -74,11 +78,16 @@ _WEEKDAY_MAP = { 'mon': 0, 'tue': 1, 'wed': 2, 'thu': 3, 'fri': 4, 'sat': 5, 'sun': 6, } +# Interval multipliers — kept aligned with the engine's existing +# ``_calc_delay_seconds`` in ``core/automation_engine.py``. Adding +# entries here without also updating the engine would silently drift: +# this function would honour the new unit while the live engine path +# defaults it to hours. Keep the maps in sync until PR 2 collapses the +# engine through this function. _INTERVAL_MULTIPLIERS = { 'minutes': 60, 'hours': 60 * 60, 'days': 60 * 60 * 24, - 'weeks': 60 * 60 * 24 * 7, } @@ -256,15 +265,24 @@ def _ensure_utc(dt: datetime) -> datetime: def _resolve_tz(name: Optional[str]): - """Look up an IANA tz by name. Falls back to UTC when zoneinfo - isn't available (3.8 / minimal images) or the name is unknown.""" + """Look up an IANA tz by name. Falls back to UTC when the name is + unknown — ``ZoneInfoNotFoundError`` is the symptom of either a + typo in the tz string or ``tzdata`` missing on the host. Logged + once per unknown name so the user can see WHY their schedule + isn't running in the timezone they configured.""" if not name: return timezone.utc - if ZoneInfo is None: - return timezone.utc try: return ZoneInfo(name) except ZoneInfoNotFoundError: + if name not in _UNKNOWN_TZ_WARNED: + _UNKNOWN_TZ_WARNED.add(name) + logger.warning( + "Unknown timezone %r — schedule will run against UTC. " + "Check the spelling (IANA format like 'America/Los_Angeles') " + "or install the `tzdata` package on minimal hosts.", + name, + ) return timezone.utc diff --git a/requirements.txt b/requirements.txt index 4a94d257..b1fda9f0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,7 +32,10 @@ psutil==7.2.2 # minimal Docker base images that ship without the system tz database. # Consumed by ``core/automation/schedule.py`` for daily / weekly / # monthly schedule next-run computation in the user's local timezone. -tzdata==2026.2 +# Loose-pinned because IANA tz data changes a few times a year for +# real-world DST policy updates; pinning to one snapshot would freeze +# the app's tz knowledge to the build date. +tzdata>=2024.1 # YouTube support -- unpinned; yt-dlp must track upstream releases to stay functional yt-dlp>=2026.3.17 diff --git a/tests/automation/test_schedule.py b/tests/automation/test_schedule.py index a73782ff..e7301a0f 100644 --- a/tests/automation/test_schedule.py +++ b/tests/automation/test_schedule.py @@ -61,15 +61,27 @@ def test_returns_none_for_non_dict_config(): ('minutes', 60), ('hours', 3600), ('days', 86400), - ('weeks', 86400 * 7), ]) def test_interval_units(unit, seconds_per_unit): - """Every supported unit scales the interval correctly.""" + """Every supported unit scales the interval correctly. Kept in + lockstep with the engine's existing ``_calc_delay_seconds`` map + — see _INTERVAL_MULTIPLIERS docstring.""" now = _utc(2026, 5, 27, 12, 0) result = next_run_at('schedule', {'interval': 3, 'unit': unit}, now) assert result == now + timedelta(seconds=3 * seconds_per_unit) +def test_interval_weeks_unit_falls_back_to_hours_matching_engine(): + """Engine's ``_calc_delay_seconds`` only recognises minutes / hours + / days — anything else defaults to hours. Drift between this helper + and the engine would silently mis-schedule rows whose config snuck + through with an unsupported unit. Pin the alignment until PR 2 + collapses both paths through this function.""" + now = _utc(2026, 5, 27, 12, 0) + # 'weeks' is not in our map; falls back to hours. + assert next_run_at('schedule', {'interval': 2, 'unit': 'weeks'}, now) == now + timedelta(hours=2) + + def test_interval_unknown_unit_defaults_to_hours(): """Backward compat with DB rows whose ``unit`` field is missing or an unrecognised value — engine's historic behaviour was to @@ -180,6 +192,130 @@ def test_daily_unknown_tz_falls_back_to_utc(): assert result == _utc(2026, 5, 27, 15, 0) +def test_unknown_tz_logs_warning_once(caplog): + """Silent fallback to UTC was a bug — user configures + 'America/Los_Angeles' but tzdata is missing → schedule runs at the + wrong hour with no log line. Log once per unknown name so the + misconfiguration is debuggable from a single grep, and don't spam + the log on every poll cycle for the same row.""" + import logging + from core.automation import schedule + schedule._UNKNOWN_TZ_WARNED.clear() # fresh state for the test + now = _utc(2026, 5, 27, 12, 0) + with caplog.at_level(logging.WARNING, logger='soulsync.automation.schedule'): + # Two calls with the same bad name — only ONE warning emitted. + next_run_at('daily_time', {'time': '09:00', 'tz': 'Bogus/Tz'}, now) + next_run_at('daily_time', {'time': '09:00', 'tz': 'Bogus/Tz'}, now) + matching = [r for r in caplog.records if 'Bogus/Tz' in r.getMessage()] + assert len(matching) == 1 + assert 'tzdata' in matching[0].getMessage().lower() + + +def test_unknown_tz_warning_includes_helpful_hint(): + """Log line must point the user at the two real causes: typo in + the IANA name, or missing tzdata on the host. Without that hint + the symptom (schedule running at UTC offset) is bewildering.""" + import logging + from core.automation import schedule + schedule._UNKNOWN_TZ_WARNED.clear() + caplog_records = [] + + class _Capture(logging.Handler): + def emit(self, record): + caplog_records.append(record.getMessage()) + + handler = _Capture() + logger_obj = logging.getLogger('soulsync.automation.schedule') + logger_obj.addHandler(handler) + try: + next_run_at('daily_time', {'time': '09:00', 'tz': 'Made/Up'}, + _utc(2026, 5, 27, 12, 0)) + finally: + logger_obj.removeHandler(handler) + assert any("'Made/Up'" in m for m in caplog_records) + assert any('IANA' in m for m in caplog_records) + + +# --------------------------------------------------------------------------- +# DST edge cases — pin that ``zoneinfo``'s default resolution handles +# spring-forward gap + fall-back ambiguity sensibly. Both transitions +# happen in the user's local tz, NOT in UTC, so the result UTC offset +# changes across the boundary. +# --------------------------------------------------------------------------- + + +def test_dst_spring_forward_lands_after_the_gap(): + """In Los Angeles, 2026-03-08 02:30 doesn't exist — clocks jump + from 02:00 PST directly to 03:00 PDT. A schedule for 02:30 daily + that fires through this transition must NOT raise and must land + on a real instant. ``zoneinfo``'s default resolution maps the + gap to the post-jump side (treating 02:30 as 03:30 PDT), so the + UTC equivalent shifts by an hour relative to non-DST days.""" + # 2026-03-08 00:00 UTC = 2026-03-07 16:00 PST (still PST). + # Schedule fires 02:30 LA daily. Next run on 03-07 was 02:30 PST + # = 10:30 UTC. We're querying after that → next run is 03-08 + # 02:30 LA, which falls in the spring-forward gap. zoneinfo + # resolves to 03:30 PDT = 10:30 UTC (offset already shifted to + # PDT for the rest of the day post-transition). + now = _utc(2026, 3, 8, 0, 0) + result = next_run_at('daily_time', + {'time': '02:30', 'tz': 'America/Los_Angeles'}, + now) + # Must be aware UTC, must NOT crash on the gap. + assert result is not None + assert result.tzinfo is not None + # Result is somewhere on 03-08 — exact time depends on zoneinfo's + # gap-resolution policy, but it must be on the right day and + # past ``now``. + assert result > now + assert result.date() == datetime(2026, 3, 8).date() + + +def test_dst_fall_back_handles_ambiguous_local_time(): + """2026-11-01 01:30 in Los Angeles happens TWICE (once at PDT + UTC-7, once at PST UTC-8 after the fall-back). A daily schedule + for 01:30 must resolve to ONE instant — ``zoneinfo``'s default + picks the first occurrence (PDT), so the UTC time is 08:30.""" + # 2026-11-01 00:00 UTC = 2026-10-31 17:00 PDT. + # Next 01:30 LA is 2026-11-01 — ambiguous, zoneinfo defaults to + # the earlier (PDT) instant: 08:30 UTC. + now = _utc(2026, 11, 1, 0, 0) + result = next_run_at('daily_time', + {'time': '01:30', 'tz': 'America/Los_Angeles'}, + now) + assert result is not None + # 01:30 PDT (UTC-7) → 08:30 UTC. + assert result == _utc(2026, 11, 1, 8, 30) + + +def test_weekly_across_dst_boundary_keeps_local_wall_clock(): + """User schedules "every Sunday at 09:00 LA". Crossing the + spring-forward DST boundary, the LOCAL wall clock stays at 09:00 + even though the UTC equivalent shifts by an hour. Pre-DST Sunday + 09:00 PST = 17:00 UTC; post-DST Sunday 09:00 PDT = 16:00 UTC.""" + # Pre-DST Sunday: 2026-03-01. + pre_dst = _utc(2026, 3, 1, 10, 0) # Sunday 02:00 PST already past 09:00? No — 02:00 < 09:00, so today still qualifies. + result_pre = next_run_at('weekly_time', + {'time': '09:00', 'days': ['sun'], + 'tz': 'America/Los_Angeles'}, + pre_dst) + # 09:00 PST = 17:00 UTC. + assert result_pre == _utc(2026, 3, 1, 17, 0) + + # Post-DST Sunday: 2026-03-15 (the 8th was DST switch day). + post_dst = _utc(2026, 3, 15, 10, 0) # 03:00 PDT — before 09:00. + result_post = next_run_at('weekly_time', + {'time': '09:00', 'days': ['sun'], + 'tz': 'America/Los_Angeles'}, + post_dst) + # 09:00 PDT = 16:00 UTC. + assert result_post == _utc(2026, 3, 15, 16, 0) + # Same local wall clock, different UTC — the kind of bug that + # caused the May 2026 "next in 8h" tz mismatch. + assert result_pre.hour == 17 + assert result_post.hour == 16 + + # --------------------------------------------------------------------------- # Weekly (``trigger_type='weekly_time'``) # ---------------------------------------------------------------------------