ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix(timezone): make profile-keyed tz cache atomic and add cron persistence regression

Browse Source 
Follow-up on the cherry-picked #92489 base: replace the four separate
process-global cache slots with one lock-guarded identity->(name, zone)
mapping so racing profile-scoped threads can never publish a mixed
identity/value pair (the P1 interleaving flagged in the #92489 review),
keep each profile's resolved zone hot across multiplex switches, and pin
the #97905 symptom with a real-store regression test: a foreign-process
tick (desktop multiplex ticker pattern) must persist next_run_at with the
job-owning profile's UTC offset.

Fixes #97905. Refs #88220, #92489.
This commit is contained in:
Teknium
2026-08-31 08:27:54 -07:00
parent 9d8a053ce5
commit 6cc4f3913d
3 changed files with 174 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_time.py
+34 -25
View File
@@ -15,9 +15,10 @@ crashes due to a bad timezone string.
import logging
import os
import threading
from datetime import datetime
from hermes_constants import get_config_path
from typing import Optional
from typing import Dict, Optional, Tuple
logger = logging.getLogger(__name__)
@@ -28,16 +29,21 @@ except ImportError:
from backports.zoneinfo import ZoneInfo # type: ignore[no-redef]
# Cached state, keyed to the active timezone source. This process can multiplex
# profiles by switching HERMES_HOME, so a single unkeyed value would leak the
# first profile's timezone into later profile-scoped work.
# Call reset_cache() to force re-resolution (e.g. after config changes).
_cached_tz: Optional[ZoneInfo] = None
_cached_tz_name: Optional[str] = None
_cache_resolved: bool = False
_cache_identity: Optional[tuple[str, str]] = None
# profiles by switching HERMES_HOME (context override or env), so a single
# unkeyed process-global value would leak the first profile's timezone into
# later profile-scoped work (e.g. the desktop multiplex cron ticker persisting
# another profile's ``next_run_at`` under the backend's own timezone).
#
# Entries are published atomically under ``_cache_lock`` as one
# ``identity -> (name, ZoneInfo | None)`` mapping, so two profile-scoped
# threads racing through resolution can never publish a mixed
# identity/value pair. Each profile's resolved zone stays hot across
# multiplex switches. Call reset_cache() after in-place config changes.
_cache_lock = threading.Lock()
_tz_cache: Dict[Tuple[str, str], Tuple[str, Optional[ZoneInfo]]] = {}
def _timezone_cache_identity() -> tuple[str, str]:
def _timezone_cache_identity() -> Tuple[str, str]:
"""Return the active source identity for the timezone cache."""
tz_env = os.getenv("HERMES_TIMEZONE", "").strip()
if tz_env:
@@ -107,17 +113,25 @@ def _get_zoneinfo(name: str) -> Optional[ZoneInfo]:
def get_timezone() -> Optional[ZoneInfo]:
"""Return the active profile's configured ZoneInfo, or None (server-local).
The cache is isolated by the active environment override or profile config
path. Call ``reset_cache()`` after editing the active config in place.
The cache is isolated by the active timezone source — the explicit
``HERMES_TIMEZONE`` override or the active profile's config path — so a
process that multiplexes profiles (desktop cron ticker, multiplex
gateway) never reuses another profile's timezone. Call ``reset_cache()``
after editing the active config in place.
"""
global _cached_tz, _cached_tz_name, _cache_resolved, _cache_identity
cache_identity = _timezone_cache_identity()
if not _cache_resolved or _cache_identity != cache_identity:
_cached_tz_name = _resolve_timezone_name()
_cached_tz = _get_zoneinfo(_cached_tz_name)
_cache_identity = cache_identity
_cache_resolved = True
return _cached_tz
with _cache_lock:
entry = _tz_cache.get(cache_identity)
if entry is not None:
return entry[1]
# Resolve outside the lock (config file I/O); publish atomically below.
name = _resolve_timezone_name()
tz = _get_zoneinfo(name)
with _cache_lock:
# First writer wins so concurrent resolvers of the SAME identity
# converge on one ZoneInfo object; a different identity's write can
# never be mixed into this one — the (name, tz) pair is one value.
return _tz_cache.setdefault(cache_identity, (name, tz))[1]
def reset_cache() -> None:
@@ -127,11 +141,8 @@ def reset_cache() -> None:
config edit or ``HERMES_TIMEZONE`` update) to force ``get_timezone()`` /
``now()`` to read the new value instead of the value cached at first use.
"""
global _cached_tz, _cached_tz_name, _cache_resolved, _cache_identity
_cached_tz = None
_cached_tz_name = None
_cache_resolved = False
_cache_identity = None
with _cache_lock:
_tz_cache.clear()
def now() -> datetime:
@@ -146,5 +157,3 @@ def now() -> datetime:
return datetime.now(tz)
# No timezone configured — use server-local (still tz-aware)
return datetime.now().astimezone()
tests/cron/test_cron_next_run_profile_timezone.py
+80
View File
@@ -0,0 +1,80 @@
"""Regression test for #97905 / carrier PR #92489.
A multiplex ticker (desktop dashboard backend, multiplex gateway) resolves
``hermes_time`` under the process's own startup profile, then ticks OTHER
profiles' cron stores via ``set_hermes_home_override()`` + ``use_cron_store()``.
Before the profile-keyed timezone cache, the first profile's resolved zone
was process-global, so ``compute_next_run`` / ``create_job`` / ``mark_job_run``
persisted ``next_run_at`` into the ticked profile's jobs.json with the FOREIGN
process's UTC offset — e.g. ``0 14 * * *`` for an America/New_York profile
stored as ``14:00+00:00``, which becomes due at 10:00 ET and fires four hours
early (#97905; single-profile sibling report #88220).
The invariant pinned here: ``next_run_at`` written during a foreign-process
tick carries the JOB-OWNING profile's configured UTC offset.
"""
from datetime import datetime
from pathlib import Path
from zoneinfo import ZoneInfo
import pytest
import hermes_time
@pytest.fixture(autouse=True)
def _fresh_tz_cache(monkeypatch):
monkeypatch.delenv("HERMES_TIMEZONE", raising=False)
hermes_time.reset_cache()
yield
hermes_time.reset_cache()
def test_foreign_process_tick_persists_owning_profile_offset(
tmp_path, monkeypatch
):
"""next_run_at persisted during a multiplex tick uses the ticked
profile's timezone, not the backend process's."""
backend_home = tmp_path / "backend" # desktop backend's own profile: UTC
profile_a = tmp_path / "north-caribbean" # job-owning profile: Eastern
backend_home.mkdir()
profile_a.mkdir()
(backend_home / "config.yaml").write_text(
"timezone: UTC\n", encoding="utf-8"
)
(profile_a / "config.yaml").write_text(
"timezone: America/New_York\n", encoding="utf-8"
)
monkeypatch.setenv("HERMES_HOME", str(backend_home))
# Backend process resolves its own timezone first (process startup).
assert hermes_time.now().utcoffset() == datetime.now(
ZoneInfo("UTC")
).utcoffset()
from hermes_constants import (
reset_hermes_home_override,
set_hermes_home_override,
)
from cron.jobs import create_job, load_jobs, use_cron_store
# Exactly the scoping the multiplex ticker applies per profile
# (cron/scheduler_provider.py::_tick_profiles).
token = set_hermes_home_override(str(profile_a))
try:
with use_cron_store(profile_a):
create_job(name="daily-2pm", prompt="x", schedule="0 14 * * *")
job = load_jobs()[0]
finally:
reset_hermes_home_override(token)
next_run = datetime.fromisoformat(job["next_run_at"])
expected_offset = datetime.now(ZoneInfo("America/New_York")).utcoffset()
assert next_run.utcoffset() == expected_offset, (
f"next_run_at {job['next_run_at']} persisted with foreign offset "
f"{next_run.utcoffset()} instead of the owning profile's "
f"{expected_offset} — fires (offset delta) early (#97905)"
)
# And the wall clock honors the cron expression in the profile's zone.
assert (next_run.hour, next_run.minute) == (14, 0)
tests/test_timezone.py
+60 -5
View File
@@ -21,11 +21,8 @@ import hermes_time
def _reset_hermes_time_cache():
"""Reset the hermes_time module cache (replacement for removed reset_cache)."""
hermes_time._cached_tz = None
hermes_time._cached_tz_name = None
hermes_time._cache_resolved = False
hermes_time._cache_identity = None
"""Reset the hermes_time module cache."""
hermes_time.reset_cache()
# =========================================================================
@@ -106,6 +103,64 @@ class TestGetTimezone:
monkeypatch.setenv("HERMES_HOME", str(second_home))
assert str(hermes_time.get_timezone()) == "America/New_York"
# Switching BACK must return the first profile's zone (per-identity
# entries stay hot; no single-slot ping-pong).
monkeypatch.setenv("HERMES_HOME", str(first_home))
assert str(hermes_time.get_timezone()) == "Asia/Tokyo"
def test_concurrent_profile_resolution_never_mixes_zones(
self, tmp_path, monkeypatch
):
"""Racing profile-scoped threads must never observe a foreign zone.
The multiplex cron ticker lets profile-A work (mark_job_run /
compute_next_run) overlap the ticker advancing to profile B. The
cache publication must be atomic per identity: identity A can never
be paired with profile B's ZoneInfo (#97905 review finding on
PR #92489).
"""
import threading
from hermes_constants import (
reset_hermes_home_override,
set_hermes_home_override,
)
zones = {"a": "Asia/Tokyo", "b": "America/New_York"}
homes = {}
for key, zone in zones.items():
home = tmp_path / key
home.mkdir()
(home / "config.yaml").write_text(
f"timezone: {zone}\n", encoding="utf-8"
)
homes[key] = home
monkeypatch.delenv("HERMES_TIMEZONE", raising=False)
errors = []
barrier = threading.Barrier(2)
def worker(key: str) -> None:
barrier.wait()
for _ in range(200):
token = set_hermes_home_override(str(homes[key]))
try:
tz = hermes_time.get_timezone()
if str(tz) != zones[key]:
errors.append((key, str(tz)))
return
finally:
reset_hermes_home_override(token)
threads = [
threading.Thread(target=worker, args=(key,)) for key in zones
]
for t in threads:
t.start()
for t in threads:
t.join()
assert not errors, f"foreign timezone observed: {errors}"
# =========================================================================