Commit graph

2 commits

Author SHA1 Message Date
Broque Thomas
3e61105a1d Close three review gaps before PR 1 ships
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.
2026-05-27 11:33:05 -07:00
Broque Thomas
ec4a55c104 Add next_run_at pure function for Auto-Sync schedule types (PR 1/4)
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: '<IANA>'}`` — 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.
2026-05-27 11:15:47 -07:00