Commit graph

7 commits

Author SHA1 Message Date
Michael Lam
b9bf00efe1 fix: shorten cron profile lock for manual runs 2026-05-06 18:11:14 +00:00
Michael Lam
8d77e0be49 fix: isolate in-process cron scheduler profiles 2026-05-06 08:47:16 -07:00
Michael Lam
3f3092a84e feat: add scheduled job profile selector 2026-05-05 01:18:34 +00:00
Frank Song
cdcd6021cc fix(cron): tighten worker-side broad-except in _run_cron_tracked (closes #1578)
Remove the try/except Exception wrapper around
cron_profile_context_for_home(...).__enter__() in _run_cron_tracked.
A silent fallback to ctx=None would leave the worker thread unpinned
against process-global HERMES_HOME, silently corrupting cross-profile
state — the same class of bug as #1573.

Add regression test to catch any future re-introduction.
2026-05-04 16:28:33 +08:00
Nathan Esquenazi
556f2390d4 test(cron-profile): auto-skip cron.jobs-dependent tests when agent unavailable
Two of the three new tests in test_scheduled_jobs_profile_isolation.py
import cron.jobs (from hermes-agent) and fail with ModuleNotFoundError
in environments where hermes-agent isn't installed at ~/hermes-agent.

The contributor's path-injection trick at module load
(`AGENT_ROOT = Path(os.environ.get("HERMES_AGENT_ROOT", Path.home() / "hermes-agent"))`)
assumes the agent lives at ~/hermes-agent, which isn't always true on
maintainer/reviewer machines or in some CI configurations. The repo's
existing convention for this is conftest.py's `_AGENT_DEPENDENT_TESTS`
auto-skip, but that requires test names to be explicitly listed.

Cleaner fix: gate the two cron.jobs-importing tests with
`pytest.importorskip("cron.jobs")` so they self-skip cleanly when the
module isn't available, while leaving the third test
(`test_cron_profile_context_serializes_concurrent_access`) untouched —
it doesn't actually need cron.jobs and provides useful coverage even
without hermes-agent installed.

Verified: full suite goes from `2 failed, 4001 passed` to `4001 passed,
57 skipped` with no regression.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-03 22:36:26 +00:00
nesquena-hermes
df03055def Address review feedback: tighten profile-resolution error handling
Three small follow-ups from the review:

1. Remove the over-broad except Exception around get_active_hermes_home()
   in _handle_cron_run. The function is in-memory dict reads + one
   Path.is_dir() stat — if it raises from inside a request handler,
   api.profiles is in a state we shouldn't be making cron decisions in.
   A silent fallback to _profile_home=None re-introduces the exact
   bug #1573 fixes (worker thread runs unpinned against process-global
   HERMES_HOME). Better to 500 the request than risk silent cross-
   profile state corruption.

2. Add a thread-safety note on os.environ mutation in api/profiles.py
   explaining why _cron_env_lock is sufficient — CPython env-var
   assignment is GIL-protected at the bytecode level but the multi-step
   read-modify-write pattern (snapshot prev → assign new → restore on
   exit) is not atomic without explicit serialization. The lock makes
   the entire context-manager body run-to-completion serially, including
   any subprocess.Popen() calls inside run_job() that inherit the env.

3. New regression test (test_cron_run_does_not_silently_swallow_profile_resolution_errors)
   pinning the no-silent-fallback contract via source-level assertion.
   Catches future re-introduction of the over-broad except clause.

Co-authored-by: kowenhaoai <kowenhaoai@users.noreply.github.com>
2026-05-03 22:29:57 +00:00
貓鷹閣 Hermes
2a8311a788 fix(cron): scheduled jobs panel respects active profile
Wrap all /api/crons* endpoints in cron_profile_context so the TLS-active
profile's jobs.json is read/written, not the process-default one.

Before: cron.jobs._get_jobs_file() reads HERMES_HOME from os.environ
(process-global) at call time, bypassing WebUI's per-request thread-local
profile. Result: the Scheduled jobs panel always showed the default
profile's jobs regardless of which profile the user selected via cookie,
and CRUD operations silently wrote to the wrong jobs.json.

Fix:
- api/profiles.py: new cron_profile_context (HTTP/TLS) and
  cron_profile_context_for_home (worker threads) context managers. Both
  hold a module-level lock, swap os.environ['HERMES_HOME'], and re-patch
  cron.jobs module-level constants (HERMES_DIR/CRON_DIR/JOBS_FILE/
  OUTPUT_DIR are import-time snapshots that don't participate in the
  module's lazy __getattr__ path).
- api/routes.py: wrap all 12 cron endpoints (GET + POST). For
  /api/crons/run, capture the TLS-active home at dispatch time and
  pass it into the background thread so cron output lands in the right
  profile directory.

Tests: 3 new regression tests in test_scheduled_jobs_profile_isolation.py
cover TLS-based pinning, explicit-home pinning, and serialization of
concurrent contexts. Full cron + profile test suite (24 tests) passes.

Refs: ~/.hermes/patches/hermes-webui_scheduled-jobs-profile-isolation.patch
Obsidian: Hermes_Patches/20260504_Hermes_WebUI_Scheduled_Jobs_Profile_Isolation.md
2026-05-04 06:00:17 +08:00