diff --git a/api/config.py b/api/config.py index 0889c613..dc6bc0d9 100644 --- a/api/config.py +++ b/api/config.py @@ -31,16 +31,63 @@ HOME = Path.home() REPO_ROOT = Path(__file__).parent.parent.resolve() +def _hermes_home_has_webui_state(base: Path) -> bool: + """Return True when *base* looks like a populated Hermes home with WebUI state. + + 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 treat the home as "real" if any of the durable WebUI/agent + artifacts exist under it. 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 + base / "config.yaml", # agent config + base / "auth.json", # agent credentials + ) + 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: - return Path(local_app_data) / "hermes" + 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) ───────────────────────────────────────── diff --git a/api/profiles.py b/api/profiles.py index 66356ce3..ca18fc0f 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -150,11 +150,21 @@ def _resolve_base_hermes_home() -> Path: # If HERMES_HOME points to a profiles/ subdir, walk up two levels to the base return _unwrap_profile_home_to_base(p) - 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' + # 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 Exception: + # Defensive: never let a config import problem break profile resolution. + 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' _DEFAULT_HERMES_HOME = _resolve_base_hermes_home() diff --git a/api/routes.py b/api/routes.py index 86edbfef..ca68517a 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5247,9 +5247,24 @@ def handle_get(handler, parsed) -> bool: elif alive is False: running = False configured = True - else: # alive is None → gateway not configured / unavailable + else: # alive is None + # `alive is None` conflates two very different states: + # (a) no gateway metadata at all → genuinely not configured + # (b) gateway metadata EXISTS but is stale/inconclusive (e.g. a + # freshly-started gateway that hasn't ticked yet, or a + # cross-container gateway whose running-state went stale). + # Case (b) still proves a gateway is *configured* — the banner + # must not report "Gateway not configured" just because no + # conversation has happened yet and identity_map is empty (#3194). + details = health.get("details") or {} + has_gateway_metadata = bool(details.get("gateway_state")) or ( + details.get("reason") in { + "gateway_stale_running_state", + "gateway_stale_stopped_state", + } + ) + configured = True if has_gateway_metadata else bool(identity_map) running = bool(identity_map) - configured = bool(identity_map) platforms_set: set[str] = set() for meta in identity_map.values(): diff --git a/tests/test_issue2905_windows_home_migration_safety.py b/tests/test_issue2905_windows_home_migration_safety.py new file mode 100644 index 00000000..012234df --- /dev/null +++ b/tests/test_issue2905_windows_home_migration_safety.py @@ -0,0 +1,196 @@ +"""Regression coverage for #2905 — Windows upgrade stranding WebUI state. + +v0.51.134 (PR #2897) moved the Windows default Hermes home from +``%USERPROFILE%\\.hermes`` to ``%LOCALAPPDATA%\\hermes`` to match the agent. +Upgrading users whose WebUI sessions/pins/settings still lived at the old +location opened the app to an empty state — the data was intact on disk but at +an address the new build no longer read. + +The fix makes ``_platform_default_hermes_home()`` prefer the populated legacy +``%USERPROFILE%\\.hermes`` ONLY when the new ``%LOCALAPPDATA%\\hermes`` location +is not yet established. It is: + * non-destructive — no files are moved (a move would be its own data-loss risk) + * self-healing — affected users find their data on next launch, no action needed + * surgical — fresh installs / already-migrated users / explicit overrides are + completely unaffected. + +These tests fake Windows semantics on a POSIX CI host by swapping ``config.os`` +for a shim whose ``name`` is ``'nt'`` and pointing HOME / LOCALAPPDATA at temp +dirs. They assert the full truth table plus the no-regression guards. +""" +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +import api.config as config + + +class _WindowsOSShim: + """Stand-in for the ``os`` module that reports ``name == 'nt'``.""" + + name = "nt" + environ = os.environ + + def __getattr__(self, key): # delegate everything else to the real os + return getattr(os, key) + + def getenv(self, *args, **kwargs): + return os.getenv(*args, **kwargs) + + +def _populate_webui_state(base: Path) -> None: + (base / "webui" / "sessions").mkdir(parents=True, exist_ok=True) + (base / "webui" / "settings.json").write_text('{"pinned":["a"]}', encoding="utf-8") + (base / "config.yaml").write_text("model: x\n", encoding="utf-8") + + +@pytest.fixture +def windows_env(monkeypatch, tmp_path): + """Yield (legacy_home, new_home) with Windows path semantics faked. + + Returns the two candidate base homes; the caller populates whichever it + needs before calling ``config._platform_default_hermes_home()``. + """ + home = tmp_path / "userprofile" # %USERPROFILE% + localappdata = tmp_path / "localappdata" # %LOCALAPPDATA% + home.mkdir() + localappdata.mkdir() + + legacy_home = home / ".hermes" + new_home = localappdata / "hermes" + + monkeypatch.setattr(config, "HOME", home) + monkeypatch.setattr(config, "os", _WindowsOSShim()) + monkeypatch.setenv("LOCALAPPDATA", str(localappdata)) + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.delenv("HERMES_BASE_HOME", raising=False) + + return legacy_home, new_home + + +def test_upgrade_fingerprint_prefers_populated_legacy_home(windows_env): + """The #2905 bug case: legacy populated, new empty → resolve to legacy.""" + legacy_home, new_home = windows_env + _populate_webui_state(legacy_home) + # new_home intentionally left empty + + result = config._platform_default_hermes_home() + + assert result == legacy_home, ( + "Windows upgrade must not strand WebUI state: when %LOCALAPPDATA%/hermes " + "is empty but %USERPROFILE%/.hermes holds the user's sessions/pins, the " + "default home must resolve to the legacy location (#2905)." + ) + + +def test_fresh_install_uses_new_localappdata_home(windows_env): + """Neither location populated → use the new %LOCALAPPDATA% default (no regression on #2840).""" + legacy_home, new_home = windows_env + # both empty + + result = config._platform_default_hermes_home() + + assert result == new_home + + +def test_already_migrated_uses_new_home(windows_env): + """New location populated → never reach back to legacy.""" + legacy_home, new_home = windows_env + _populate_webui_state(new_home) + + result = config._platform_default_hermes_home() + + assert result == new_home + + +def test_both_populated_trusts_new_home(windows_env): + """If both exist, the new location wins — we never silently divert an + established %LOCALAPPDATA% install back to a stale legacy dir.""" + legacy_home, new_home = windows_env + _populate_webui_state(legacy_home) + _populate_webui_state(new_home) + + result = config._platform_default_hermes_home() + + assert result == new_home + + +def test_legacy_dir_present_but_empty_does_not_divert(windows_env): + """An empty/initialized-but-stateless legacy dir must NOT trigger the + fallback — otherwise a stray empty %USERPROFILE%/.hermes would shadow a + fresh install.""" + legacy_home, new_home = windows_env + legacy_home.mkdir(parents=True) # exists but no webui/config/auth markers + + result = config._platform_default_hermes_home() + + assert result == new_home + + +def test_does_nothing_on_posix(monkeypatch, tmp_path): + """On POSIX (os.name != 'nt') the resolver always returns ~/.hermes, + regardless of any LOCALAPPDATA value — the fix is Windows-only.""" + home = tmp_path / "home" + home.mkdir() + monkeypatch.setattr(config, "HOME", home) + # 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) + + result = config._platform_default_hermes_home() + + assert result == home / ".hermes" + + +def test_no_files_are_moved_by_resolution(windows_env): + """The fix is non-destructive: resolving the home must not create, move, + or delete anything at either location.""" + legacy_home, new_home = windows_env + _populate_webui_state(legacy_home) + legacy_sessions = legacy_home / "webui" / "sessions" + before_new_exists = new_home.exists() + + config._platform_default_hermes_home() + + # Legacy data untouched, new location not fabricated. + assert legacy_sessions.is_dir() + assert (legacy_home / "webui" / "settings.json").exists() + assert new_home.exists() == before_new_exists + + +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 + empty = tmp_path / "empty" + empty.mkdir() + assert config._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 + + def test_config_yaml_marker_counts(self, tmp_path): + (tmp_path / "config.yaml").write_text("x: 1\n", encoding="utf-8") + assert config._hermes_home_has_webui_state(tmp_path) is True + + def test_auth_json_marker_counts(self, tmp_path): + (tmp_path / "auth.json").write_text("{}", encoding="utf-8") + assert config._hermes_home_has_webui_state(tmp_path) is True + + +def test_profiles_base_home_delegates_to_config(monkeypatch, tmp_path): + """profiles._resolve_base_hermes_home() must share config's resolution 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) + + assert profiles._resolve_base_hermes_home() == sentinel diff --git a/tests/test_issue3194_gateway_configured_banner.py b/tests/test_issue3194_gateway_configured_banner.py new file mode 100644 index 00000000..8c75dc20 --- /dev/null +++ b/tests/test_issue3194_gateway_configured_banner.py @@ -0,0 +1,140 @@ +"""Regression coverage for #3194 — two-container Docker first-deploy shows +"Gateway not configured" even though the gateway is running. + +Reported by @chenghaopeng: after a fresh ``docker-compose.two-container.yml`` +deploy, the WebUI banner says "Gateway not configured" while +``hermes gateway status`` reports the gateway is running. The trigger is an +empty ``identity_map`` (no conversation has happened yet, so no session +metadata exists) combined with an ``alive is None`` health payload whose +``details.reason`` is ``gateway_stale_running_state`` (the gateway is up but +hasn't ticked ``updated_at`` recently enough for the freshness check). + +Before the fix, ``/api/gateway/status`` set ``configured = bool(identity_map)`` +on the ``alive is None`` branch, so an empty identity_map → ``configured=False`` +→ the misleading banner. The fix recognizes that an ``alive is None`` payload +which still carries gateway metadata (a ``gateway_state`` detail, or a stale- +running / stale-stopped reason) proves the gateway IS configured. + +Mirrors the FakeHandler isolation pattern in +``tests/test_gateway_status_agent_health.py``. +""" +from __future__ import annotations + +import json + + +class _FakeHandler: + """Minimal BaseHTTPRequestHandler stand-in for routes.handle_get.""" + + def __init__(self): + self.status = None + self.sent_headers = [] + self.body = bytearray() + self.wfile = self + + def send_response(self, code): + self.status = code + + def send_header(self, key, value): + self.sent_headers.append((key, value)) + + def end_headers(self): + pass + + def write(self, data): + self.body.extend(data if isinstance(data, (bytes, bytearray)) else data.encode("utf-8")) + + def get_json(self): + return json.loads(self.body.decode("utf-8")) + + +def _call_gateway_status(monkeypatch, *, health_payload, identity_map=None): + """Invoke handle_get for /api/gateway/status with a stubbed health payload.""" + from urllib.parse import urlparse + + from api import routes + + monkeypatch.setattr(routes, "build_agent_health_payload", lambda: health_payload) + monkeypatch.setattr( + routes, "_load_gateway_session_identity_map", lambda: (identity_map or {}) + ) + + handler = _FakeHandler() + parsed = urlparse("/api/gateway/status") + routes.handle_get(handler, parsed) + return handler.get_json() + + +def test_stale_running_with_empty_identity_map_is_configured(monkeypatch): + """#3194 core case: gateway up but not yet ticked, no sessions yet. + + alive=None + reason=gateway_stale_running_state + empty identity_map must + NOT report 'Gateway not configured'. + """ + payload = { + "alive": None, + "details": {"state": "unknown", "reason": "gateway_stale_running_state", + "gateway_state": "running"}, + } + data = _call_gateway_status(monkeypatch, health_payload=payload, identity_map={}) + + assert data["configured"] is True, ( + "A stale-running gateway with no conversations yet is still configured " + "— the banner must not say 'Gateway not configured' (#3194)." + ) + # No live tick / no sessions → not 'running' for the activity indicator, + # but that's a separate signal from 'configured'. + assert data["running"] is False + + +def test_gateway_state_detail_alone_marks_configured(monkeypatch): + """Any alive=None payload carrying a gateway_state detail is configured, + even if the reason string is something else.""" + payload = { + "alive": None, + "details": {"state": "unknown", "reason": "gateway_stale_stopped_state", + "gateway_state": "stopped"}, + } + data = _call_gateway_status(monkeypatch, health_payload=payload, identity_map={}) + assert data["configured"] is True + + +def test_truly_unconfigured_stays_unconfigured(monkeypatch): + """No-regression guard: alive=None with reason=gateway_not_configured and + no metadata and no identity_map → genuinely not configured.""" + payload = { + "alive": None, + "details": {"state": "unknown", "reason": "gateway_not_configured"}, + } + data = _call_gateway_status(monkeypatch, health_payload=payload, identity_map={}) + assert data["configured"] is False + assert data["running"] is False + + +def test_unconfigured_but_with_sessions_still_configured(monkeypatch): + """Pre-existing behavior preserved: even with no gateway metadata, a + non-empty identity_map implies a configured gateway.""" + payload = { + "alive": None, + "details": {"state": "unknown", "reason": "gateway_not_configured"}, + } + idmap = {"sid-1": {"platform": "telegram", "raw_source": "telegram"}} + data = _call_gateway_status(monkeypatch, health_payload=payload, identity_map=idmap) + assert data["configured"] is True + assert data["running"] is True + + +def test_alive_true_unaffected(monkeypatch): + """A live gateway is configured + running regardless of identity_map.""" + payload = {"alive": True, "details": {"state": "alive"}} + data = _call_gateway_status(monkeypatch, health_payload=payload, identity_map={}) + assert data["configured"] is True + assert data["running"] is True + + +def test_alive_false_configured_not_running(monkeypatch): + """alive=False (metadata exists, process down) stays configured-but-down.""" + payload = {"alive": False, "details": {"state": "down", "reason": "gateway_not_running"}} + data = _call_gateway_status(monkeypatch, health_payload=payload, identity_map={}) + assert data["configured"] is True + assert data["running"] is False