test-infra: hard-isolate test state from production + resilient teardown (#4289)

* test-infra: hard-isolate test state from production + robust teardown

Two production-safety fixes to tests/conftest.py + tests/_pytest_port.py:

1. TEST_STATE_DIR now defaults under the OS temp dir (/tmp/hermes-webui-tests/)
   instead of HERMES_HOME/webui-test-<hash> — which had been writing test state
   INTO the real ~/.hermes/profiles/webui/ (144 leaked webui-test-* dirs found).
   Added a hard-stop guard: refuse to run if the resolved test state dir is
   inside the production Hermes home, so a misconfigured env can never let tests
   wipe/clobber production profiles/sessions/credentials.

2. _rmtree_retry is now resilient on Linux (was a bare shutil.rmtree): retries
   transient OSError [Errno 39] Directory-not-empty races (a background daemon
   writing into the tree during teardown) + a final ignore_errors sweep, and
   warns instead of raising so a benign cleanup race can't spuriously ERROR a
   test (the failure mode seen near #4283 context-reconciliation tests).

* test-infra: anchor production-proximity guard on literal ~/.hermes (not $HERMES_HOME)

The first guard compared TEST_STATE_DIR against $HERMES_HOME, which is overridden
to TEST_STATE_DIR itself during a run -> false-tripped collection (6 modules) with
'test dir X is inside production home X'. Now anchor on the literal user-home
~/.hermes and explicitly allow any dir under the OS temp root, so the temp default
never trips while a real ~/.hermes target still hard-stops.

* test-infra: close TMPDIR-under-~/.hermes guard hole (Codex)

Codex flagged: the 'temp is always allowed' exception could be exploited if
TMPDIR itself is set under ~/.hermes — then a 'temp' path is a production path and
_under_temp would wrongly suppress the guard. Now the temp exception only holds
when the temp root is itself OUTSIDE the production home. Verified: normal /tmp
default allowed; TMPDIR=~/.hermes/tmp now trips the guard.

* test-infra: mirror production-proximity guard into _pytest_port.py (Opus defense-in-depth)

* test-infra: remove ~/.hermes production-path fallbacks from 4 test files (Codex)

Codex's kickback found the real gap: test_sprint5, test_sprint45,
test_onboarding_existing_config, test_regressions each hardcoded a
~/.hermes/webui-mvp-test fallback for their state dir (used when running
standalone or if the env var is unset). Repointed all to the shared isolated
TEST_STATE_DIR from _pytest_port (temp-rooted). Also refreshed stale
~/.hermes/webui-mvp-test references in test_gateway_sync comment + ARCHITECTURE.md.
Now ZERO production-path fallbacks remain in tests.

---------

Co-authored-by: nesquena-hermes <agent@nesquena-hermes>

ARCHITECTURE.md

@@ -145,7 +145,7 @@ Environment variables controlling behavior:
 Test isolation environment variables (set by conftest.py):
 
     HERMES_WEBUI_TEST_PORT=...                         Optional pinned test port
-    HERMES_WEBUI_TEST_STATE_DIR=~/.hermes/webui-test-* Optional pinned test state
+    HERMES_WEBUI_TEST_STATE_DIR=/tmp/hermes-webui-tests/* Optional pinned test state (default: OS temp dir; must be outside ~/.hermes)
     HERMES_WEBUI_DEFAULT_WORKSPACE=.../test-workspace  Isolated test workspace
 
 Tests NEVER talk to the production server (port 8787).

tests/_pytest_port.py

@@ -29,17 +29,42 @@ def _auto_state_dir_name(repo_root: pathlib.Path) -> str:
 
 _TESTS_DIR   = pathlib.Path(__file__).parent.resolve()
 _REPO_ROOT   = _TESTS_DIR.parent.resolve()
-_HERMES_HOME = pathlib.Path(os.getenv('HERMES_HOME',
-                             str(pathlib.Path.home() / '.hermes')))
 
 TEST_PORT = int(os.environ.get('HERMES_WEBUI_TEST_PORT',
                                str(_auto_test_port(_REPO_ROOT))))
 BASE = f"http://127.0.0.1:{TEST_PORT}"
 
+# Test state dir: prefer the value conftest.py published to the environment.
+# The standalone fallback anchors under the OS temp dir (NOT ~/.hermes) so test
+# state is never created inside the production Hermes home — matching conftest's
+# hard-isolation default. (See conftest.py TEST_STATE_DIR.)
+import tempfile as _tempfile
+_TEST_STATE_ROOT = pathlib.Path(
+    os.environ.get('HERMES_WEBUI_TEST_STATE_ROOT', _tempfile.gettempdir())
+) / 'hermes-webui-tests'
 TEST_STATE_DIR = pathlib.Path(os.environ.get(
     'HERMES_WEBUI_TEST_STATE_DIR',
-    str(_HERMES_HOME / _auto_state_dir_name(_REPO_ROOT))
-))
+    str(_TEST_STATE_ROOT / _auto_state_dir_name(_REPO_ROOT))
+)).resolve()
+
+# Defense-in-depth: mirror conftest.py's production-proximity guard so a
+# standalone import (without conftest) can't resolve a state dir inside the real
+# ~/.hermes either. Same logic: trip only if under literal ~/.hermes and the temp
+# root isn't itself under production.
+_PROD_HERMES_HOME = (pathlib.Path.home() / '.hermes').resolve()
+_TEMP_ROOT = pathlib.Path(_tempfile.gettempdir()).resolve()
+_temp_root_is_safe = not (
+    _TEMP_ROOT == _PROD_HERMES_HOME or _PROD_HERMES_HOME in _TEMP_ROOT.parents
+)
+_under_temp = _temp_root_is_safe and (
+    TEST_STATE_DIR == _TEMP_ROOT or _TEMP_ROOT in TEST_STATE_DIR.parents
+)
+_under_prod = TEST_STATE_DIR == _PROD_HERMES_HOME or _PROD_HERMES_HOME in TEST_STATE_DIR.parents
+if _under_prod and not _under_temp:
+    raise RuntimeError(
+        f"REFUSING TO RUN: test state dir {TEST_STATE_DIR} is inside the production "
+        f"Hermes home {_PROD_HERMES_HOME}. Tests must never touch production files."
+    )
 
 # Default model injected by conftest — tests that mutate the default model
 # must restore to this value so later tests see a consistent baseline.

tests/conftest.py

@@ -72,10 +72,52 @@ def _auto_state_dir_name(repo_root) -> str:
 TEST_PORT      = int(os.getenv('HERMES_WEBUI_TEST_PORT',
                                str(_auto_test_port(REPO_ROOT))))
 TEST_BASE      = f"http://127.0.0.1:{TEST_PORT}"
+
+# ── Test state dir: HARD-ISOLATED from production ──────────────────────────
+# Test state must NEVER live inside the real Hermes home (~/.hermes or any
+# HERMES_HOME), or anywhere near production profiles/files/server. Earlier this
+# defaulted to ``HERMES_HOME / webui-test-<hash>`` which wrote test state INTO
+# ~/.hermes/profiles/<...>/ (observed: 144 leaked webui-test-* dirs in the real
+# profile home). We now anchor the default under the OS temp dir, in a dedicated
+# `hermes-webui-tests/` namespace, fully outside any production tree.
+import tempfile as _tempfile
+_TEST_STATE_ROOT = pathlib.Path(
+    os.getenv('HERMES_WEBUI_TEST_STATE_ROOT', _tempfile.gettempdir())
+) / 'hermes-webui-tests'
 TEST_STATE_DIR = pathlib.Path(os.getenv(
     'HERMES_WEBUI_TEST_STATE_DIR',
-    str(HERMES_HOME / _auto_state_dir_name(REPO_ROOT))
-))
+    str(_TEST_STATE_ROOT / _auto_state_dir_name(REPO_ROOT))
+)).resolve()
+
+# Production-proximity guard: refuse to run if the resolved test state dir lands
+# inside the REAL Hermes home tree — a misconfigured HERMES_WEBUI_TEST_STATE_DIR
+# pointing at ~/.hermes would let tests wipe/clobber production profiles,
+# sessions, and credentials on teardown. We anchor on the literal user-home
+# `~/.hermes` (NOT $HERMES_HOME): HERMES_HOME is frequently overridden to a
+# profile dir or, during a test run, to TEST_STATE_DIR itself — comparing
+# against it would either miss a real production path or false-trip when the
+# test dir legitimately lives in /tmp. A test dir under the OS temp dir is always
+# allowed even if it nominally sits below a temp-rooted HERMES_HOME.
+_PROD_HERMES_HOME = (HOME / '.hermes').resolve()
+_TEMP_ROOT = pathlib.Path(_tempfile.gettempdir()).resolve()
+# The temp-root exception only holds when the temp root is itself OUTSIDE the
+# production home. If TMPDIR is (mis)configured under ~/.hermes, a "temp" path is
+# still a production path — don't let it suppress the guard.
+_temp_root_is_safe = not (
+    _TEMP_ROOT == _PROD_HERMES_HOME or _PROD_HERMES_HOME in _TEMP_ROOT.parents
+)
+_under_temp = _temp_root_is_safe and (
+    TEST_STATE_DIR == _TEMP_ROOT or _TEMP_ROOT in TEST_STATE_DIR.parents
+)
+_under_prod = TEST_STATE_DIR == _PROD_HERMES_HOME or _PROD_HERMES_HOME in TEST_STATE_DIR.parents
+if _under_prod and not _under_temp:
+    raise RuntimeError(
+        f"REFUSING TO RUN: test state dir {TEST_STATE_DIR} is inside the production "
+        f"Hermes home {_PROD_HERMES_HOME}. Tests must never touch production files. "
+        f"Unset HERMES_WEBUI_TEST_STATE_DIR (defaults to a temp dir) or point it "
+        f"outside ~/.hermes."
+    )
+
 TEST_WORKSPACE = TEST_STATE_DIR / 'test-workspace'
 
 # Publish at module level so api.config, _pytest_port.py, and any test module
@@ -685,13 +727,21 @@ def _kill_port_owner(port):
 
 
 def _rmtree_retry(path):
-    """Remove a tree, retrying on Windows after clearing read-only bits."""
+    """Remove a tree, retrying transient races (Linux + Windows).
+
+    The retry loop covers two real failure modes seen in CI/local:
+      * Windows: read-only bits / lingering handles (cleared via onexc/onerror).
+      * Linux: `OSError [Errno 39] Directory not empty` — a background thread or
+        daemon (model-metadata fetcher, session-events watcher, profile writer)
+        creates a file inside the tree WHILE shutil.rmtree is walking it, so the
+        parent rmdir fails even though we just emptied it. A short retry lets the
+        racing writer settle; a final ignore_errors sweep guarantees teardown
+        never fails the test over a pure cleanup race (the dir is abandoned test
+        state, not an assertion target).
+    """
     target = pathlib.Path(path)
     if not target.exists():
         return
-    if sys.platform != "win32":
-        shutil.rmtree(target)
-        return
 
     def _clear_readonly(_func, entry, _exc):
         try:
@@ -704,7 +754,7 @@ def _rmtree_retry(path):
         {"onexc": _clear_readonly}
         if "onexc" in inspect.signature(shutil.rmtree).parameters
         else {"onerror": _clear_readonly}
-    )
+    ) if sys.platform == "win32" else {}
 
     attempts = 5
     last_exc = None
@@ -720,13 +770,27 @@ def _rmtree_retry(path):
                 break
             time.sleep(0.3)
 
+    # Final fallback: a concurrent-writer race (Errno 39) shouldn't fail teardown.
+    # Best-effort ignore_errors sweep; if anything remains it's abandoned test
+    # state under HERMES_HOME, not something a test asserts on.
+    shutil.rmtree(target, ignore_errors=True)
+    if not target.exists():
+        return
     leftovers = []
     try:
         leftovers = [child.name for child in list(target.iterdir())[:5]]
     except Exception:
         pass
+    # Don't raise — log-and-continue. Raising here turns a benign cleanup race
+    # into a spurious test ERROR (the behaviour #4283-area tests hit when a
+    # model-metadata background fetch repopulated the profile dir mid-teardown).
+    import warnings as _warnings
     leftover_note = f" leftovers={leftovers}" if leftovers else ""
-    raise RuntimeError(f"Could not remove {target} after {attempts} attempts.{leftover_note}") from last_exc
+    _warnings.warn(
+        f"_rmtree_retry: could not fully remove {target} after {attempts} attempts"
+        f"{leftover_note} (last_exc={last_exc!r}); left for OS temp cleanup.",
+        stacklevel=2,
+    )
 
 
 # ── Session-scoped test server ────────────────────────────────────────────────

tests/test_gateway_sync.py

@@ -48,7 +48,7 @@ def _get_test_state_dir():
     (via os.environ.setdefault) so that tests writing directly to state.db always
     use the same path the test server was started with.  If the env var is not
     set (e.g. when running this file standalone), fall back to the conftest
-    formula: HERMES_HOME/webui-mvp-test.
+    formula via _pytest_port (temp-rooted, never ~/.hermes).
     """
     # Use _pytest_port which applies the same auto-derivation as conftest.py
     from tests._pytest_port import TEST_STATE_DIR as _ptsd

tests/test_onboarding_existing_config.py

@@ -290,7 +290,8 @@ def _server_hermes_home() -> pathlib.Path:
     env_path = data.get("system", {}).get("env_path", "")
     if env_path:
         return pathlib.Path(env_path).parent
-    return pathlib.Path(os.environ.get("HERMES_WEBUI_TEST_STATE_DIR", str(pathlib.Path.home() / ".hermes" / "webui-mvp-test")))
+    from tests._pytest_port import TEST_STATE_DIR
+    return pathlib.Path(os.environ.get("HERMES_WEBUI_TEST_STATE_DIR", str(TEST_STATE_DIR)))
 
 
 def _server_reachable() -> bool:

tests/test_regressions.py

@@ -116,7 +116,8 @@ def test_session_with_tool_calls_in_json_loads_ok(cleanup_test_sessions):
     sid = make_session(cleanup_test_sessions)
 
     # Manually inject tool_calls into the session's JSON file
-    sessions_dir = pathlib.Path(os.environ.get("HERMES_WEBUI_TEST_STATE_DIR", str(pathlib.Path.home() / ".hermes" / "webui-mvp-test"))) / "sessions"
+    from tests._pytest_port import TEST_STATE_DIR
+    sessions_dir = pathlib.Path(os.environ.get("HERMES_WEBUI_TEST_STATE_DIR", str(TEST_STATE_DIR))) / "sessions"
     session_file = sessions_dir / f"{sid}.json"
     if session_file.exists():
         d = json.loads(session_file.read_text())

tests/test_sprint45.py

@@ -14,15 +14,14 @@ import urllib.request
 
 import os
 
-from tests._pytest_port import BASE
+from tests._pytest_port import BASE, TEST_STATE_DIR
 REPO = pathlib.Path(__file__).parent.parent
 # Use HERMES_WEBUI_TEST_STATE_DIR if available (set by conftest for the test process),
-# falling back to the conventional webui-mvp-test path.
+# falling back to the shared isolated TEST_STATE_DIR (temp-rooted, never ~/.hermes).
 def _get_settings_file() -> pathlib.Path:
     """Resolve SETTINGS_FILE at call time (env var set by conftest after module import)."""
     state_dir = pathlib.Path(
-        os.environ.get("HERMES_WEBUI_TEST_STATE_DIR",
-                       str(pathlib.Path.home() / ".hermes" / "webui-mvp-test"))
+        os.environ.get("HERMES_WEBUI_TEST_STATE_DIR", str(TEST_STATE_DIR))
     )
     return state_dir / "settings.json"
 

tests/test_sprint5.py

@@ -225,8 +225,9 @@ def test_file_save_path_traversal_blocked(cleanup_test_sessions):
     assert status in (400, 500)
 
 def test_session_index_created_after_save(cleanup_test_sessions):
-    # Index is created in the TEST state dir, not the production dir
-    test_state_dir = pathlib.Path(os.environ.get("HERMES_WEBUI_TEST_STATE_DIR", str(pathlib.Path.home() / ".hermes" / "webui-mvp-test")))
+    # Index is created in the TEST state dir, not the production dir.
+    # Use the shared isolated TEST_STATE_DIR (temp-rooted, never ~/.hermes).
+    from tests._pytest_port import TEST_STATE_DIR as test_state_dir
     index_path = test_state_dir / "sessions" / "_index.json"
     make_session_tracked(cleanup_test_sessions)
     # Index may not exist yet if cleanup already wiped it -- just check the endpoint works