diff --git a/api/profiles.py b/api/profiles.py index 943abff..f74cd91 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -16,9 +16,44 @@ from pathlib import Path # ── Module state ──────────────────────────────────────────────────────────── _active_profile = 'default' _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. -_DEFAULT_HERMES_HOME = Path(os.getenv('HERMES_HOME', str(Path.home() / '.hermes'))) + +def _resolve_base_hermes_home() -> Path: + """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: diff --git a/static/workspace.js b/static/workspace.js index 94b6847..13eb439 100644 --- a/static/workspace.js +++ b/static/workspace.js @@ -1,7 +1,13 @@ async function api(path,opts={}){ const url=new URL(path,location.origin); 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')||''; return ct.includes('application/json')?res.json():res.text(); } diff --git a/tests/test_sprint23.py b/tests/test_sprint23.py index 8a7041c..deedda3 100644 --- a/tests/test_sprint23.py +++ b/tests/test_sprint23.py @@ -122,3 +122,72 @@ def test_panels_js_clears_model_on_switch(): assert "localStorage.removeItem('hermes-webui-model')" in content assert "loadWorkspaceList" 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" + )