fix: apply Opus+Codex gate findings — stale-stopped not configured (#1944), narrow #2905 markers, scope except
Both pre-release reviewers (Opus advisor + Codex regression gate) converged on the same MUST-FIX: - #3194: treating gateway_stale_stopped_state as 'configured' contradicted #1944 (a stopped root gateway should read like 'not configured' so the banner doesn't nag). Now ONLY stale-RUNNING metadata (reason=gateway_stale_running_state or gateway_state=='running') flips configured=True; stale-stopped falls through to bool(identity_map) like the genuinely-unconfigured case. Updated the test accordingly + added a stale-stopped no-regression test. Opus follow-ups also applied: - #2905: narrowed the populated-home markers to WebUI-only artifacts (webui/, webui/sessions, webui/settings.json), dropping config.yaml/auth.json so a long-time agent user installing WebUI fresh isn't wrongly diverted to the legacy %USERPROFILE%\.hermes (auth.json predates #2897 there). - profiles._resolve_base_hermes_home(): narrowed except Exception -> ImportError so a real bug in the config helper still surfaces. Adjacent suites green: #2840, #1879, gateway_status_agent_health (66 tests).
This commit is contained in:
1 parent
5dc11ef977
commit
5742a23836
5 files changed
+64
-26
No files matched your search
+11
-5
@@ -32,13 +32,21 @@ 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.
|
||||
"""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 treat the home as "real" if any of the durable WebUI/agent
|
||||
artifacts exist under it. Cheap stat-only checks; never raises.
|
||||
(#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():
|
||||
@@ -47,8 +55,6 @@ def _hermes_home_has_webui_state(base: Path) -> bool:
|
||||
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:
|
||||
|
||||
+2
-1
@@ -158,8 +158,9 @@ def _resolve_base_hermes_home() -> Path:
|
||||
try:
|
||||
from api.config import _platform_default_hermes_home
|
||||
return _platform_default_hermes_home()
|
||||
except Exception:
|
||||
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:
|
||||
|
||||
+17
-12
@@ -5250,20 +5250,25 @@ def handle_get(handler, parsed) -> bool:
|
||||
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).
|
||||
# (b) gateway metadata EXISTS but is stale/inconclusive.
|
||||
# A stale-but-RUNNING gateway (freshly started, hasn't ticked
|
||||
# `updated_at` yet, or cross-container) still proves the gateway is
|
||||
# *configured* — the banner must not report "Gateway not configured"
|
||||
# just because no conversation has happened yet and identity_map is
|
||||
# empty (#3194).
|
||||
#
|
||||
# A stale-STOPPED gateway is deliberately NOT treated as configured:
|
||||
# agent_health emits `gateway_stale_stopped_state` precisely so a
|
||||
# stopped service the user isn't running reads like "no root gateway
|
||||
# configured" rather than nagging (#1944). So stale-stopped falls
|
||||
# through to the identity_map signal like the genuinely-unconfigured
|
||||
# case.
|
||||
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",
|
||||
}
|
||||
gateway_running_metadata = (
|
||||
details.get("reason") == "gateway_stale_running_state"
|
||||
or details.get("gateway_state") == "running"
|
||||
)
|
||||
configured = True if has_gateway_metadata else bool(identity_map)
|
||||
configured = True if gateway_running_metadata else bool(identity_map)
|
||||
running = bool(identity_map)
|
||||
|
||||
platforms_set: set[str] = set()
|
||||
|
||||
@@ -174,14 +174,23 @@ class TestHermesHomeHasWebuiState:
|
||||
(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")
|
||||
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
|
||||
|
||||
def test_auth_json_marker_counts(self, tmp_path):
|
||||
(tmp_path / "auth.json").write_text("{}", encoding="utf-8")
|
||||
def test_webui_dir_alone_counts(self, tmp_path):
|
||||
(tmp_path / "webui").mkdir()
|
||||
assert config._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/
|
||||
dir is NOT treated as WebUI state — otherwise a long-time agent user
|
||||
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
|
||||
|
||||
|
||||
def test_profiles_base_home_delegates_to_config(monkeypatch, tmp_path):
|
||||
"""profiles._resolve_base_hermes_home() must share config's resolution so
|
||||
|
||||
@@ -87,16 +87,33 @@ def test_stale_running_with_empty_identity_map_is_configured(monkeypatch):
|
||||
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."""
|
||||
def test_gateway_state_running_detail_marks_configured(monkeypatch):
|
||||
"""An alive=None payload whose details report gateway_state == 'running'
|
||||
is configured, even if the reason string differs — the running metadata
|
||||
is the signal."""
|
||||
payload = {
|
||||
"alive": None,
|
||||
"details": {"state": "unknown", "reason": "cross_container_freshness",
|
||||
"gateway_state": "running"},
|
||||
}
|
||||
data = _call_gateway_status(monkeypatch, health_payload=payload, identity_map={})
|
||||
assert data["configured"] is True
|
||||
|
||||
|
||||
def test_stale_stopped_with_empty_identity_map_not_configured(monkeypatch):
|
||||
"""No-regression for #1944: a stale-STOPPED gateway must NOT report
|
||||
configured when there's no traffic. agent_health emits
|
||||
gateway_stale_stopped_state precisely so a stopped service the user isn't
|
||||
running reads like 'no root gateway configured' rather than nagging.
|
||||
Only stale-RUNNING metadata flips configured=True (#3194)."""
|
||||
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
|
||||
assert data["configured"] is False
|
||||
assert data["running"] is False
|
||||
|
||||
|
||||
def test_truly_unconfigured_stays_unconfigured(monkeypatch):
|
||||
|
||||
Reference in new issue
Block a user