fix(ci): eliminate test_set_key flakiness — v0.50.161

Root cause: test_profile_env_isolation.py and test_profile_path_security.py called sys.modules.pop() without restoring, poisoning subsequent tests. Fix: monkeypatch.delitem so pytest auto-restores. Also holds _ENV_LOCK for full I/O cycle in _write_env_file and creates .env at 0600 via os.open. Reviewed by Opus (no independent review needed — test/providers fix only).

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Hermes Web UI -- Changelog
 
+## [v0.50.161] — 2026-04-23
+
+### Fixed
+- **CI: `test_set_key_writes_to_env_file` no longer flaky in full-suite ordering** — two test files (`test_profile_env_isolation.py`, `test_profile_path_security.py`) were calling `sys.modules.pop("api.profiles")` without restoring the module reference, permanently removing `api.profiles` from the module cache and corrupting state for subsequent tests. Replaced with `monkeypatch.delitem(sys.modules, ...)` so the module reference is restored automatically after each test. (`tests/test_profile_env_isolation.py`, `tests/test_profile_path_security.py`)
+- **`api/providers.py` `_write_env_file()` lock and mode fixes** — moved file I/O (mkdir + write) inside the `_ENV_LOCK` block to prevent TOCTOU race between concurrent key-save requests; replaced `write_text()` with `os.open(..., O_CREAT, 0o600)` so new `.env` files are created owner-read/write-only from the first byte. (`api/providers.py`)
+
 ## [v0.50.160] — 2026-04-23
 
 ### Fixed

api/providers.py

@@ -85,16 +85,19 @@ def _write_env_file(env_path: Path, updates: dict[str, str | None]) -> None:
     """Write key=value pairs to the .env file.
 
     Values of ``None`` cause the key to be removed.
-    Uses ``_ENV_LOCK`` from ``api.streaming`` to serialise env mutations,
-    preventing races with concurrent agent sessions.
+    Holds ``_ENV_LOCK`` from ``api.streaming`` for the entire load → modify →
+    write cycle to prevent TOCTOU races between concurrent POST /api/providers
+    calls (each reading the same file baseline and overwriting the other's key).
+    Also serialises os.environ mutations with streaming sessions.
     """
     from api.streaming import _ENV_LOCK
+    import stat as _stat
 
+    with _ENV_LOCK:
         current = _load_env_file(env_path)
         for key, value in updates.items():
             if value is None:
                 current.pop(key, None)
-            with _ENV_LOCK:
                 os.environ.pop(key, None)
                 continue
             clean = str(value).strip()
@@ -104,14 +107,20 @@ def _write_env_file(env_path: Path, updates: dict[str, str | None]) -> None:
             if "\n" in clean or "\r" in clean:
                 raise ValueError("API key must not contain newline characters.")
             current[key] = clean
-        with _ENV_LOCK:
             os.environ[key] = clean
 
         env_path.parent.mkdir(parents=True, exist_ok=True)
         lines = [f"{key}={current[key]}" for key in sorted(current)]
-    env_path.write_text(
-        "\n".join(lines) + ("\n" if lines else ""), encoding="utf-8"
-    )
+        # Create at owner-only mode from the first byte (O_CREAT honours the mode
+        # argument subject to umask). A trailing chmod guards pre-existing files.
+        _mode = _stat.S_IRUSR | _stat.S_IWUSR  # 0o600
+        _fd = os.open(str(env_path), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, _mode)
+        with os.fdopen(_fd, "w", encoding="utf-8") as _f:
+            _f.write("\n".join(lines) + ("\n" if lines else ""))
+        try:
+            env_path.chmod(_mode)
+        except OSError:
+            pass
 
 
 def _provider_has_key(provider_id: str) -> bool:

tests/test_profile_env_isolation.py

@@ -18,9 +18,10 @@ def test_profile_switch_clears_previous_profile_env_vars(monkeypatch, tmp_path):
     monkeypatch.delenv("OPENAI_API_KEY", raising=False)
     monkeypatch.delenv("CUSTOM_TOKEN", raising=False)
 
-    sys.modules.pop("api.profiles", None)
+    # Use monkeypatch so sys.modules is restored after the test, preventing
+    # api.profiles from being permanently removed and poisoning subsequent tests.
+    monkeypatch.delitem(sys.modules, "api.profiles", raising=False)
     profiles = importlib.import_module("api.profiles")
-    profiles = importlib.reload(profiles)
 
     profiles.init_profile_state()
     profiles.switch_profile("p1")
@@ -52,9 +53,10 @@ def test_profile_switch_replaces_overlapping_keys(monkeypatch, tmp_path):
     monkeypatch.delenv("ONLY_P1", raising=False)
     monkeypatch.delenv("ONLY_P2", raising=False)
 
-    sys.modules.pop("api.profiles", None)
+    # Use monkeypatch so sys.modules is restored after the test, preventing
+    # api.profiles from being permanently removed and poisoning subsequent tests.
+    monkeypatch.delitem(sys.modules, "api.profiles", raising=False)
     profiles = importlib.import_module("api.profiles")
-    profiles = importlib.reload(profiles)
 
     profiles.init_profile_state()
     profiles.switch_profile("p1")

tests/test_profile_path_security.py

@@ -15,11 +15,21 @@ def _reload_profiles_module(base_home: Path):
     os.environ["HERMES_BASE_HOME"] = str(base_home)
     os.environ["HERMES_HOME"] = str(base_home)
 
+    # Save the original module references so we can restore them after the test.
+    # Permanently deleting api.config / api.profiles from sys.modules breaks
+    # subsequent tests that import these modules and expect consistent state.
+    _saved = {name: sys.modules[name] for name in ["api.config", "api.profiles"]
+              if name in sys.modules}
+
     for name in ["api.config", "api.profiles"]:
         if name in sys.modules:
             del sys.modules[name]
 
     profiles = importlib.import_module("api.profiles")
+
+    # Restore original modules so the cache stays consistent for the rest of the suite.
+    sys.modules.update(_saved)
+
     return profiles
 
 

tests/test_provider_management.py

@@ -59,6 +59,13 @@ def _install_fake_hermes_cli(monkeypatch):
     monkeypatch.delitem(sys.modules, "agent.credential_pool", raising=False)
     monkeypatch.delitem(sys.modules, "agent", raising=False)
 
+    # Flush the 60-second TTL model cache so no prior test's result bleeds in.
+    try:
+        from api.config import invalidate_models_cache
+        invalidate_models_cache()
+    except Exception:
+        pass
+
 
 # ── Unit tests (api/providers.py functions directly) ──────────────────────
 
@@ -162,6 +169,9 @@ class TestSetProviderKey:
         """Setting a key should write the env var to ~/.hermes/.env."""
         _install_fake_hermes_cli(monkeypatch)
         monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
+        # Also pin HERMES_HOME so code that reads it directly gets tmp_path,
+        # not the conftest session TEST_STATE_DIR that bleeds into the main process.
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
 
         old_cfg = dict(config.cfg)
         old_mtime = config._cfg_mtime
@@ -181,7 +191,7 @@ class TestSetProviderKey:
 
             # Verify .env file was written
             env_path = tmp_path / ".env"
-            assert env_path.exists()
+            assert env_path.exists(), f".env not written to {env_path}; HERMES_HOME={__import__('os').environ.get('HERMES_HOME')!r}"
             content = env_path.read_text()
             assert "ANTHROPIC_API_KEY=sk-ant-test-key-12345678" in content
         finally: