Skip to content

fix(logging): stop pytest runs from writing to the live agent.log - #57119

Closed
lEWFkRAD wants to merge 3 commits into
NousResearch:mainfrom
lEWFkRAD:fix/test-log-isolation
Closed

fix(logging): stop pytest runs from writing to the live agent.log#57119
lEWFkRAD wants to merge 3 commits into
NousResearch:mainfrom
lEWFkRAD:fix/test-log-isolation

Conversation

@lEWFkRAD

@lEWFkRAD lEWFkRAD commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops pytest tests/ from appending mock-provider log records to the real user's agent.log (%LOCALAPPDATA%\hermes\logs\agent.log on native Windows, ~/.hermes/logs/agent.log on POSIX).

run_agent._hermes_home is resolved once at import time. Under pytest, test modules are imported during collection — before the per-test _hermetic_environment fixture redirects HERMES_HOME — so agent_init's setup_logging(hermes_home=_ra()._hermes_home) attached root-logger file handlers to the real install's logs/agent.log. Every mock-provider test in the process then wrote to the live log, producing entries like provider=xai-oauth base_url=https://api.x.ai/v1 model=grok-4.3 ... msg='Say OK' that look exactly like the agent silently calling real provider APIs.

The fix is at the callsite (per the guidance already in the tests/conftest.py docstring) plus defense-in-depth in the conftest itself:

  1. agent_init now calls setup_logging() with no hermes_home= override — the default resolves get_hermes_home() at call time, which reads the env var live. Behavior for real CLI/gateway runs is unchanged (env doesn't change between import and agent init there).
  2. tests/conftest.py exports a sandbox HERMES_HOME at conftest import time, before pytest imports any test module, so every other import-time get_hermes_home() consumer (cli._hermes_home, dotenv loading) freezes a sandbox path instead of the real install.

Related Issue

Fixes #57118

Type of Change

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

Changes Made

  • agent/agent_init.py — drop the hermes_home=_ra()._hermes_home override on the setup_logging() call; resolve the log directory from the live environment instead of run_agent's import-time frozen path.
  • tests/conftest.py — set HERMES_HOME to a session tempdir at module import time (unconditionally, so a HERMES_HOME inherited from a developer shell or Docker deployment can't leak into tests either). The existing per-test fixture still re-points it per test.
  • tests/run_agent/test_logging_home_isolation.py (new) — two regression tests:
    • test_agent_logging_ignores_import_time_frozen_home: simulates a stale run_agent._hermes_home and asserts AIAgent.__init__ attaches agent.log under the live HERMES_HOME, with nothing under the stale path. Fails if the agent_init fix is reverted.
    • test_conftest_sandboxes_hermes_home_at_import_time: asserts run_agent._hermes_home did not freeze the platform-default (real) home. Fails if the conftest sandbox is reverted.

How to Test

  1. On a machine with a live Hermes install, note the line count of <hermes-home>/logs/agent.log.
  2. On main, run python -m pytest tests/run_agent/test_run_agent_codex_responses.py -qgrep 'api.x.ai' <hermes-home>/logs/agent.log shows new mock records (13 new lines in my repro, Windows 11 native).
  3. On this branch, repeat — the live log gains zero test records (verified: 93 tests across test_run_agent_codex_responses.py + test_auxiliary_client_xai_oauth_recovery.py, live-log signature count unchanged).
  4. python -m pytest tests/run_agent/test_logging_home_isolation.py -q — both regression tests pass; each fails with its half of the fix reverted.

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 (fix(windows): native Windows correctness fixes + green CLI test suite #57016 touches tests/cli/conftest.py reload pollution but not this)
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (per-file runs of the affected areas: tests/run_agent/test_run_agent.py 414 passed, tests/run_agent/test_run_agent_codex_responses.py + xai recovery 93 passed, tests/cron/test_cron_profile_isolation.py, tests/cron/test_cron_no_agent.py, tests/cli/test_cli_approval_ui.py, tests/agent/test_auxiliary_client.py all green; tests/test_hermes_logging.py / tests/test_hermes_constants.py have pre-existing Windows-native chmod/symlink failures identical on unmodified main)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 native (Python 3.12)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — comments at both changed sites explain the import-time-freeze trap
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — the mechanism is platform-independent (POSIX pytest tests/ wrote to the real ~/.hermes/logs/agent.log the same way); fix uses only tempfile/env, no platform branches
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Pollution on main (live %LOCALAPPDATA%\hermes\logs\agent.log after running one test file):

2026-07-02 09:59:12,610 INFO [...] run_agent: OpenAI client created (agent_init, shared=True) thread=MainThread:11288 provider=xai-oauth base_url=https://api.x.ai/v1 model=grok-4.3
2026-07-02 09:59:13,113 INFO [...] agent.turn_context: conversation turn: session=... model=gpt-5.4 provider=copilot platform=unknown history=0 msg='Say OK'

After this branch: same run adds zero records to the live log.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jul 2, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the import-time HERMES_HOME path; the current defect is still present at agent/agent_init.py:706.

Problems

  • The new helper in tests/run_agent/test_logging_home_isolation.py:25 only scans root handlers. Current main routes real file handlers through the async QueueListener; hermes_logging.rotating_file_handlers() is the supported accessor (hermes_logging.py:694-700). The proposed assertion will therefore not see agent.log.
  • Resolving a per-test live home at agent/agent_init.py:706 needs handler lifecycle work. tests/conftest.py:355-361 changes HERMES_HOME every test, while setup_logging() registers per-path handlers before its initialized guard (hermes_logging.py:320-365) and only deduplicates identical paths (hermes_logging.py:740-746). That can retain queued handlers for prior test homes.

Suggested changes

  • Make the new test use rotating_file_handlers() and reset queued logging state in its fixture/teardown.
  • Ensure changing HERMES_HOME does not accumulate stale logging handlers, then cover that invariant.

Automated hermes-sweeper review.

from pathlib import Path
from unittest.mock import patch

import run_agent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main keeps rotating file handlers on the QueueListener, not the root logger (hermes_logging.py:694-700). Please inspect hermes_logging.rotating_file_handlers() here and reset queued-handler state in teardown; otherwise this helper cannot see the agent.log handler and the test leaks listener/file-handler state.

lEWFkRAD and others added 2 commits July 15, 2026 10:04
run_agent._hermes_home is frozen at import time. Under pytest, test
modules are imported during collection — before the per-test fixture
redirects HERMES_HOME — so agent_init's setup_logging(hermes_home=...)
bound root-logger file handlers to the REAL Hermes home
(%LOCALAPPDATA%\hermes\logs\agent.log on native Windows). Every
mock-provider test then appended records to the live install's log,
which read like real X.ai/Copilot API calls.

* agent_init: call setup_logging() with no override so the log dir is
  resolved from the environment at call time (get_hermes_home()).
* tests/conftest.py: export a sandbox HERMES_HOME at conftest import
  time, before pytest imports any test module, as defense-in-depth for
  other import-time get_hermes_home() consumers (cli._hermes_home,
  dotenv loading).
* Regression tests for both (each fails with its fix reverted).

Fixes NousResearch#57118

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lEWFkRAD
lEWFkRAD force-pushed the fix/test-log-isolation branch from 11e971d to 249db87 Compare July 15, 2026 14:05
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @lEWFkRAD — closing as resolved on main: PR #74553 landed a session-level HERMES_HOME sandbox at conftest import time (via #71271), which closes the import-time-frozen log path this PR targeted, and tests/test_log_isolation.py now pins the invariant. Your diagnosis of the class (also filed as #57118) was right — credited there too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pytest runs pollute the real HERMES_HOME agent.log with mock-provider records (import-time frozen run_agent._hermes_home)

3 participants