fix(profiles): avoid config import cycle
This commit is contained in:
@@ -26,76 +26,15 @@ from pathlib import Path
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
# ── Basic layout ──────────────────────────────────────────────────────────────
|
||||
HOME = Path.home()
|
||||
import api.paths as _paths
|
||||
|
||||
HOME = _paths.HOME
|
||||
_hermes_home_has_webui_state = _paths._hermes_home_has_webui_state
|
||||
_platform_default_hermes_home = _paths._platform_default_hermes_home
|
||||
|
||||
# REPO_ROOT is the directory that contains this file's parent (api/ -> repo root)
|
||||
REPO_ROOT = Path(__file__).parent.parent.resolve()
|
||||
|
||||
|
||||
def _hermes_home_has_webui_state(base: Path) -> bool:
|
||||
"""Return True when *base* holds real WebUI state under its ``webui/`` dir.
|
||||
|
||||
Used only on Windows to detect a pre-v0.51.134 install at the legacy
|
||||
``%USERPROFILE%\\.hermes`` location so we don't strand the user's existing
|
||||
sessions/pins/settings when the default moved to ``%LOCALAPPDATA%\\hermes``
|
||||
(#2905).
|
||||
|
||||
We intentionally check ONLY WebUI-owned artifacts (the ``webui/`` subtree),
|
||||
NOT agent-owned files like ``config.yaml`` / ``auth.json``. The agent has
|
||||
defaulted to ``%LOCALAPPDATA%\\hermes`` on Windows since before #2897, so a
|
||||
long-time agent user who never ran WebUI at the legacy location would have a
|
||||
stray ``auth.json`` there — keying on that would wrongly divert a *fresh*
|
||||
WebUI install to the legacy dir. Only ``webui/`` state is what actually
|
||||
gets stranded by the move, so it is the correct and narrow signal.
|
||||
Cheap stat-only checks; never raises.
|
||||
"""
|
||||
try:
|
||||
if not base.is_dir():
|
||||
return False
|
||||
markers = (
|
||||
base / "webui" / "sessions", # WebUI session store
|
||||
base / "webui" / "settings.json", # WebUI UI settings + pins
|
||||
base / "webui", # WebUI state dir at all
|
||||
)
|
||||
return any(m.exists() for m in markers)
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
def _platform_default_hermes_home() -> Path:
|
||||
"""Return the platform-aware default Hermes home when HERMES_HOME is unset.
|
||||
|
||||
Native Windows Hermes Agent installs default to %LOCALAPPDATA%\\hermes,
|
||||
while POSIX installs use ~/.hermes.
|
||||
|
||||
Windows migration safety (#2905): v0.51.134 moved the Windows default from
|
||||
``%USERPROFILE%\\.hermes`` to ``%LOCALAPPDATA%\\hermes`` to match the agent.
|
||||
Upgrading users whose WebUI state still lives at the old location saw an
|
||||
empty app (sessions/pins/settings "lost" — actually just at an address the
|
||||
new build no longer reads). To avoid stranding that data, prefer the
|
||||
legacy ``%USERPROFILE%\\.hermes`` ONLY when it is populated AND the new
|
||||
``%LOCALAPPDATA%\\hermes`` location is not yet established. This is a
|
||||
non-destructive, self-healing fallback: no files are moved, and once the
|
||||
new location has state (fresh installs, or users who set HERMES_HOME) the
|
||||
legacy path is never preferred. Explicit HERMES_HOME / HERMES_WEBUI_STATE_DIR
|
||||
overrides take precedence upstream and are unaffected.
|
||||
"""
|
||||
if os.name == "nt":
|
||||
local_app_data = os.getenv("LOCALAPPDATA", "").strip()
|
||||
if local_app_data:
|
||||
new_home = Path(local_app_data) / "hermes"
|
||||
legacy_home = HOME / ".hermes"
|
||||
# Only fall back to the legacy home if it actually holds state and
|
||||
# the new location has not been established yet — the exact
|
||||
# post-upgrade fingerprint from #2905.
|
||||
if (
|
||||
legacy_home != new_home
|
||||
and not _hermes_home_has_webui_state(new_home)
|
||||
and _hermes_home_has_webui_state(legacy_home)
|
||||
):
|
||||
return legacy_home
|
||||
return new_home
|
||||
return HOME / ".hermes"
|
||||
|
||||
# ── Network config (env-overridable) ─────────────────────────────────────────
|
||||
HOST = os.getenv("HERMES_WEBUI_HOST", "127.0.0.1")
|
||||
PORT = int(os.getenv("HERMES_WEBUI_PORT", "8787"))
|
||||
|
||||
77
api/paths.py
Normal file
77
api/paths.py
Normal file
@@ -0,0 +1,77 @@
|
||||
"""Shared path helpers for Hermes WebUI.
|
||||
|
||||
Keep low-level filesystem defaults here instead of in ``api.config`` so modules
|
||||
that need the default Hermes home can import them without triggering config's
|
||||
larger startup side effects.
|
||||
"""
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
HOME = Path.home()
|
||||
|
||||
|
||||
def _hermes_home_has_webui_state(base: Path) -> bool:
|
||||
"""Return True when *base* holds real WebUI state under its ``webui/`` dir.
|
||||
|
||||
Used only on Windows to detect a pre-v0.51.134 install at the legacy
|
||||
``%USERPROFILE%\\.hermes`` location so we don't strand the user's existing
|
||||
sessions/pins/settings when the default moved to ``%LOCALAPPDATA%\\hermes``
|
||||
(#2905).
|
||||
|
||||
We intentionally check ONLY WebUI-owned artifacts (the ``webui/`` subtree),
|
||||
NOT agent-owned files like ``config.yaml`` / ``auth.json``. The agent has
|
||||
defaulted to ``%LOCALAPPDATA%\\hermes`` on Windows since before #2897, so a
|
||||
long-time agent user who never ran WebUI at the legacy location would have a
|
||||
stray ``auth.json`` there — keying on that would wrongly divert a *fresh*
|
||||
WebUI install to the legacy dir. Only ``webui/`` state is what actually
|
||||
gets stranded by the move, so it is the correct and narrow signal.
|
||||
Cheap stat-only checks; never raises.
|
||||
"""
|
||||
try:
|
||||
if not base.is_dir():
|
||||
return False
|
||||
markers = (
|
||||
base / "webui" / "sessions", # WebUI session store
|
||||
base / "webui" / "settings.json", # WebUI UI settings + pins
|
||||
base / "webui", # WebUI state dir at all
|
||||
)
|
||||
return any(m.exists() for m in markers)
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
def _platform_default_hermes_home() -> Path:
|
||||
"""Return the platform-aware default Hermes home when HERMES_HOME is unset.
|
||||
|
||||
Native Windows Hermes Agent installs default to %LOCALAPPDATA%\\hermes,
|
||||
while POSIX installs use ~/.hermes.
|
||||
|
||||
Windows migration safety (#2905): v0.51.134 moved the Windows default from
|
||||
``%USERPROFILE%\\.hermes`` to ``%LOCALAPPDATA%\\hermes`` to match the agent.
|
||||
Upgrading users whose WebUI state still lives at the old location saw an
|
||||
empty app (sessions/pins/settings "lost" — actually just at an address the
|
||||
new build no longer reads). To avoid stranding that data, prefer the
|
||||
legacy ``%USERPROFILE%\\.hermes`` ONLY when it is populated AND the new
|
||||
``%LOCALAPPDATA%\\hermes`` location is not yet established. This is a
|
||||
non-destructive, self-healing fallback: no files are moved, and once the
|
||||
new location has state (fresh installs, or users who set HERMES_HOME) the
|
||||
legacy path is never preferred. Explicit HERMES_HOME / HERMES_WEBUI_STATE_DIR
|
||||
overrides take precedence upstream and are unaffected.
|
||||
"""
|
||||
if os.name == "nt":
|
||||
local_app_data = os.getenv("LOCALAPPDATA", "").strip()
|
||||
if local_app_data:
|
||||
new_home = Path(local_app_data) / "hermes"
|
||||
legacy_home = HOME / ".hermes"
|
||||
# Only fall back to the legacy home if it actually holds state and
|
||||
# the new location has not been established yet — the exact
|
||||
# post-upgrade fingerprint from #2905.
|
||||
if (
|
||||
legacy_home != new_home
|
||||
and not _hermes_home_has_webui_state(new_home)
|
||||
and _hermes_home_has_webui_state(legacy_home)
|
||||
):
|
||||
return legacy_home
|
||||
return new_home
|
||||
return HOME / ".hermes"
|
||||
@@ -152,20 +152,13 @@ def _resolve_base_hermes_home() -> Path:
|
||||
|
||||
# Platform default. On Windows this includes the #2905 migration-safety
|
||||
# fallback (prefer the populated legacy %USERPROFILE%\.hermes over an
|
||||
# empty %LOCALAPPDATA%\hermes). Delegate to config so the base-home
|
||||
# resolution used for the active-profile pointer can never drift from the
|
||||
# one config.STATE_DIR is derived from.
|
||||
try:
|
||||
from api.config import _platform_default_hermes_home
|
||||
return _platform_default_hermes_home()
|
||||
except ImportError:
|
||||
# Defensive: never let a config import problem break profile resolution.
|
||||
# Scoped to ImportError so a real bug inside the helper still surfaces.
|
||||
if os.name == 'nt':
|
||||
local_app_data = os.getenv('LOCALAPPDATA', '').strip()
|
||||
if local_app_data:
|
||||
return Path(local_app_data) / 'hermes'
|
||||
return Path.home() / '.hermes'
|
||||
# empty %LOCALAPPDATA%\hermes). Import the shared path helper directly
|
||||
# instead of importing api.config here; api.config imports profiles during
|
||||
# startup, so going through config creates a partial-module circular import
|
||||
# when api.profiles is imported first.
|
||||
from api.paths import _platform_default_hermes_home
|
||||
|
||||
return _platform_default_hermes_home()
|
||||
|
||||
_DEFAULT_HERMES_HOME = _resolve_base_hermes_home()
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@ from pathlib import Path
|
||||
import pytest
|
||||
|
||||
import api.config as config
|
||||
import api.paths as paths
|
||||
|
||||
|
||||
class _WindowsOSShim:
|
||||
@@ -62,8 +63,8 @@ def windows_env(monkeypatch, tmp_path):
|
||||
legacy_home = home / ".hermes"
|
||||
new_home = localappdata / "hermes"
|
||||
|
||||
monkeypatch.setattr(config, "HOME", home)
|
||||
monkeypatch.setattr(config, "os", _WindowsOSShim())
|
||||
monkeypatch.setattr(paths, "HOME", home)
|
||||
monkeypatch.setattr(paths, "os", _WindowsOSShim())
|
||||
monkeypatch.setenv("LOCALAPPDATA", str(localappdata))
|
||||
monkeypatch.delenv("HERMES_HOME", raising=False)
|
||||
monkeypatch.delenv("HERMES_BASE_HOME", raising=False)
|
||||
@@ -135,7 +136,8 @@ def test_does_nothing_on_posix(monkeypatch, tmp_path):
|
||||
regardless of any LOCALAPPDATA value — the fix is Windows-only."""
|
||||
home = tmp_path / "home"
|
||||
home.mkdir()
|
||||
monkeypatch.setattr(config, "HOME", home)
|
||||
monkeypatch.setattr(paths, "HOME", home)
|
||||
monkeypatch.setattr(paths, "os", os)
|
||||
# real os.name is 'posix' on CI; do NOT swap in the Windows shim
|
||||
monkeypatch.setenv("LOCALAPPDATA", str(tmp_path / "lad"))
|
||||
monkeypatch.delenv("HERMES_HOME", raising=False)
|
||||
@@ -165,23 +167,23 @@ class TestHermesHomeHasWebuiState:
|
||||
"""Unit coverage for the marker-detection helper."""
|
||||
|
||||
def test_empty_or_missing_dir_is_not_state(self, tmp_path):
|
||||
assert config._hermes_home_has_webui_state(tmp_path / "nope") is False
|
||||
assert paths._hermes_home_has_webui_state(tmp_path / "nope") is False
|
||||
empty = tmp_path / "empty"
|
||||
empty.mkdir()
|
||||
assert config._hermes_home_has_webui_state(empty) is False
|
||||
assert paths._hermes_home_has_webui_state(empty) is False
|
||||
|
||||
def test_webui_sessions_marker_counts(self, tmp_path):
|
||||
(tmp_path / "webui" / "sessions").mkdir(parents=True)
|
||||
assert config._hermes_home_has_webui_state(tmp_path) is True
|
||||
assert paths._hermes_home_has_webui_state(tmp_path) is True
|
||||
|
||||
def test_webui_settings_marker_counts(self, tmp_path):
|
||||
(tmp_path / "webui").mkdir()
|
||||
(tmp_path / "webui" / "settings.json").write_text("{}", encoding="utf-8")
|
||||
assert config._hermes_home_has_webui_state(tmp_path) is True
|
||||
assert paths._hermes_home_has_webui_state(tmp_path) is True
|
||||
|
||||
def test_webui_dir_alone_counts(self, tmp_path):
|
||||
(tmp_path / "webui").mkdir()
|
||||
assert config._hermes_home_has_webui_state(tmp_path) is True
|
||||
assert paths._hermes_home_has_webui_state(tmp_path) is True
|
||||
|
||||
def test_agent_only_artifacts_do_not_count(self, tmp_path):
|
||||
"""A home with ONLY agent files (config.yaml / auth.json) and no webui/
|
||||
@@ -189,17 +191,17 @@ class TestHermesHomeHasWebuiState:
|
||||
installing WebUI fresh would be wrongly diverted to the legacy dir."""
|
||||
(tmp_path / "config.yaml").write_text("model: x\n", encoding="utf-8")
|
||||
(tmp_path / "auth.json").write_text("{}", encoding="utf-8")
|
||||
assert config._hermes_home_has_webui_state(tmp_path) is False
|
||||
assert paths._hermes_home_has_webui_state(tmp_path) is False
|
||||
|
||||
|
||||
def test_profiles_base_home_delegates_to_config(monkeypatch, tmp_path):
|
||||
"""profiles._resolve_base_hermes_home() must share config's resolution so
|
||||
def test_profiles_base_home_uses_shared_path_helper(monkeypatch, tmp_path):
|
||||
"""profiles._resolve_base_hermes_home() must share config's path helper so
|
||||
the active-profile pointer never diverges from config.STATE_DIR (#2905)."""
|
||||
import api.profiles as profiles
|
||||
|
||||
sentinel = tmp_path / "sentinel-home"
|
||||
monkeypatch.delenv("HERMES_HOME", raising=False)
|
||||
monkeypatch.delenv("HERMES_BASE_HOME", raising=False)
|
||||
monkeypatch.setattr(config, "_platform_default_hermes_home", lambda: sentinel)
|
||||
monkeypatch.setattr(paths, "_platform_default_hermes_home", lambda: sentinel)
|
||||
|
||||
assert profiles._resolve_base_hermes_home() == sentinel
|
||||
|
||||
50
tests/test_issue3283_profiles_config_import_order.py
Normal file
50
tests/test_issue3283_profiles_config_import_order.py
Normal file
@@ -0,0 +1,50 @@
|
||||
"""Regression coverage for #3283 profile/config import ordering.
|
||||
|
||||
Importing ``api.profiles`` before ``api.config`` used to trigger a circular import
|
||||
through ``profiles._resolve_base_hermes_home() -> api.config``. ``api.config``
|
||||
then caught the partial-module ``ImportError`` from its startup
|
||||
``init_profile_state`` import, so the later config import never initialized the
|
||||
sticky active profile.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def test_profiles_first_then_config_still_initializes_active_profile(tmp_path):
|
||||
home = tmp_path / "home"
|
||||
base = home / ".hermes"
|
||||
profile_home = base / "profiles" / "webui"
|
||||
profile_home.mkdir(parents=True)
|
||||
(base / "active_profile").write_text("webui", encoding="utf-8")
|
||||
|
||||
repo_root = Path(__file__).resolve().parents[1]
|
||||
env = os.environ.copy()
|
||||
env.pop("HERMES_HOME", None)
|
||||
env.pop("HERMES_BASE_HOME", None)
|
||||
env.pop("HERMES_WEBUI_STATE_DIR", None)
|
||||
env["HOME"] = str(home)
|
||||
env["PYTHONPATH"] = str(repo_root)
|
||||
|
||||
code = """
|
||||
import os
|
||||
import api.profiles
|
||||
import api.config
|
||||
print(os.environ.get('HERMES_HOME', ''))
|
||||
"""
|
||||
proc = subprocess.run(
|
||||
[sys.executable, "-c", code],
|
||||
cwd=repo_root,
|
||||
env=env,
|
||||
text=True,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
timeout=20,
|
||||
check=False,
|
||||
)
|
||||
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
assert proc.stdout.strip() == str(profile_home)
|
||||
Reference in New Issue
Block a user