Merge pull request #50112 from NousResearch/salvage/f5-cron-storage-root

fix(cron): anchor cron storage at the default root home (#32091)

cron/jobs.py

@@ -31,7 +31,7 @@ except ImportError:  # pragma: no cover - non-Windows
     msvcrt = None
 from datetime import datetime, timedelta
 from pathlib import Path
-from hermes_constants import get_hermes_home
+from hermes_constants import get_default_hermes_root, get_hermes_home
 from typing import Optional, Dict, List, Any, Union
 
 logger = logging.getLogger(__name__)
@@ -49,7 +49,7 @@ except ImportError:
 # Configuration
 # =============================================================================
 
-HERMES_DIR = get_hermes_home().resolve()
+HERMES_DIR = get_default_hermes_root().resolve()
 CRON_DIR = HERMES_DIR / "cron"
 JOBS_FILE = CRON_DIR / "jobs.json"
 # Heartbeat file the in-process ticker touches on every loop iteration. The
@@ -615,10 +615,44 @@ def get_ticker_success_age() -> Optional[float]:
 # Job CRUD Operations
 # =============================================================================
 
+_WARNED_ORPHAN_STORE = False
+
+
+def _warn_if_orphaned_profile_store() -> None:
+    """Loudly warn (once) if the root store is empty but a profile-local
+    jobs.json exists from before #32091's root-anchoring fix.
+
+    Such a file is now unreachable (the store anchors at the default root, not
+    the active profile). The jobs in it were already orphaned pre-fix (the
+    profile-less gateway never read them), so this is not a regression — but a
+    user who could SEE them in `cron list` under their profile would otherwise
+    find them silently gone. Point them at the path instead of failing silent.
+    """
+    global _WARNED_ORPHAN_STORE
+    if _WARNED_ORPHAN_STORE:
+        return
+    try:
+        active = get_hermes_home().resolve()
+        if active == HERMES_DIR:
+            return  # not in a profile; nothing could be orphaned
+        legacy = active / "cron" / "jobs.json"
+        if legacy.exists():
+            _WARNED_ORPHAN_STORE = True
+            logger.warning(
+                "Cron jobs now live at %s (shared across profiles). A legacy "
+                "profile-local store exists at %s and is no longer read; "
+                "re-create those jobs or move them into the root store. (#32091)",
+                JOBS_FILE, legacy,
+            )
+    except Exception:
+        pass  # best-effort advisory; never block load_jobs
+
+
 def load_jobs() -> List[Dict[str, Any]]:
     """Load all jobs from storage."""
     ensure_dirs()
     if not JOBS_FILE.exists():
+        _warn_if_orphaned_profile_store()
         return []
 
     _strict_retry = False  # track whether we used the strict=False fallback

cron/scheduler.py

@@ -283,9 +283,17 @@ def _get_hermes_home() -> Path:
 
 
 def _get_lock_paths() -> tuple[Path, Path]:
-    """Resolve cron lock paths at call time so profile/env changes are honored."""
-    hermes_home = _get_hermes_home()
-    lock_dir = hermes_home / "cron"
+    """Resolve cron lock paths at call time so profile/env changes are honored.
+
+    Anchored on the DEFAULT ROOT home (not the active profile), matching the
+    jobs store in cron.jobs (which uses get_default_hermes_root). The tick lock
+    is storage-coordination — it must live next to the single jobs.json so that
+    tickers running under different profiles share one lock and can't
+    double-fire the relocated store (#32091). Execution context (.env,
+    config.yaml, scripts) stays profile-aware via _get_hermes_home().
+    """
+    from hermes_constants import get_default_hermes_root
+    lock_dir = (_hermes_home or get_default_hermes_root()) / "cron"
     return lock_dir, lock_dir / ".tick.lock"
 
 

cron/suggestions.py

@@ -36,13 +36,13 @@ import uuid
 from pathlib import Path
 from typing import Any, Dict, List, Optional
 
-from hermes_constants import get_hermes_home
+from hermes_constants import get_default_hermes_root
 from hermes_time import now as _hermes_now
 from utils import atomic_replace
 
 logger = logging.getLogger(__name__)
 
-CRON_DIR = get_hermes_home().resolve() / "cron"
+CRON_DIR = get_default_hermes_root().resolve() / "cron"
 SUGGESTIONS_FILE = CRON_DIR / "suggestions.json"
 
 # In-process lock protecting load->modify->save cycles (the background review

scripts/release.py

@@ -1225,6 +1225,7 @@ AUTHOR_MAP = {
     "holynn@placeholder.local": "holynn-q",
     "agent@hermes.local": "jacdevos",
     "sunsky.lau@gmail.com": "liuhao1024",
+    "mohamed.origami@gmail.com": "mohamedorigami-jpg",  # PR #32117 (cron storage root anchor; #32091)
     "rob@rbrtbn.com": "rbrtbn",
     "haaasined@gmail.com": "VinciZhu",
     "fabianoeq@gmail.com": "rodrigoeqnit",

tests/cron/test_claim_job_for_fire.py

@@ -14,7 +14,10 @@ import pytest
 def temp_home(tmp_path, monkeypatch):
     """Isolated HERMES_HOME so jobs.json doesn't touch the real store."""
     monkeypatch.setenv("HERMES_HOME", str(tmp_path))
-    # cron.jobs caches no home at import; get_hermes_home() reads the env live.
+    # NOTE: cron.jobs resolves its store paths (JOBS_FILE, CRON_DIR) from
+    # get_default_hermes_root() at IMPORT time, so setting HERMES_HOME here does
+    # not re-point an already-imported module's store. These tests exercise the
+    # claim logic on in-memory job dicts and don't depend on the on-disk path.
     yield tmp_path
 
 

tests/cron/test_cron_profile_storage.py

@@ -0,0 +1,105 @@
+"""Regression tests for #32091 — profile-scoped cron jobs orphaned.
+
+Cron storage (CRON_DIR/JOBS_FILE) must anchor at the *default root* Hermes
+home, not the active profile's home. Otherwise a job created from a
+profile-scoped agent session writes to ~/.hermes/profiles/<p>/cron/jobs.json,
+while the profile-less gateway reads only ~/.hermes/cron/jobs.json — the job
+is silently orphaned (looks healthy in `list`, never fires).
+"""
+import importlib
+import os
+from pathlib import Path
+
+
+def test_cron_storage_anchors_at_root_under_profile(tmp_path, monkeypatch):
+    """Under a profile HERMES_HOME (<root>/profiles/<name>), the cron store
+    resolves to <root>/cron, NOT <root>/profiles/<name>/cron."""
+    root = tmp_path / "hermes_home"
+    profile_home = root / "profiles" / "myprofile"
+    profile_home.mkdir(parents=True)
+
+    # Pretend the platform default root IS our tmp root, and the active
+    # HERMES_HOME is a profile under it (the #32091 scenario).
+    import hermes_constants
+    monkeypatch.setattr(hermes_constants, "_get_platform_default_hermes_home",
+                        lambda: root)
+    monkeypatch.setenv("HERMES_HOME", str(profile_home))
+
+    # get_default_hermes_root must return the ROOT, not the profile dir.
+    assert hermes_constants.get_default_hermes_root().resolve() == root.resolve()
+    # ...while get_hermes_home (used elsewhere) follows the profile override.
+    assert hermes_constants.get_hermes_home().resolve() == profile_home.resolve()
+
+    # cron/jobs.py computes HERMES_DIR from get_default_hermes_root at import,
+    # so a fresh import under this env anchors the store at <root>/cron.
+    import cron.jobs as jobs
+    importlib.reload(jobs)
+    try:
+        assert jobs.HERMES_DIR.resolve() == root.resolve()
+        assert jobs.JOBS_FILE.resolve() == (root / "cron" / "jobs.json").resolve()
+        # The orphan path (<profile>/cron/jobs.json) must NOT be the store.
+        assert jobs.JOBS_FILE.resolve() != (profile_home / "cron" / "jobs.json").resolve()
+    finally:
+        # Restore module state for other tests (reload under the real env).
+        monkeypatch.undo()
+        importlib.reload(jobs)
+
+
+def test_cron_storage_unaffected_when_no_profile(tmp_path, monkeypatch):
+    """With no profile (HERMES_HOME == root), behavior is unchanged: store at
+    <root>/cron."""
+    root = tmp_path / "hermes_home"
+    root.mkdir(parents=True)
+    import hermes_constants
+    monkeypatch.setattr(hermes_constants, "_get_platform_default_hermes_home",
+                        lambda: root)
+    monkeypatch.setenv("HERMES_HOME", str(root))
+
+    import cron.jobs as jobs
+    importlib.reload(jobs)
+    try:
+        assert jobs.JOBS_FILE.resolve() == (root / "cron" / "jobs.json").resolve()
+    finally:
+        monkeypatch.undo()
+        importlib.reload(jobs)
+
+
+def test_tick_lock_anchors_at_root_under_profile(tmp_path, monkeypatch):
+    """The cron tick lock must live at <root>/cron/.tick.lock, NOT the profile
+    dir — otherwise tickers under different profiles grab different locks and
+    double-fire the (now root-anchored) jobs store (#32091)."""
+    import importlib
+    root = tmp_path / "hermes_home"
+    profile_home = root / "profiles" / "p"
+    profile_home.mkdir(parents=True)
+    import hermes_constants
+    monkeypatch.setattr(hermes_constants, "_get_platform_default_hermes_home", lambda: root)
+    monkeypatch.setenv("HERMES_HOME", str(profile_home))
+    import cron.scheduler as sched
+    importlib.reload(sched)
+    try:
+        # _hermes_home override is None -> uses get_default_hermes_root()
+        sched._hermes_home = None
+        lock_dir, lock_file = sched._get_lock_paths()
+        assert lock_dir.resolve() == (root / "cron").resolve()
+        assert lock_file.resolve() == (root / "cron" / ".tick.lock").resolve()
+        assert lock_dir.resolve() != (profile_home / "cron").resolve()
+    finally:
+        monkeypatch.undo()
+        importlib.reload(sched)
+
+
+def test_get_default_hermes_root_docker_layouts(tmp_path, monkeypatch):
+    """get_default_hermes_root resolves the root for Docker/custom HERMES_HOME
+    (outside ~/.hermes), so cron storage works in containers."""
+    import hermes_constants
+    native = tmp_path / "native_home"
+    monkeypatch.setattr(hermes_constants, "_get_platform_default_hermes_home", lambda: native)
+
+    # Docker custom root (outside native): HERMES_HOME itself IS the root.
+    monkeypatch.setenv("HERMES_HOME", "/opt/data")
+    assert hermes_constants.get_default_hermes_root() == Path("/opt/data")
+
+    # Docker profile layout: <custom>/profiles/<name> -> <custom>.
+    monkeypatch.setenv("HERMES_HOME", "/opt/data/profiles/coder")
+    assert hermes_constants.get_default_hermes_root() == Path("/opt/data")