Two urgent breaking bugs that had no PR, combined into one hotfix. #2905 (data-loss-class): v0.51.134 moved the Windows default Hermes home from %USERPROFILE%\.hermes to %LOCALAPPDATA%\hermes (PR #2897) with no migration, so upgrading Windows users opened the app to empty sessions/pins/settings (data intact on disk, at an address the new build no longer read). _platform_default_hermes_home() now prefers the populated legacy home ONLY when the new location is not yet established — non-destructive (no file moves) and self-healing on next launch. profiles._resolve_base_hermes_home() delegates to the same config helper so the active-profile pointer can never drift from STATE_DIR. #3194: GET /api/gateway/status reported 'Gateway not configured' on a fresh two-container Docker deploy because an alive=None + gateway_stale_running_state health payload with an empty identity_map fell through to configured=bool(identity_map)=False. The alive=None branch now treats a payload carrying gateway metadata (gateway_state detail, or a stale-running/ stale-stopped reason) as configured. Tests: +17 regression tests (11 for #2905 incl. full truth table + non-destructive guard + POSIX no-op; 6 for #3194 incl. 5 no-regression guards). Full suite 7090 passed, 0 failed. Closes #2905 Closes #3194
This commit is contained in:
1 parent
1d4d40b16d
commit
5dc11ef977
5 files changed
+416
-8
No files matched your search
+48
-1
@@ -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) ─────────────────────────────────────────
|
||||
|
||||
+15
-5
@@ -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()
|
||||
|
||||
|
||||
+17
-2
@@ -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():
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
Reference in new issue
Block a user