Skip to content

fix(cron): handle naive legacy timestamps in due-job checks#1308

Closed
0xNyk wants to merge 1 commit intoNousResearch:mainfrom
0xNyk:fix/cron-naive-timestamps
Closed

fix(cron): handle naive legacy timestamps in due-job checks#1308
0xNyk wants to merge 1 commit intoNousResearch:mainfrom
0xNyk:fix/cron-naive-timestamps

Conversation

@0xNyk
Copy link
Contributor

@0xNyk 0xNyk commented Mar 14, 2026

Replaces #929 (rebased to resolve merge conflicts with upstream main).

Summary

  • Fix _ensure_aware() to interpret naive datetimes as system-local wall time (via datetime.now().astimezone().tzinfo), then convert to Hermes timezone — instead of blindly stamping with replace(tzinfo=hermes_tz) which shifted the absolute time
  • Normalize already-aware datetimes to Hermes timezone via .astimezone(target_tz) for consistent comparisons in get_due_jobs()
  • Add tests covering: naive→aware absolute time preservation, aware→hermes normalization, cross-timezone due-job detection, and the original system-ahead-of-hermes regression scenario

Test plan

  • pytest tests/test_timezone.py — all 22 tests pass (including 4 new ones)

Cherry-picked from PR NousResearch#807 by 0xNyk, rebased onto current main.

When HERMES_TIMEZONE differs from system local timezone, naive (legacy)
timestamps were misinterpreted by _ensure_aware() — it stamped them
with the Hermes timezone via replace(tzinfo=...), but they were created
using datetime.now() (system local time). This could shift the absolute
time and cause overdue jobs to appear not-due.

Fix: interpret naive datetimes as system-local wall time first, then
convert to Hermes timezone. Already-aware datetimes are normalized to
Hermes timezone for consistent comparisons.

Added 3 tests:
- _ensure_aware preserves absolute time for naive datetimes
- _ensure_aware normalizes aware datetimes to Hermes tz
- get_due_jobs detects naive past timestamps as due even when Hermes tz
  is far behind system local tz (the scenario from NousResearch#806)

Fixes NousResearch#806

Co-authored-by: Nyk <0xnykcd@googlemail.com>
teknium1 pushed a commit that referenced this pull request Mar 14, 2026
Cherry-picked from PR #1308 by 0xNyk.

Adds an end-to-end regression test covering a Hermes timezone far behind
system local time (Pacific/Midway, UTC-11) to ensure legacy naive cron
timestamps are still recognized as due under large timezone mismatches.
teknium1 added a commit that referenced this pull request Mar 14, 2026
Merging the remaining useful regression coverage from #1308 on top of the already-merged cron fix in #949.
@teknium1
Copy link
Contributor

Merged the remaining useful coverage via PR #1319 on top of the already-merged fix from PR #949. Your extra cross-timezone regression test was salvaged onto current main with authorship preserved here. Thanks.

@teknium1 teknium1 closed this Mar 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants