mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
fix(tests): stop the suite writing into the operator's real Hermes logs
hermes_cli/main.py calls setup_logging() at module scope. That resolves get_hermes_home() and attaches rotating file handlers to the ROOT logger via a QueueHandler. So merely importing it - which many test modules do, directly or transitively - points the whole pytest session's logging at <HERMES_HOME>/logs/agent.log and errors.log. The _isolate_env fixture already sandboxes HERMES_HOME, but fixtures run after collection has imported the test modules, and by then the handler holds an absolute path to the real file. Verified by importing hermes_cli.main in a clean interpreter and walking the queue listener: both handlers pointed at the developer's own ~/.hermes/logs/. Measured on a live install: 126 warnings in a personal agent.log came from test runs rather than the running gateway - phantom 'FakeTree' Discord registration failures and 'rejected invalid API key' entries whose paths only exist in tests/gateway/test_api_server_runs.py. That noise makes genuine warnings hard to find exactly when someone is debugging. conftest is imported before any test module, so sandboxing HERMES_HOME there closes the window. The per-test fixture still applies afterwards. Also fixes 4 pre-existing failures: tests/gateway/test_channel_directory.py TestBuildFromSessions was reading the operator's real sessions data for the same reason. Full gateway+tools suites: 66 failures on clean origin/main, 62 with this change, 0 new. The regression guard asserts the value captured AT conftest import - reading os.environ inside a test passes even with the fix removed, because the per-test fixture has sandboxed it by then. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
dda289933d
commit
ef1b06d853
2 changed files with 142 additions and 8 deletions
|
|
@ -20,9 +20,12 @@ test runner at ``scripts/run_tests.sh``.
|
|||
"""
|
||||
|
||||
import asyncio
|
||||
import atexit
|
||||
import os
|
||||
import shutil
|
||||
import sqlite3
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
|
@ -33,6 +36,42 @@ if str(PROJECT_ROOT) not in sys.path:
|
|||
sys.path.insert(0, str(PROJECT_ROOT))
|
||||
|
||||
|
||||
# ── Sandbox HERMES_HOME before ANY test module is imported ──────────────────
|
||||
# `hermes_cli/main.py` calls `setup_logging()` at MODULE level, which resolves
|
||||
# `get_hermes_home()` and attaches rotating file handlers to the ROOT logger.
|
||||
# So merely importing it - which many test modules do, directly or
|
||||
# transitively - points the whole pytest session's logging at the operator's
|
||||
# real `~/.hermes/logs/agent.log` and `errors.log`.
|
||||
#
|
||||
# The `_isolate_env` fixture below also sandboxes HERMES_HOME, but fixtures run
|
||||
# AFTER collection imports test modules, by which point the handler already
|
||||
# holds an absolute path to the real log. Measured on a live install: 126
|
||||
# warnings in the operator's agent.log came from test runs, not the gateway -
|
||||
# enough noise to make genuine warnings hard to find.
|
||||
#
|
||||
# conftest is imported before any test module, so setting it here closes that
|
||||
# window. The per-test fixture still applies for everything after import.
|
||||
#
|
||||
# ORDER MATTERS: the kanban write guard's deny-list (further down) must know
|
||||
# the REAL Hermes root — capture it BEFORE the sandbox rewires HERMES_HOME,
|
||||
# otherwise the deny-list would point at the throwaway tempdir and the guard
|
||||
# would silently stop protecting the operator's actual ~/.hermes (#69385).
|
||||
_PRE_SANDBOX_KANBAN_OVERRIDE = os.environ.get("HERMES_KANBAN_HOME", "").strip()
|
||||
_PRE_SANDBOX_HERMES_HOME = os.environ.get("HERMES_HOME", "")
|
||||
if not os.environ.get("HERMES_HOME"):
|
||||
_SESSION_HERMES_HOME = tempfile.mkdtemp(prefix="hermes-test-home-")
|
||||
os.environ["HERMES_HOME"] = _SESSION_HERMES_HOME
|
||||
atexit.register(shutil.rmtree, _SESSION_HERMES_HOME, True)
|
||||
|
||||
#: HERMES_HOME as it stood when conftest was imported - i.e. before any test
|
||||
#: module could import code that configures logging. Recorded so the guard in
|
||||
#: tests/test_log_isolation.py can assert the sandbox existed AT THAT MOMENT.
|
||||
#: Reading os.environ from inside a test is useless here: the per-test
|
||||
#: `_isolate_env` fixture has sandboxed it by then, so the check would pass
|
||||
#: even with this block removed.
|
||||
HERMES_HOME_AT_CONFTEST_IMPORT = os.environ.get("HERMES_HOME", "")
|
||||
|
||||
|
||||
# ── Per-file process isolation ──────────────────────────────────────────────
|
||||
# Tests run via ``scripts/run_tests_parallel.py``, which spawns a fresh
|
||||
# ``python -m pytest <file>`` subprocess per test file. Cross-file state
|
||||
|
|
@ -516,16 +555,23 @@ def _neutralize_macos_keychain_creds(request, monkeypatch):
|
|||
def _capture_real_kanban_root() -> Path:
|
||||
"""Resolve the REAL kanban root from the pre-test environment.
|
||||
|
||||
Runs at conftest import time, before any fixture rewires HERMES_HOME.
|
||||
Mirrors ``kanban_db.kanban_home()`` resolution order:
|
||||
Uses the pre-sandbox environment snapshot taken at the very top of this
|
||||
file (before the session HERMES_HOME sandbox rewired the env), so the
|
||||
deny-list keeps pointing at the operator's actual root. Mirrors
|
||||
``kanban_db.kanban_home()`` resolution order:
|
||||
1. ``HERMES_KANBAN_HOME`` env var when set and non-empty
|
||||
2. ``get_default_hermes_root()`` otherwise
|
||||
2. the real (pre-sandbox) Hermes root otherwise
|
||||
"""
|
||||
override = os.environ.get("HERMES_KANBAN_HOME", "").strip()
|
||||
if override:
|
||||
return Path(override).expanduser().resolve()
|
||||
from hermes_constants import get_default_hermes_root
|
||||
return get_default_hermes_root().resolve()
|
||||
if _PRE_SANDBOX_KANBAN_OVERRIDE:
|
||||
return Path(_PRE_SANDBOX_KANBAN_OVERRIDE).expanduser().resolve()
|
||||
if _PRE_SANDBOX_HERMES_HOME:
|
||||
# HERMES_HOME was genuinely set before the sandbox — honor it via the
|
||||
# normal resolver (it may be a profile dir whose root matters).
|
||||
from hermes_constants import get_default_hermes_root
|
||||
return get_default_hermes_root().resolve()
|
||||
# No pre-existing HERMES_HOME: the real root is the platform default,
|
||||
# NOT the sandbox tempdir now sitting in the env.
|
||||
return (Path.home() / ".hermes").resolve()
|
||||
|
||||
|
||||
_REAL_KANBAN_ROOT = _capture_real_kanban_root()
|
||||
|
|
|
|||
88
tests/test_log_isolation.py
Normal file
88
tests/test_log_isolation.py
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
"""The test suite must never write into the operator's real Hermes logs.
|
||||
|
||||
`hermes_cli/main.py` calls `setup_logging()` at module scope, which resolves
|
||||
`get_hermes_home()` and attaches rotating file handlers to the ROOT logger.
|
||||
Importing it - which many test modules do, directly or transitively - wires
|
||||
the whole pytest session's logging to `<HERMES_HOME>/logs/agent.log`.
|
||||
|
||||
If HERMES_HOME is not already sandboxed at that moment, that is the
|
||||
operator's real log. Measured on a live install, 126 warnings in a personal
|
||||
`agent.log` came from test runs rather than the running gateway: phantom
|
||||
`FakeTree` Discord failures and `rejected invalid API key` entries from
|
||||
`test_api_server_runs.py`. Noise like that makes genuine warnings hard to
|
||||
find precisely when someone is debugging.
|
||||
|
||||
The per-test env fixture cannot close this: fixtures run after collection has
|
||||
imported the test modules, and by then the handler holds an absolute path.
|
||||
`tests/conftest.py` sets HERMES_HOME at module scope for that reason - this
|
||||
guards the property so a refactor cannot quietly undo it.
|
||||
"""
|
||||
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _real_hermes_home() -> Path:
|
||||
"""Where the operator's logs live, ignoring any test sandboxing."""
|
||||
return Path.home() / ".hermes"
|
||||
|
||||
|
||||
def _all_file_destinations() -> list[str]:
|
||||
"""Every file path the root logger can reach, including via a QueueHandler.
|
||||
|
||||
Logging is routed through a queue, so the file handlers hang off the
|
||||
listener rather than the root logger - checking `root.handlers` alone
|
||||
reports nothing and looks falsely clean.
|
||||
"""
|
||||
seen: list[str] = []
|
||||
|
||||
def collect(handlers) -> None:
|
||||
for handler in handlers or ():
|
||||
path = getattr(handler, "baseFilename", None)
|
||||
if path:
|
||||
seen.append(str(path))
|
||||
listener = getattr(handler, "listener", None)
|
||||
if listener is not None:
|
||||
collect(getattr(listener, "handlers", ()))
|
||||
|
||||
collect(logging.getLogger().handlers)
|
||||
|
||||
try:
|
||||
import hermes_logging
|
||||
|
||||
listener = getattr(hermes_logging, "_queue_listener", None)
|
||||
if listener is not None:
|
||||
collect(getattr(listener, "handlers", ()))
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return seen
|
||||
|
||||
|
||||
class TestLogIsolation:
|
||||
def test_hermes_home_is_sandboxed_before_imports(self):
|
||||
# Deliberately NOT os.environ: by test time the per-test `_isolate_env`
|
||||
# fixture has sandboxed HERMES_HOME, so reading it here would pass even
|
||||
# with the conftest block deleted. Assert the value captured at conftest
|
||||
# import, which is the moment that actually matters.
|
||||
from tests.conftest import HERMES_HOME_AT_CONFTEST_IMPORT as home
|
||||
|
||||
assert home, "conftest must set HERMES_HOME before test modules import"
|
||||
assert Path(home).resolve() != _real_hermes_home().resolve(), (
|
||||
f"HERMES_HOME pointed at the operator's real home ({home}) when "
|
||||
"conftest loaded; import-time setup_logging() writes to their agent.log"
|
||||
)
|
||||
|
||||
def test_importing_the_cli_does_not_target_the_real_logs(self):
|
||||
pytest.importorskip("hermes_cli.main")
|
||||
|
||||
real_logs = str(_real_hermes_home() / "logs")
|
||||
offenders = [p for p in _all_file_destinations() if p.startswith(real_logs)]
|
||||
|
||||
assert offenders == [], (
|
||||
"the test session is writing into the operator's real Hermes logs:\n "
|
||||
+ "\n ".join(offenders)
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue