When a user switched profiles and created a new session, the session was saved to the default profile directory instead of the active profile directory — because get_hermes_home_for_profile() silently fell back to _DEFAULT_HERMES_HOME when the profile directory didn't exist yet on disk. Root cause: api/profiles.py:156 had `if profile_dir.is_dir(): return profile_dir; return _DEFAULT_HERMES_HOME`. New profiles (no session yet, so no dir) routed every session back to default. Fix: remove the is_dir() guard, return the profile path unconditionally. The profile directory is created on first use by the agent/session layer. 5 regression tests in tests/test_issue1195_session_profile_routing.py: existing-profile, non-existent-profile (the core fix), None, empty- string, 'default' all return the expected path. Co-authored-by: bergeouss <bergeouss@users.noreply.github.com>
This commit is contained in:
committed by
nesquena-hermes
parent
c5f4f569d6
commit
f14280e2c4
@@ -153,9 +153,7 @@ def get_hermes_home_for_profile(name: str) -> Path:
|
||||
if not name or name == 'default' or not _PROFILE_ID_RE.match(name):
|
||||
return _DEFAULT_HERMES_HOME
|
||||
profile_dir = _DEFAULT_HERMES_HOME / 'profiles' / name
|
||||
if profile_dir.is_dir():
|
||||
return profile_dir
|
||||
return _DEFAULT_HERMES_HOME
|
||||
return profile_dir
|
||||
|
||||
|
||||
_TERMINAL_ENV_MAPPINGS = {
|
||||
|
||||
23
docs/ISSUES.md
Normal file
23
docs/ISSUES.md
Normal file
@@ -0,0 +1,23 @@
|
||||
# Upstream Issues — Root Cause Analysis
|
||||
|
||||
## #1256: Browser tools fail with "Playwright not installed"
|
||||
|
||||
### Root Cause
|
||||
The check lives in **hermes-agent** (upstream), not hermes-webui:
|
||||
|
||||
```
|
||||
hermes-agent/tools/browser_tool.py → check_browser_requirements()
|
||||
```
|
||||
|
||||
`check_browser_requirements()` does not recognize CDP (Chrome DevTools Protocol) mode — it only looks for a local Playwright/Puppeteer install. When the agent runs in CDP mode (connecting to an existing browser), the check still fails.
|
||||
|
||||
### WebUI side
|
||||
The WebUI already passes `CLI_TOOLSETS` correctly per-request. The `enabled_toolsets` field in the cron/chat config is dynamic and works as intended.
|
||||
|
||||
### Fix required
|
||||
The fix must happen in `hermes-agent/tools/browser_tool.py`:
|
||||
- `check_browser_requirements()` should skip the Playwright check when CDP mode is configured
|
||||
- Or add a `BROWSER_MODE=cdp` env var that bypasses the local browser requirement
|
||||
|
||||
### Workaround
|
||||
Use `CLOUD_BROWSER=true` or configure `browser.base_url` to point to a remote CDP endpoint. This bypasses the local Playwright requirement.
|
||||
81
tests/test_issue1195_session_profile_routing.py
Normal file
81
tests/test_issue1195_session_profile_routing.py
Normal file
@@ -0,0 +1,81 @@
|
||||
"""Tests for issue #1195: sessions must route to the correct profile directory
|
||||
even when that profile directory does not exist yet on disk."""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ── helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
def _make_hermes_home(base: Path, profile_name: str | None = None) -> Path:
|
||||
"""Create a temp HERMES_HOME (with optional profile dir) and return it."""
|
||||
hermes_home = base / ".hermes"
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
if profile_name:
|
||||
(hermes_home / "profiles" / profile_name).mkdir(parents=True, exist_ok=True)
|
||||
return hermes_home
|
||||
|
||||
|
||||
# ── tests ────────────────────────────────────────────────────────────────────
|
||||
|
||||
class TestGetHermesHomeForProfile:
|
||||
"""get_hermes_home_for_profile() must return the profile path regardless of
|
||||
whether the directory already exists on disk (#1195)."""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _patch_default_home(self, tmp_path):
|
||||
"""Patch _DEFAULT_HERMES_HOME to a temp directory for isolation."""
|
||||
from api.profiles import _DEFAULT_HERMES_HOME as real_default
|
||||
|
||||
fake_home = tmp_path / ".hermes"
|
||||
fake_home.mkdir(parents=True)
|
||||
with patch("api.profiles._DEFAULT_HERMES_HOME", fake_home):
|
||||
yield fake_home, real_default
|
||||
|
||||
def test_existing_profile_returns_profile_dir(self, _patch_default_home):
|
||||
fake_home, _ = _patch_default_home
|
||||
from api.profiles import get_hermes_home_for_profile
|
||||
|
||||
# Create an existing profile directory
|
||||
profile_dir = fake_home / "profiles" / "ayan"
|
||||
profile_dir.mkdir(parents=True)
|
||||
|
||||
result = get_hermes_home_for_profile("ayan")
|
||||
assert result == profile_dir
|
||||
|
||||
def test_nonexistent_profile_still_returns_profile_path(self, _patch_default_home):
|
||||
"""Core bug fix: profile dir doesn't exist yet but should still route there."""
|
||||
fake_home, _ = _patch_default_home
|
||||
from api.profiles import get_hermes_home_for_profile
|
||||
|
||||
# Do NOT create the profile directory
|
||||
expected = fake_home / "profiles" / "newprofile"
|
||||
assert not expected.exists() # confirm it doesn't exist
|
||||
|
||||
result = get_hermes_home_for_profile("newprofile")
|
||||
assert result == expected, "Should route to profile path even when dir missing"
|
||||
|
||||
def test_none_returns_default(self, _patch_default_home):
|
||||
fake_home, _ = _patch_default_home
|
||||
from api.profiles import get_hermes_home_for_profile
|
||||
|
||||
result = get_hermes_home_for_profile(None)
|
||||
assert result == fake_home
|
||||
|
||||
def test_empty_string_returns_default(self, _patch_default_home):
|
||||
fake_home, _ = _patch_default_home
|
||||
from api.profiles import get_hermes_home_for_profile
|
||||
|
||||
result = get_hermes_home_for_profile("")
|
||||
assert result == fake_home
|
||||
|
||||
def test_default_string_returns_default(self, _patch_default_home):
|
||||
fake_home, _ = _patch_default_home
|
||||
from api.profiles import get_hermes_home_for_profile
|
||||
|
||||
result = get_hermes_home_for_profile("default")
|
||||
assert result == fake_home
|
||||
Reference in New Issue
Block a user