test(hermetic): gate on unrestored sys.modules purges (+2 leakers)

tests/agent/test_save_url_image.py

@@ -79,18 +79,30 @@ def http_server(tmp_path, monkeypatch):
     monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
     (tmp_path / ".hermes").mkdir()
 
-    # Force the constants/image cache helpers to re-read HERMES_HOME.
+    # Force the constants/image cache helpers to re-read HERMES_HOME, then
+    # RESTORE them. An unrestored purge hands every later importer a brand-new
+    # module object, so a subsequent test that captured a reference to (or
+    # monkeypatched) one of these silently operates on an orphaned copy — the
+    # bug class fixed in PR #538. Caught here by the sys.modules leak gate.
     import sys
-    for mod in list(sys.modules):
-        if mod.startswith("hermes_constants") or mod.startswith("agent.image_gen_provider"):
-            sys.modules.pop(mod, None)
+    _saved_modules = {
+        name: mod
+        for name, mod in sys.modules.items()
+        if name.startswith("hermes_constants")
+        or name.startswith("agent.image_gen_provider")
+    }
+    for mod in _saved_modules:
+        sys.modules.pop(mod, None)
 
     httpd = socketserver.TCPServer(("127.0.0.1", 0), _TinyImageHandler)
     port = httpd.server_address[1]
     thread = threading.Thread(target=httpd.serve_forever, daemon=True)
     thread.start()
-    yield f"http://127.0.0.1:{port}", httpd
-    httpd.shutdown()
+    try:
+        yield f"http://127.0.0.1:{port}", httpd
+    finally:
+        httpd.shutdown()
+        sys.modules.update(_saved_modules)
 
 
 class TestSaveUrlImage:

tests/conftest.py

@@ -36,6 +36,16 @@
     sys.path.insert(0, str(PROJECT_ROOT))
 
 
+# ── sys.modules leak gate ───────────────────────────────────────────────────
+# Fails a test that purges sys.modules of watched modules (run_agent, agent.*,
+# tools.*, hermes_*, gateway.*, plugins.*) without restoring them. That bug class
+# cost ~46 suite failures from ONE fixture (PR #538) and ~20 more from a second:
+# later importers get a brand-new module object, so a subsequent test patches an
+# orphaned copy while the code under test imports a different one.
+# Opt out with @pytest.mark.allow_sys_modules_purge (and say why).
+pytest_plugins = ["tests.sys_modules_leak_gate"]
+
+
 def _strip_nonsandbox_file_handlers(sandbox_prefix=None):
     """Remove any logging file handler (root + named loggers) whose target file
     lives OUTSIDE the per-test sandbox — chiefly the real ``~/.hermes/logs/
@@ -1296,6 +1306,11 @@ def pytest_configure(config):  # noqa: D401 — pytest hook
         # still runs per-file under CI where the default limit suffices.
         pass
 
+    config.addinivalue_line(
+        "markers",
+        "allow_sys_modules_purge: test intentionally leaves sys.modules purged "
+        "(opts out of the sys.modules leak gate — say why in a comment)",
+    )
     config.addinivalue_line(
         "markers",
         f"{_LIVE_SYSTEM_GUARD_BYPASS_MARK}: bypass the live-system guard "

tests/hermes_cli/test_kanban_per_profile_cap.py

@@ -21,11 +21,25 @@ def isolated_kanban_home_with_profiles(monkeypatch):
     for prof in ("alpha", "beta", "default"):
         os.makedirs(os.path.join(test_home, "profiles", prof), exist_ok=True)
     monkeypatch.setenv("HERMES_HOME", test_home)
-    for mod in list(sys.modules.keys()):
-        if mod.startswith("hermes_cli") or mod.startswith("hermes_state") or mod == "hermes_constants":
-            del sys.modules[mod]
+    # Purge so `from hermes_cli import kanban_db` re-resolves against the fresh
+    # HERMES_HOME. RESTORE afterwards: an unrestored purge hands every later
+    # importer a brand-new module object, so a subsequent test that captured a
+    # reference to (or monkeypatched) one of these silently operates on an
+    # orphaned copy — the bug class fixed in PR #538.
+    _saved_modules = {
+        name: mod
+        for name, mod in sys.modules.items()
+        if name.startswith("hermes_cli")
+        or name.startswith("hermes_state")
+        or name == "hermes_constants"
+    }
+    for mod in _saved_modules:
+        del sys.modules[mod]
     from hermes_cli import kanban_db
-    yield kanban_db
+    try:
+        yield kanban_db
+    finally:
+        sys.modules.update(_saved_modules)
 
 
 def _fake_spawn(*args, **kwargs):

tests/sys_modules_leak_gate.py

@@ -0,0 +1,126 @@
+"""Gate: a test that purges ``sys.modules`` must restore it.
+
+WHY THIS EXISTS
+---------------
+Three separate leakers of this exact shape were found in ``tests/agent`` on
+2026-08-09, each costing a real chunk of the suite:
+
+  * ``test_empty_tool_name_loop_dampening.py``  (#538) — 120 -> 74 failures
+  * ``test_verification_stop_caching.py``               — ~20 more
+  * a third found in parallel by a worker, same class
+
+The pattern is always the same. A test wants a *fresh* import (usually to pick
+up a patched module or a new ``HERMES_HOME``), so it does::
+
+    for mod in list(sys.modules):
+        if mod == "run_agent" or mod.startswith("agent."):
+            del sys.modules[mod]
+    import run_agent
+
+...and never puts them back. Every LATER importer in the same pytest process
+then receives a brand-new module object, so any subsequent test that captured a
+reference to — or monkeypatched — one of those modules is operating on a
+*different object* than the code under test imports. Its setup silently applies
+to an orphaned copy.
+
+The failures land far away from the cause and look like unrelated bugs
+("No LLM provider configured", stale caches, missing attributes), which is what
+made them expensive to find: a full bisect of the file list per victim.
+
+WHAT THIS GATE DOES
+-------------------
+Fails LOUDLY, at the leaking test, naming it — instead of mysteriously
+downstream. This is the "codify the lesson as a durable gate" half: the bisect
+found the instances, this stops the class.
+
+It is deliberately NARROW: it only fires when a test leaves ``sys.modules`` in a
+state that can poison a sibling — modules that existed before the test and are
+gone after. Adding new modules is normal (imports happen); REMOVING pre-existing
+ones is the hazard.
+"""
+
+from __future__ import annotations
+
+import sys
+
+import pytest
+
+
+# Only these prefixes can realistically poison a sibling test in this repo. A
+# broad check would fire on third-party lazy-import churn (botocore, urllib3,
+# importlib metadata) and be pure noise.
+#
+# 🔴 `plugins.` is deliberately EXCLUDED. Plugin-DISCOVERY tests
+# (tests/providers/test_plugin_discovery.py, tests/hermes_cli/test_kanban_*)
+# purge `plugins.model_providers.*` on purpose to force a re-scan, and that is
+# the behaviour under test — the modules are re-imported by the very next
+# discovery call, so they do not poison siblings the way a stale `agent.*`
+# module object does. Gating them would produce ~34 false positives on tests
+# that are working as designed. Verified 2026-08-09: both files pass on main and
+# are not among the suite's cross-test failures.
+_WATCHED_PREFIXES = ("run_agent", "agent.", "tools.", "hermes_", "gateway.")
+
+
+def _watched(name: str) -> bool:
+    return name == "run_agent" or name.startswith(_WATCHED_PREFIXES)
+
+
+def snapshot_watched() -> set:
+    """Watched modules currently in ``sys.modules`` (the 'before' half)."""
+    return {name for name in sys.modules if _watched(name)}
+
+
+def leak_failure_message(nodeid: str, removed: set) -> str:
+    """Render the failure text for a set of unrestored removals, or '' if clean.
+
+    Split out from the fixture so the gate's LOGIC is directly testable without
+    driving pytest internals.
+    """
+    if not removed:
+        return ""
+    sample = ", ".join(sorted(removed)[:6])
+    more = f" (+{len(removed) - 6} more)" if len(removed) > 6 else ""
+    return (
+        f"{nodeid} removed {len(removed)} module(s) from sys.modules "
+        f"and did not restore them: {sample}{more}\n\n"
+        "Every later importer now gets a BRAND-NEW module object, so any test "
+        "that captured a reference to (or monkeypatched) one of these will "
+        "silently operate on an orphaned copy. This is the bug class fixed in "
+        "PR #538 — it cost ~46 suite failures from a single fixture.\n\n"
+        "Fix: save and restore around the purge, including parent-module "
+        "attributes:\n"
+        "    _MISSING = object()\n"
+        "    saved = dict(sys.modules)\n"
+        "    saved_attrs = {}   # (parent_module, child_name) -> value or _MISSING\n"
+        "    ...purge + re-import...\n"
+        "    finally:\n"
+        "        for n in list(sys.modules):\n"
+        "            if n not in saved: sys.modules.pop(n, None)\n"
+        "        sys.modules.update(saved)\n"
+        "        for (parent, child), v in saved_attrs.items():\n"
+        "            parent.__dict__.pop(child, None) if v is _MISSING else "
+        "parent.__dict__.__setitem__(child, v)\n\n"
+        "If the purge is genuinely required to persist, mark the test "
+        "@pytest.mark.allow_sys_modules_purge and say why."
+    )
+
+
+@pytest.fixture(autouse=True)
+def _sys_modules_leak_gate(request):
+    """Fail a test that DELETES a pre-existing watched module without restoring it.
+
+    Opt out for a test that legitimately must leave the purge in place::
+
+        @pytest.mark.allow_sys_modules_purge
+        def test_something(): ...
+    """
+    if request.node.get_closest_marker("allow_sys_modules_purge"):
+        yield
+        return
+
+    before = snapshot_watched()
+    yield
+    removed = before - snapshot_watched()
+    message = leak_failure_message(request.node.nodeid, removed)
+    if message:
+        pytest.fail(message)

tests/test_sys_modules_leak_gate.py

@@ -0,0 +1,114 @@
+"""Proof that the sys.modules leak gate FIRES — and stays quiet otherwise.
+
+A gate that has never been shown to fail is not a gate. These exercise the
+gate's real logic (``snapshot_watched`` + ``leak_failure_message``) rather than
+driving pytest internals, so both directions are provable and the test is not
+coupled to fixture plumbing.
+"""
+
+from __future__ import annotations
+
+import sys
+import types
+
+import pytest
+
+from tests.sys_modules_leak_gate import (
+    _watched,
+    leak_failure_message,
+    snapshot_watched,
+)
+
+NODE = "tests/fake/test_thing.py::test_case"
+
+
+def _removed_by(body) -> set:
+    """Run ``body`` and return the watched modules it removed without restoring."""
+    before = snapshot_watched()
+    body()
+    return before - snapshot_watched()
+
+
+@pytest.fixture
+def sentinel_module():
+    name = "agent._leak_gate_sentinel"
+    sys.modules[name] = types.ModuleType(name)
+    yield name
+    sys.modules.pop(name, None)
+
+
+class TestGateFires:
+    def test_unrestored_purge_is_detected(self, sentinel_module):
+        """THE case: a purge with no restore must be caught."""
+        removed = _removed_by(lambda: sys.modules.pop(sentinel_module, None))
+        assert sentinel_module in removed
+
+    def test_failure_message_is_produced_and_names_the_test(self, sentinel_module):
+        removed = _removed_by(lambda: sys.modules.pop(sentinel_module, None))
+        msg = leak_failure_message(NODE, removed)
+        assert msg, "gate produced no failure message for a real leak"
+        assert NODE in msg
+        assert sentinel_module in msg
+        assert "did not restore" in msg
+
+    def test_message_carries_the_fix_recipe(self, sentinel_module):
+        """The next person shouldn't have to re-derive the fix."""
+        removed = _removed_by(lambda: sys.modules.pop(sentinel_module, None))
+        msg = leak_failure_message(NODE, removed)
+        assert "sys.modules.update(saved)" in msg
+        assert "parent.__dict__" in msg
+        assert "allow_sys_modules_purge" in msg
+
+    def test_message_truncates_a_large_leak(self):
+        removed = {f"agent.mod{i}" for i in range(20)}
+        msg = leak_failure_message(NODE, removed)
+        assert "removed 20 module(s)" in msg
+        assert "+14 more" in msg
+
+
+class TestGateStaysQuiet:
+    def test_restored_purge_is_clean(self, sentinel_module):
+        """Purge + restore — the CORRECT pattern — must not fire."""
+        def body():
+            saved = dict(sys.modules)
+            sys.modules.pop(sentinel_module, None)
+            sys.modules.update(saved)
+        assert _removed_by(body) == set()
+
+    def test_adding_modules_is_not_a_leak(self):
+        """Imports happen; only REMOVING pre-existing modules is the hazard."""
+        name = "agent._leak_gate_added"
+        try:
+            assert _removed_by(lambda: sys.modules.setdefault(name, types.ModuleType(name))) == set()
+        finally:
+            sys.modules.pop(name, None)
+
+    def test_untouched_run_is_clean(self):
+        assert _removed_by(lambda: None) == set()
+
+    def test_unwatched_module_removal_is_ignored(self):
+        """Third-party lazy-import churn must not produce noise."""
+        name = "some_vendor_lib._lazy"
+        sys.modules[name] = types.ModuleType(name)
+        assert _removed_by(lambda: sys.modules.pop(name, None)) == set()
+
+    def test_empty_removal_yields_no_message(self):
+        assert leak_failure_message(NODE, set()) == ""
+
+
+class TestWatchedPrefixesAreNarrow:
+    @pytest.mark.parametrize("name,expected", [
+        ("run_agent", True),
+        ("agent.title_generator", True),
+        ("tools.browser_tool", True),
+        ("hermes_logging", True),
+        ("gateway.run", True),
+        ("plugins.discord", False),   # deliberately excluded — discovery tests re-scan
+        ("botocore.session", False),
+        ("urllib3.connection", False),
+        # must not match on a bare substring — a broad check would be pure noise
+        ("agentic_unrelated", False),
+        ("my_agent", False),
+    ])
+    def test_prefix_matching(self, name, expected):
+        assert _watched(name) is expected