fix: profile switch fails with 'does not exist' when server starts on non-default profile
Root cause: _DEFAULT_HERMES_HOME was evaluated at module import time from
os.getenv('HERMES_HOME'). HERMES_HOME is a MUTABLE env var -- init_profile_state()
at server startup calls _set_hermes_home() which writes to os.environ['HERMES_HOME'].
If the sticky active_profile file pointed to e.g. 'webui', HERMES_HOME was set to
~/.hermes/profiles/webui BEFORE api/profiles.py imported. So _DEFAULT_HERMES_HOME
resolved to ~/.hermes/profiles/webui. Then switch_profile('webui') computed:
home = ~/.hermes/profiles/webui / 'profiles' / 'webui'
= ~/.hermes/profiles/webui/profiles/webui -- doesn't exist -> 404 ValueError
Fix: replace the one-liner assignment with _resolve_base_hermes_home() which:
1. Checks HERMES_BASE_HOME env var (explicit override)
2. Checks HERMES_HOME -- but if it looks like a profiles/ subdir (parent.name ==
'profiles'), walks up two levels to the actual base
3. Falls back to Path.home() / '.hermes'
This means the server can start with HERMES_HOME pointing to any profile and
_DEFAULT_HERMES_HOME will still correctly point to ~/.hermes.
Also fix: api() helper in workspace.js was throwing new Error(await res.text())
which surfaced raw JSON to the UI: 'Switch failed: {"error":"Profile X does not exist."}'
Now parses the JSON and extracts j.error so the toast shows clean human-readable text.
Regression tests added in test_sprint23.py:
- test_profile_switch_base_home_not_subdir: static analysis verifying the resolver
- test_api_helper_returns_clean_error_message: verifies api() parses JSON errors
- test_profile_switch_resolve_base_home_logic: verifies the profiles/ subdir detection
Tests: 426 passed, 0 failed.
This commit is contained in:
@@ -16,9 +16,44 @@ from pathlib import Path
|
|||||||
# ── Module state ────────────────────────────────────────────────────────────
|
# ── Module state ────────────────────────────────────────────────────────────
|
||||||
_active_profile = 'default'
|
_active_profile = 'default'
|
||||||
_profile_lock = threading.Lock()
|
_profile_lock = threading.Lock()
|
||||||
# Read from env var so test isolation (HERMES_HOME=TEST_STATE_DIR) is respected.
|
|
||||||
# Evaluated at import time; server restart picks up any env change.
|
def _resolve_base_hermes_home() -> Path:
|
||||||
_DEFAULT_HERMES_HOME = Path(os.getenv('HERMES_HOME', str(Path.home() / '.hermes')))
|
"""Return the BASE ~/.hermes directory — the root that contains profiles/.
|
||||||
|
|
||||||
|
This is intentionally distinct from HERMES_HOME, which tracks the *active
|
||||||
|
profile's* home and changes on every profile switch. The base dir must
|
||||||
|
always point to the top-level .hermes regardless of which profile is active.
|
||||||
|
|
||||||
|
Resolution order:
|
||||||
|
1. HERMES_BASE_HOME env var (set explicitly, highest priority)
|
||||||
|
2. HERMES_HOME env var — but only if it does NOT look like a profile subdir
|
||||||
|
(i.e. its parent is not named 'profiles'). This handles test isolation
|
||||||
|
where HERMES_HOME is set to an isolated test state dir.
|
||||||
|
3. ~/.hermes (always-correct default)
|
||||||
|
|
||||||
|
The bug this prevents: if HERMES_HOME has already been mutated to
|
||||||
|
/home/user/.hermes/profiles/webui (by init_profile_state at startup),
|
||||||
|
reading it here would make _DEFAULT_HERMES_HOME point to that subdir,
|
||||||
|
causing switch_profile('webui') to look for
|
||||||
|
/home/user/.hermes/profiles/webui/profiles/webui — which doesn't exist.
|
||||||
|
"""
|
||||||
|
# Explicit override for tests or unusual setups
|
||||||
|
base_override = os.getenv('HERMES_BASE_HOME', '').strip()
|
||||||
|
if base_override:
|
||||||
|
return Path(base_override).expanduser()
|
||||||
|
|
||||||
|
hermes_home = os.getenv('HERMES_HOME', '').strip()
|
||||||
|
if hermes_home:
|
||||||
|
p = Path(hermes_home).expanduser()
|
||||||
|
# If HERMES_HOME points to a profiles/ subdir, walk up two levels to the base
|
||||||
|
if p.parent.name == 'profiles':
|
||||||
|
return p.parent.parent
|
||||||
|
# Otherwise trust it (e.g. test isolation sets HERMES_HOME to TEST_STATE_DIR)
|
||||||
|
return p
|
||||||
|
|
||||||
|
return Path.home() / '.hermes'
|
||||||
|
|
||||||
|
_DEFAULT_HERMES_HOME = _resolve_base_hermes_home()
|
||||||
|
|
||||||
|
|
||||||
def _read_active_profile_file() -> str:
|
def _read_active_profile_file() -> str:
|
||||||
|
|||||||
@@ -1,7 +1,13 @@
|
|||||||
async function api(path,opts={}){
|
async function api(path,opts={}){
|
||||||
const url=new URL(path,location.origin);
|
const url=new URL(path,location.origin);
|
||||||
const res=await fetch(url.href,{credentials:'include',headers:{'Content-Type':'application/json'},...opts});
|
const res=await fetch(url.href,{credentials:'include',headers:{'Content-Type':'application/json'},...opts});
|
||||||
if(!res.ok)throw new Error(await res.text());
|
if(!res.ok){
|
||||||
|
const text=await res.text();
|
||||||
|
// Parse JSON error body and surface the human-readable message,
|
||||||
|
// rather than showing raw JSON like {"error":"Profile 'x' does not exist."}
|
||||||
|
try{const j=JSON.parse(text);throw new Error(j.error||j.message||text);}
|
||||||
|
catch(e){if(e instanceof SyntaxError)throw new Error(text);throw e;}
|
||||||
|
}
|
||||||
const ct=res.headers.get('content-type')||'';
|
const ct=res.headers.get('content-type')||'';
|
||||||
return ct.includes('application/json')?res.json():res.text();
|
return ct.includes('application/json')?res.json():res.text();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -122,3 +122,72 @@ def test_panels_js_clears_model_on_switch():
|
|||||||
assert "localStorage.removeItem('hermes-webui-model')" in content
|
assert "localStorage.removeItem('hermes-webui-model')" in content
|
||||||
assert "loadWorkspaceList" in content
|
assert "loadWorkspaceList" in content
|
||||||
assert "renderSessionList" in content
|
assert "renderSessionList" in content
|
||||||
|
|
||||||
|
|
||||||
|
# ── Regression: profile switch base dir bug (PR #44) ──────────────────────
|
||||||
|
|
||||||
|
def test_profile_switch_base_home_not_subdir():
|
||||||
|
"""_DEFAULT_HERMES_HOME must always be the base ~/.hermes root, not a
|
||||||
|
profile subdir. Regression: if HERMES_HOME was mutated to a profiles/
|
||||||
|
subdir at server startup, switch_profile() looked for
|
||||||
|
~/.hermes/profiles/X/profiles/X which never exists — returning 404.
|
||||||
|
|
||||||
|
We verify the fix is present via static analysis of profiles.py.
|
||||||
|
The live-switch variant is in test_profile_switch_returns_default_model_and_workspace.
|
||||||
|
"""
|
||||||
|
content = (REPO_ROOT / "api" / "profiles.py").read_text()
|
||||||
|
|
||||||
|
# The fix must define a resolver function that handles the profiles/ subdir case
|
||||||
|
assert "_resolve_base_hermes_home" in content, (
|
||||||
|
"profiles.py must define _resolve_base_hermes_home() to safely resolve "
|
||||||
|
"the base HERMES_HOME regardless of HERMES_HOME env var mutation"
|
||||||
|
)
|
||||||
|
assert "p.parent.name == 'profiles'" in content, (
|
||||||
|
"_resolve_base_hermes_home must detect when HERMES_HOME points to a "
|
||||||
|
"profiles/ subdir (e.g. ~/.hermes/profiles/webui) and walk up to base"
|
||||||
|
)
|
||||||
|
assert "p.parent.parent" in content, (
|
||||||
|
"_resolve_base_hermes_home must return p.parent.parent when HERMES_HOME "
|
||||||
|
"is a profiles/ subdir, giving back the actual ~/.hermes base"
|
||||||
|
)
|
||||||
|
# _DEFAULT_HERMES_HOME must be set from the resolver, not directly from env
|
||||||
|
assert "_DEFAULT_HERMES_HOME = _resolve_base_hermes_home()" in content, (
|
||||||
|
"_DEFAULT_HERMES_HOME must be assigned from _resolve_base_hermes_home(), "
|
||||||
|
"not directly from os.getenv('HERMES_HOME')"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_api_helper_returns_clean_error_message():
|
||||||
|
"""workspace.js api() helper must parse JSON error bodies and surface
|
||||||
|
the human-readable 'error' field, not raw JSON like
|
||||||
|
{'error': 'Profile X does not exist.'}.
|
||||||
|
|
||||||
|
Regression: api() did `throw new Error(await res.text())` which made
|
||||||
|
showToast display 'Switch failed: {"error":"Profile X does not exist."}' --
|
||||||
|
JSON noise the user shouldn't see.
|
||||||
|
"""
|
||||||
|
content = (REPO_ROOT / "static" / "workspace.js").read_text()
|
||||||
|
# Must parse the JSON error body
|
||||||
|
assert "JSON.parse(text)" in content, (
|
||||||
|
"api() must parse JSON error bodies -- raw res.text() leaks JSON to the UI"
|
||||||
|
)
|
||||||
|
# Must extract the .error field
|
||||||
|
assert "j.error" in content, (
|
||||||
|
"api() must extract j.error from parsed JSON error response"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_profile_switch_resolve_base_home_logic():
|
||||||
|
"""Static analysis: _resolve_base_hermes_home() must handle the case
|
||||||
|
where HERMES_HOME points to a profiles/ subdir by walking up to the base.
|
||||||
|
"""
|
||||||
|
content = (REPO_ROOT / "api" / "profiles.py").read_text()
|
||||||
|
assert "_resolve_base_hermes_home" in content, (
|
||||||
|
"profiles.py must define _resolve_base_hermes_home()"
|
||||||
|
)
|
||||||
|
assert "p.parent.name == 'profiles'" in content, (
|
||||||
|
"_resolve_base_hermes_home must detect and unwrap profiles/ subdir paths"
|
||||||
|
)
|
||||||
|
assert "p.parent.parent" in content, (
|
||||||
|
"_resolve_base_hermes_home must walk up two levels from a profiles/ subdir"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user