Merge pull request #50034 from NousResearch/salvage/cron-tz-offset-repair

fix(cron): repair migrated timezone offsets to prevent double-fire

cron/jobs.py

@@ -409,6 +409,31 @@ def _ensure_aware(dt: datetime) -> datetime:
     return dt.astimezone(target_tz)
 
 
+def _timezone_offset_mismatch(stored: datetime, current: datetime) -> bool:
+    """Return True when a stored aware timestamp uses a different UTC offset.
+
+    Naive stored timestamps return False: they carry no offset to compare, and
+    are normalized by ``_ensure_aware`` instead — they intentionally never take
+    the offset-repair path.
+    """
+    if stored.tzinfo is None or current.tzinfo is None:
+        return False
+    return stored.utcoffset() != current.utcoffset()
+
+
+def _stored_wall_clock_is_future(stored: datetime, current: datetime) -> bool:
+    """Return True when the stored local wall-clock time has not arrived yet.
+
+    Cron schedules express local wall-clock intent. If Hermes/system local time
+    changes after next_run_at was persisted, an old offset can make a future
+    wall-clock run look due at the converted absolute time (for example
+    21:00+10 becomes 13:00+02). Comparing naive wall-clock values lets us
+    distinguish that migration case from a genuinely missed run whose scheduled
+    wall time has already passed.
+    """
+    return stored.replace(tzinfo=None) > current.replace(tzinfo=None)
+
+
 def _recoverable_oneshot_run_at(
     schedule: Dict[str, Any],
     now: datetime,
@@ -1276,10 +1301,50 @@ def _get_due_jobs_locked() -> List[Dict[str, Any]]:
                     needs_save = True
                     break
 
-        next_run_dt = _ensure_aware(datetime.fromisoformat(next_run))
+        raw_next_run_dt = datetime.fromisoformat(next_run)
+        schedule = job.get("schedule", {})
+        kind = schedule.get("kind")
+
+        next_run_dt = _ensure_aware(raw_next_run_dt)
+        # Migration repair: a cron job persists next_run_at as an absolute
+        # instant, but the cron expr describes local wall-clock intent. If the
+        # configured/system timezone changed after persistence, the stored
+        # instant's offset no longer matches now's, and its converted time can
+        # look due hours early (21:00+10 -> 13:00+02). When the stored *wall
+        # clock* is still in the future, recompute from the schedule so we fire
+        # at the intended local time instead of early-then-again.
+        #
+        # TRADE-OFF: this cannot distinguish a config/host TZ migration from a
+        # legitimate DST offset change. A DST boundary that satisfies all four
+        # conditions will recompute (and thus SKIP the pending occurrence, no
+        # catch-up) rather than fire it. Accepted: in the pure-migration case
+        # the recompute lands on the same wall-clock time later the same period,
+        # and DST-boundary collisions with a still-future stored wall clock are
+        # rare relative to the double-fire bug this prevents (#28934).
+        if (
+            kind == "cron"
+            and next_run_dt <= now
+            and _timezone_offset_mismatch(raw_next_run_dt, now)
+            and _stored_wall_clock_is_future(raw_next_run_dt, now)
+        ):
+            new_next = compute_next_run(schedule, now.isoformat())
+            if new_next:
+                logger.info(
+                    "Job '%s' next_run_at offset changed (%s -> %s). "
+                    "Recomputing cron run to preserve local wall-clock intent: %s",
+                    job.get("name", job["id"]),
+                    raw_next_run_dt.utcoffset(),
+                    now.utcoffset(),
+                    new_next,
+                )
+                for rj in raw_jobs:
+                    if rj["id"] == job["id"]:
+                        rj["next_run_at"] = new_next
+                        needs_save = True
+                        break
+                continue
+
         if next_run_dt <= now:
-            schedule = job.get("schedule", {})
-            kind = schedule.get("kind")
 
             # For recurring jobs, check if the scheduled time is stale
             # (gateway was down and missed the window). Fast-forward to

tests/cron/test_jobs.py

@@ -849,6 +849,151 @@ class TestGetDueJobs:
         assert recovered_dt > now
 
 
+    def test_cron_next_run_offset_migration_is_rescheduled_not_fired(self, tmp_cron_dir, monkeypatch):
+        current_tz = timezone(timedelta(hours=2))
+        now = datetime(2026, 5, 19, 13, 2, 0, tzinfo=current_tz)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+
+        # A 21:00 cron was stored while Hermes/system local time was UTC+10.
+        # After the host moves to UTC+02, that absolute timestamp converts to
+        # 13:00+02.  At 13:02+02 the old code considered it due and fired, even
+        # though the user's local wall-clock cron intent is still 21:00.
+        save_jobs(
+            [{
+                "id": "cron-tz-migrate",
+                "name": "Migrated local cron",
+                "prompt": "...",
+                "schedule": {"kind": "cron", "expr": "0 21 * * 2", "display": "0 21 * * 2"},
+                "schedule_display": "0 21 * * 2",
+                "repeat": {"times": None, "completed": 0},
+                "enabled": True,
+                "state": "scheduled",
+                "paused_at": None,
+                "paused_reason": None,
+                "created_at": "2026-05-12T21:00:00+10:00",
+                "next_run_at": "2026-05-19T21:00:00+10:00",
+                "last_run_at": "2026-05-12T21:00:00+10:00",
+                "last_status": "ok",
+                "last_error": None,
+                "deliver": "local",
+                "origin": None,
+            }]
+        )
+
+        assert get_due_jobs() == []
+        repaired = datetime.fromisoformat(get_job("cron-tz-migrate")["next_run_at"])
+        assert repaired == datetime(2026, 5, 19, 21, 0, 0, tzinfo=current_tz)
+
+    def test_cron_offset_migration_does_not_repair_already_passed_wall_time(self, tmp_cron_dir, monkeypatch):
+        current_tz = timezone(timedelta(hours=2))
+        now = datetime(2026, 5, 19, 13, 2, 0, tzinfo=current_tz)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+
+        save_jobs(
+            [{
+                "id": "cron-tz-missed",
+                "name": "Migrated missed cron",
+                "prompt": "...",
+                "schedule": {"kind": "cron", "expr": "0 9 * * 2", "display": "0 9 * * 2"},
+                "schedule_display": "0 9 * * 2",
+                "repeat": {"times": None, "completed": 0},
+                "enabled": True,
+                "state": "scheduled",
+                "paused_at": None,
+                "paused_reason": None,
+                "created_at": "2026-05-12T09:00:00+10:00",
+                "next_run_at": "2026-05-19T09:00:00+10:00",
+                "last_run_at": "2026-05-12T09:00:00+10:00",
+                "last_status": "ok",
+                "last_error": None,
+                "deliver": "local",
+                "origin": None,
+            }]
+        )
+
+        # The wall-clock time has already passed, so this follows the existing
+        # stale-run fast-forward behavior instead of the timezone-migration
+        # repair path for future wall-clock runs.
+        assert get_due_jobs() == []
+        repaired = datetime.fromisoformat(get_job("cron-tz-missed")["next_run_at"])
+        assert repaired == datetime(2026, 5, 26, 9, 0, 0, tzinfo=current_tz)
+
+    def test_same_tz_due_cron_still_fires(self, tmp_cron_dir, monkeypatch):
+        """Guard must NOT over-fire: a due cron in the SAME offset fires normally."""
+        current_tz = timezone(timedelta(hours=2))
+        now = datetime(2026, 5, 19, 21, 0, 30, tzinfo=current_tz)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+        save_jobs([{
+            "id": "cron-same-tz", "name": "same tz", "prompt": "...",
+            "schedule": {"kind": "cron", "expr": "0 21 * * 2", "display": "0 21 * * 2"},
+            "schedule_display": "0 21 * * 2",
+            "repeat": {"times": None, "completed": 0},
+            "enabled": True, "state": "scheduled", "paused_at": None, "paused_reason": None,
+            "created_at": "2026-05-12T21:00:00+02:00",
+            "next_run_at": "2026-05-19T21:00:00+02:00",  # same offset as now
+            "last_run_at": "2026-05-12T21:00:00+02:00",
+            "last_status": "ok", "last_error": None, "deliver": "local", "origin": None,
+        }])
+        # offset matches -> guard skips -> the genuinely-due job is returned to fire.
+        due = get_due_jobs()
+        assert [j["id"] for j in due] == ["cron-same-tz"]
+
+    def test_interval_job_with_stale_offset_is_unaffected(self, tmp_cron_dir, monkeypatch):
+        """The offset-repair guard is cron-only; interval jobs never take it.
+
+        A stale-offset interval job whose converted instant is well past the
+        grace window is handled by the pre-existing stale fast-forward path
+        (not the cron repair path). Verify it fast-forwards via interval math
+        (next = now + interval), proving the cron-only guard didn't touch it.
+        """
+        current_tz = timezone(timedelta(hours=2))
+        now = datetime(2026, 5, 19, 13, 2, 0, tzinfo=current_tz)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+        save_jobs([{
+            "id": "interval-stale-tz", "name": "interval", "prompt": "...",
+            "schedule": {"kind": "interval", "minutes": 60, "display": "every 1h"},
+            "schedule_display": "every 1h",
+            "repeat": {"times": None, "completed": 0},
+            "enabled": True, "state": "scheduled", "paused_at": None, "paused_reason": None,
+            "created_at": "2026-05-19T10:00:00+10:00",
+            "next_run_at": "2026-05-19T12:00:00+10:00",  # stale offset, instant 04:00+02 (well past)
+            "last_run_at": "2026-05-19T11:00:00+10:00",
+            "last_status": "ok", "last_error": None, "deliver": "local", "origin": None,
+        }])
+        get_due_jobs()
+        # The cron-only repair path would have produced a cron occurrence; instead
+        # the interval stale fast-forward recomputes next = now + 60m (interval
+        # math), confirming the guard did not intercept this interval job.
+        nr = datetime.fromisoformat(get_job("interval-stale-tz")["next_run_at"])
+        assert nr == now + timedelta(minutes=60)
+
+    def test_offset_migration_at_wall_clock_equal_now_falls_through(self, tmp_cron_dir, monkeypatch):
+        """Boundary: stored wall-clock == now wall-clock (strict >) does NOT take
+        the repair path — it falls through to the existing due/fast-forward logic."""
+        current_tz = timezone(timedelta(hours=2))
+        now = datetime(2026, 5, 19, 13, 0, 0, tzinfo=current_tz)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+        save_jobs([{
+            "id": "cron-wall-equal", "name": "wall equal", "prompt": "...",
+            "schedule": {"kind": "cron", "expr": "0 13 * * 2", "display": "0 13 * * 2"},
+            "schedule_display": "0 13 * * 2",
+            "repeat": {"times": None, "completed": 0},
+            "enabled": True, "state": "scheduled", "paused_at": None, "paused_reason": None,
+            "created_at": "2026-05-12T13:00:00+10:00",
+            # stored naive wall-clock 13:00 == now naive wall-clock 13:00 -> strict > is False
+            "next_run_at": "2026-05-19T13:00:00+10:00",
+            "last_run_at": "2026-05-12T13:00:00+10:00",
+            "last_status": "ok", "last_error": None, "deliver": "local", "origin": None,
+        }])
+        # _stored_wall_clock_is_future is strict (>), so 13:00 == 13:00 is False
+        # -> repair guard skipped -> existing logic handles it (does not raise).
+        get_due_jobs()  # must not raise / must not take the repair branch
+        # next_run_at must NOT have been rewritten to a future cron occurrence by
+        # the repair path (it either fires or fast-forwards via the normal path).
+        nr = get_job("cron-wall-equal")["next_run_at"]
+        assert nr is None or datetime.fromisoformat(nr).utcoffset() == now.utcoffset() or "+10:00" in nr
+
+
 class TestEnabledToolsets:
     def test_enabled_toolsets_stored(self, tmp_cron_dir):
         job = create_job(prompt="monitor", schedule="every 1h", enabled_toolsets=["web", "terminal"])