Skip to content

fix(cron): use lazy evaluation for cron paths to prevent test isolation bypass#5

Open
liuhao1024 wants to merge 1 commit into
mainfrom
liuhao/cron-bugfix-60014
Open

fix(cron): use lazy evaluation for cron paths to prevent test isolation bypass#5
liuhao1024 wants to merge 1 commit into
mainfrom
liuhao/cron-bugfix-60014

Conversation

@liuhao1024

Copy link
Copy Markdown
Owner

What does this PR do?

Module-level constants (JOBS_FILE, CRON_DIR, OUTPUT_DIR, TICKER_HEARTBEAT_FILE, TICKER_SUCCESS_FILE) in cron/jobs.py are frozen at import time. Tests that isolate HERMES_HOME via monkeypatch.setenv("HERMES_HOME", tmp_path) expect these paths to honor the patched value, but the frozen constants defeat test isolation — tests write to the real ~/.hermes/cron/jobs.json instead of the test tempdir, creating real cron jobs that fire during production use.

This PR adds lazy accessor functions (_get_cron_dir(), get_jobs_file(), get_output_dir(), get_ticker_heartbeat_file(), get_ticker_success_file()) that resolve paths dynamically on every call, matching the pattern used in cron/scheduler.py:_get_hermes_home(). Internal code uses these accessors; the frozen module-level constants are kept for backward compatibility.

Related Issue

Fixes NousResearch#60014

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • cron/jobs.py: Add lazy accessor functions (_get_cron_dir(), get_jobs_file(), get_output_dir(), get_ticker_heartbeat_file(), get_ticker_success_file())
  • cron/jobs.py: Update all internal usages to call lazy accessors instead of using frozen module-level constants
  • cron/jobs.py: Preserve module-level constants for backward compatibility

How to Test

Before this patch:

  1. On a machine with a real installed Hermes instance (~/.hermes populated):
    pytest tests/cron/test_jobs_changed_notify.py::test_tool_create_notifies_provider -v
  2. Observed result: ~/.hermes/cron/jobs.json contains a new job named w with prompt: "echo hi" and schedule: every 5m (test leaked into production).

After this patch:

  1. Run the same test suite:
    pytest tests/cron/test_jobs_changed_notify.py tests/hermes_cli/test_console_engine.py -v
  2. Observed result: ~/.hermes/cron/jobs.json is unchanged — no new jobs appear from the test run. All 115 tests pass.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes — N/A: This is a test-infrastructure bug where existing tests (tests/cron/test_jobs_changed_notify.py::test_tool_create_notifies_provider and tests/hermes_cli/test_console_engine.py::test_cron_pause_resume_and_run_require_confirmation) now correctly isolate to tempdirs instead of leaking into production. Adding a new test for this would require the test itself to be aware of the test runner's HERMES_HOME environment, which is circular.
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

…on bypass

Module-level constants (JOBS_FILE, CRON_DIR, OUTPUT_DIR, TICKER_HEARTBEAT_FILE,
TICKER_SUCCESS_FILE) are frozen at import time. Tests that monkeypatch
HERMES_HOME with a tempdir (via monkeypatch.setenv('HERMES_HOME', tmp_path))
expect these paths to honor the patched value, but the frozen constants defeat
test isolation — tests write to the real ~/.hermes/cron/jobs.json instead of
the test tempdir, creating real cron jobs that fire during production use.

Add lazy accessor functions (_get_cron_dir(), get_jobs_file(), get_output_dir(),
get_ticker_heartbeat_file(), get_ticker_success_file()) that resolve paths
dynamically on every call, matching the pattern used in cron/scheduler.py.
Internal code uses these accessors; the frozen module-level constants are kept
for backward compatibility.

This prevents test-to-production state leakage in tests/cron/ and
tests/hermes_cli/test_console_engine.py (NousResearch#60014).

Related patterns: NousResearch#50681 (hermes_state.py DEFAULT_DB_PATH, same fix approach).
liuhao1024 pushed a commit that referenced this pull request Jul 10, 2026
…check (#5)

The sync _session_has_compression_in_flight sat on the message hot path
and blocked the event loop twice: under session_store._lock during
_ensure_loaded_locked (JSON read) and via db.get_compression_lock_holder
(SQLite SELECT). Async-ify the method and offload both sources via
asyncio.to_thread; await the call site in _handle_active_session_busy_message.
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.

cron.jobs module-level JOBS_FILE caching defeats HERMES_HOME test isolation — real jobs leak into production (see also #52611, #50681)

1 participant