fix(cron): record the timezone a cron schedule is evaluated in - #88581
Open
JoaoMarcos44 wants to merge 2 commits into
Open
fix(cron): record the timezone a cron schedule is evaluated in#88581JoaoMarcos44 wants to merge 2 commits into
JoaoMarcos44 wants to merge 2 commits into
Conversation
A cron expression is local wall-clock intent ("run at 14:30"), but
`next_run_at` is persisted as an absolute instant. Which zone that wall
clock belonged to was implicit: every process re-resolved it from
HERMES_TIMEZONE / config.yaml at the moment it read or wrote the job.
That makes a persisted `next_run_at` ambiguous as soon as two readers
disagree. `hermes_time` cached the zone for the whole process lifetime,
so a gateway that booted before `timezone: Asia/Shanghai` was added kept
running on server-local time while every freshly spawned CLI / web worker
resolved Asia/Shanghai immediately. The same string then meant two
different instants, and the due check silently switched between
"absolute instant" and "naive wall clock" semantics depending on an
offset comparison — moving the job by the whole UTC offset (8h) at the
next gateway restart, firing it early and swallowing the real run
(NousResearch#88220).
Root cause fix, in two parts:
1. `schedule["tz"]` records the IANA zone a cron expression is evaluated
in, stamped at parse time. `compute_next_run` anchors croniter to that
zone, so gateway, CLI and web UI all compute the same instant no
matter what each of them resolves. Legacy jobs are adopted by the
configured zone on first sight; installs with no `timezone:` set store
no `tz` and keep the previous server-local behaviour byte for byte.
2. The offset-delta migration repair from NousResearch#28934 is replaced, for stamped
jobs, by an explicit zone-identity rebase: when `timezone:` actually
changes, the job is re-anchored once to the same wall clock in the new
zone and re-stamped. Because zone identity does not change across a
DST boundary, DST no longer masquerades as a migration and no longer
skips a pending occurrence. Unstamped jobs still take the old
heuristic, so its four existing regression tests keep passing.
Also fixes a second, independent bug this surfaced: croniter walks a
*fixed* UTC offset taken from its base datetime, so handing it a
zone-aware base drifts by the DST delta across a transition
(`0 9 * * *` based on 2026-03-28T09:00+01:00 returns
2026-03-29T08:00+02:00 — an hour early). Stamped schedules are evaluated
on the naive local wall clock and localized afterwards, which keeps 09:00
meaning 09:00 on both sides of the boundary. In a virtual-clock
simulation this cost a daily job an extra fire on every spring-forward
day and a lost fire on every fall-back day, with no timezone change
involved at all.
`hermes_time` now re-resolves the configured zone at most once a minute
instead of pinning it for the process lifetime, so a config edit no
longer needs a gateway restart to take effect — and processes converge
instead of diverging. Measured cost: `now()` 0.238us -> 0.357us per call;
`get_due_jobs()` over 1000 jobs 11.19ms -> 10.27ms.
Verified with a virtual-clock ticker simulation over 47 scenarios
(13 IANA zones incl. +05:45/+08:45/+14:00 offsets, 5 cron expressions,
6 DST boundaries north and south, and restarts that flip the reader's
resolved zone): 31 scenarios fail before this change, 0 after.
Fixes NousResearch#88220
… tests _configured_tz_name() swallowed every exception with no trace, and the new bounded-TTL zone cache in hermes_time.py had zero direct test coverage (existing cron tests bypass it via monkeypatch).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A cron expression is local wall-clock intent ("run at 14:30"), but
next_run_atis persisted as an absolute instant. Which zone that wall clock belonged to was never recorded — every process re-resolved it fromHERMES_TIMEZONE/config.yamlat the moment it read or wrote the job.That makes a persisted
next_run_atambiguous the instant two readers disagree.hermes_timecached the resolved zone for the whole process lifetime, so a gateway that booted beforetimezone: Asia/Shanghaiwas added kept running on server-local time, while every freshly spawned CLI / web worker picked the new zone up immediately. The same string then meant two different instants, and the due check silently flipped between "absolute instant" and "naive wall clock" semantics depending on an offset comparison — moving the job by the whole UTC offset (8h) at the next gateway restart, firing it early and swallowing the genuine run.Fixes #88220.
%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#8b0000', 'mainBkg': '#0a0204', 'primaryTextColor': '#ffccd5', 'primaryBorderColor': '#ff0038', 'lineColor': '#ff0038'}}}%% graph TD subgraph BEFORE["Before - the zone is implicit"] A1["Gateway process<br/>booted before timezone: was set<br/>resolves server-local"] -->|writes / reads| S1[("jobs.json<br/>next_run_at: 2026-08-17T14:30+08:00")] A2["CLI / web worker<br/>resolves Asia/Shanghai"] -->|writes / reads| S1 S1 --> D1{"Due check<br/>offset label == my offset?"} D1 -->|"no - reinterpret as wall clock"| X1["Fires 8h early<br/>real run swallowed"] D1 -->|"yes - absolute instant"| X2["Different answer<br/>same file, other process"] end subgraph AFTER["After - the schedule carries its own zone"] B1["Gateway process"] --> S2[("jobs.json<br/>schedule.tz: Asia/Shanghai<br/>next_run_at: 2026-08-17T14:30+08:00")] B2["CLI / web worker"] --> S2 S2 --> C1["croniter evaluated on the<br/>naive wall clock of schedule.tz"] C1 --> R1["One instant, every reader"] S2 --> Z{"configured zone<br/>!= schedule.tz ?"} Z -->|"yes - operator changed timezone:"| Z1["Re-anchor once to the same<br/>wall clock in the new zone, re-stamp"] Z -->|"no - DST only moved the offset"| R1 endIs it really a bug?
Yes, and it is two bugs. Reproduced with a virtual-clock ticker simulation that drives the real
get_due_jobs()/mark_job_run()pair minute by minute and compares the fire log against the cron expression evaluated independently in the job's own zone.mainSample from
main,Asia/Shanghai,30 14 * * *: fired at14:30Zfor five days running; the correct instants (06:30Z) were all missed — the exact 8-hour shift in the report.Second bug this surfaced
croniterwalks a fixed UTC offset taken from its base datetime, so handing it a zone-aware base drifts by the DST delta across a transition:In the simulation this costs a daily job an extra fire on every spring-forward day and a lost fire on every fall-back day, with no timezone divergence involved. It affects every DST-observing install today.
Root cause fix
1. The schedule records the zone it is evaluated in.
parse_schedulestampsschedule["tz"]with the configured IANA zone;compute_next_runanchors croniter to it, evaluating the expression on that zone's naive wall clock and localizing the result. Gateway, CLI and web UI now compute the same instant regardless of what each of them resolves — and 09:00 stays 09:00 across a DST boundary.2. Zone-identity rebase replaces the offset-delta heuristic. When
timezone:genuinely changes, the job is re-anchored once to the same wall clock in the new zone and re-stamped (the #28934 intent, now triggered exactly). Zone identity does not change across DST, so DST no longer masquerades as a migration and no longer skips a pending occurrence.3.
hermes_timestops pinning the zone for the process lifetime. It re-resolves at most once a minute, so a config edit no longer needs a gateway restart to take effect and divergent processes converge instead of drifting apart.Why not the alternatives
f1f36b3bae(#28934) already rejected normalize-to-UTC (#28951) and rebase-and-match (#28985), and shipped the offset-delta heuristic with an explicit trade-off comment: "this cannot distinguish a config/host TZ migration from a legitimate DST offset change". Both rejected approaches, and the heuristic that won, share one blind spot — none of them record the zone, so all three have to guess from an offset. This PR removes the guess instead of tuning it; the heuristic's four regression tests still pass unchanged because unstamped jobs still take it.Backward compatibility
schedule.tzis additive and optional;parse_scheduleis the only place that builds a schedule dict, and nothing validates its keys strictly.timezone:set store notzand behave byte for byte as before.Performance
mainhermes_time.now()now()forced to re-resolve every call (TTL = 0)get_due_jobs(), 1000 stamped cron jobsThe configured zone is resolved once per due scan, not per job, so the ticker's per-job cost is a string comparison.
Test plan
tests/cron/test_cron_schedule_timezone_88220.py— 48 new tests: zone stamping, cross-process determinism, the Cron jobs fire 8 hours early after gateway restart (next_run_at persisted with +08:00 label on UTC wall-clock) #88220 reproducer end to end, zone-change rebase + idempotence, DST tick simulations (Berlin / Auckland / Los Angeles, both directions), a 13-zone i18n matrix including+05:30,+05:45,+08:45,+14:00and-11:00, and legacy-job adoption. 43 of 48 fail onmain.tests/cron+tests/test_timezone.py: 790 passed, 34 skipped. The 7 failures are the pre-existing Windows-only POSIX-permission / tilde-expansion tests — identical on a clean checkout.tests/tools: 36 pre-existing Windows-only failures on bothmainand this branch (unchanged).ruff checkclean on every changed file.Infographic :