From 63f9b719bb31ac57f83724929d5dd3799c96cad5 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 20 Apr 2026 15:12:01 -0700 Subject: [PATCH] fix(config): use Hermes config.yaml as single source of default model (#773) Removes split-brain where WebUI Settings persisted default_model separately from Hermes runtime config.yaml. New POST /api/default-model endpoint writes to config.yaml. Existing saved values migrated on first load. Fixes #761 Co-authored-by: aronprins --- CHANGELOG.md | 5 ++ api/config.py | 125 ++++++++++++++++++++++++++++++++------ api/models.py | 12 +++- api/routes.py | 9 +++ static/onboarding.js | 3 +- static/panels.js | 28 +++++++-- tests/test_batch_fixes.py | 3 +- tests/test_sprint12.py | 48 +++++++++++---- 8 files changed, 192 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ba270e..730f1cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Hermes Web UI -- Changelog +## [v0.50.114] — 2026-04-20 + +### Fixed +- **Default model now reads from Hermes config.yaml** — removes the split-brain state where WebUI Settings and the Hermes runtime/CLI/gateway could have different default models. `default_model` is no longer persisted in `settings.json`; it is read from and written to `config.yaml` via a new `POST /api/default-model` endpoint. Existing saved `default_model` values in `settings.json` are silently migrated away on first load. Saving Settings now calls `/api/default-model` when the model changed, with error handling so a config.yaml write failure doesn't leave the UI in a broken state. (#761, credit: @aronprins) + ## [v0.50.113] — 2026-04-20 ### Fixed diff --git a/api/config.py b/api/config.py index 1dcbf7a..0fadebb 100644 --- a/api/config.py +++ b/api/config.py @@ -209,6 +209,35 @@ def reload_config() -> None: logger.debug("Failed to load yaml config from %s", config_path) +def _load_yaml_config_file(config_path: Path) -> dict: + try: + import yaml as _yaml + except ImportError: + return {} + + if not config_path.exists(): + return {} + try: + loaded = _yaml.safe_load(config_path.read_text(encoding="utf-8")) + return loaded if isinstance(loaded, dict) else {} + except Exception: + logger.debug("Failed to parse yaml config from %s", config_path) + return {} + + +def _save_yaml_config_file(config_path: Path, config_data: dict) -> None: + try: + import yaml as _yaml + except ImportError as exc: + raise RuntimeError("PyYAML is required to write Hermes config.yaml") from exc + + config_path.parent.mkdir(parents=True, exist_ok=True) + config_path.write_text( + _yaml.safe_dump(config_data, sort_keys=False, allow_unicode=True), + encoding="utf-8", + ) + + # Initial load reload_config() cfg = _cfg_cache # alias for backward compat with existing references @@ -710,6 +739,69 @@ def resolve_model_provider(model_id: str) -> tuple: return model_id, config_provider, config_base_url +def get_effective_default_model(config_data: dict | None = None) -> str: + """Resolve the effective Hermes default model from config, then env overrides.""" + active_cfg = config_data if config_data is not None else cfg + default_model = DEFAULT_MODEL + + model_cfg = active_cfg.get("model", {}) + if isinstance(model_cfg, str): + default_model = model_cfg.strip() + elif isinstance(model_cfg, dict): + cfg_default = str(model_cfg.get("default") or "").strip() + if cfg_default: + default_model = cfg_default + + env_model = ( + os.getenv("HERMES_MODEL") or os.getenv("OPENAI_MODEL") or os.getenv("LLM_MODEL") + ) + if env_model: + default_model = env_model.strip() + return default_model + + +def set_hermes_default_model(model_id: str) -> dict: + """Persist the Hermes default model in config.yaml and reload runtime config.""" + selected_model = str(model_id or "").strip() + if not selected_model: + raise ValueError("model is required") + + config_path = _get_config_path() + # Hold _cfg_lock only around the read-modify-write of the YAML file. + # reload_config() acquires _cfg_lock internally (it's not reentrant) so + # it must be called AFTER releasing the lock to avoid deadlock. + with _cfg_lock: + config_data = _load_yaml_config_file(config_path) + model_cfg = config_data.get("model", {}) + if not isinstance(model_cfg, dict): + model_cfg = {} + + previous_provider = str(model_cfg.get("provider") or "").strip() + resolved_model, resolved_provider, resolved_base_url = resolve_model_provider( + selected_model + ) + persisted_model = str(resolved_model or selected_model).strip() + persisted_provider = str(resolved_provider or previous_provider or "").strip() + + model_cfg["default"] = persisted_model + if persisted_provider: + model_cfg["provider"] = persisted_provider + + if resolved_base_url: + model_cfg["base_url"] = str(resolved_base_url).strip().rstrip("/") + elif persisted_provider != previous_provider: + if persisted_provider == "openai": + model_cfg["base_url"] = "https://api.openai.com/v1" + elif not persisted_provider.startswith("custom:"): + model_cfg.pop("base_url", None) + + config_data["model"] = model_cfg + _save_yaml_config_file(config_path, config_data) + # Reload outside the lock — reload_config() acquires _cfg_lock itself. + reload_config() + return get_available_models() + + def get_available_models() -> dict: """ Return available models grouped by provider. @@ -736,7 +828,7 @@ def get_available_models() -> dict: if _current_mtime != _cfg_mtime: reload_config() active_provider = None - default_model = DEFAULT_MODEL + default_model = get_effective_default_model(cfg) groups = [] # 1. Read config.yaml model section @@ -744,7 +836,7 @@ def get_available_models() -> dict: model_cfg = cfg.get("model", {}) cfg_base_url = "" if isinstance(model_cfg, str): - default_model = model_cfg + pass # default_model already set by get_effective_default_model elif isinstance(model_cfg, dict): active_provider = model_cfg.get("provider") cfg_default = model_cfg.get("default", "") @@ -752,14 +844,7 @@ def get_available_models() -> dict: if cfg_default: default_model = cfg_default - # 2. Also check env vars for model override - env_model = ( - os.getenv("HERMES_MODEL") or os.getenv("OPENAI_MODEL") or os.getenv("LLM_MODEL") - ) - if env_model: - default_model = env_model.strip() - - # 3. Try to read auth store for active provider (if hermes is installed) + # 2. Try to read auth store for active provider (if hermes is installed) if not active_provider: try: from api.profiles import get_active_hermes_home as _gah @@ -1239,7 +1324,6 @@ def _get_session_agent_lock(session_id: str) -> threading.Lock: # ── Settings persistence ───────────────────────────────────────────────────── _SETTINGS_DEFAULTS = { - "default_model": DEFAULT_MODEL, "default_workspace": str(DEFAULT_WORKSPACE), "onboarding_completed": False, "send_key": "enter", # 'enter' or 'ctrl+enter' @@ -1259,7 +1343,7 @@ _SETTINGS_DEFAULTS = { "sidebar_density": "compact", # compact | detailed "password_hash": None, # PBKDF2-HMAC-SHA256 hash; None = auth disabled } -_SETTINGS_LEGACY_DROP_KEYS = {"assistant_language"} +_SETTINGS_LEGACY_DROP_KEYS = {"assistant_language", "default_model"} _SETTINGS_THEME_VALUES = {"light", "dark", "system"} _SETTINGS_SKIN_VALUES = { "default", @@ -1346,10 +1430,14 @@ def load_settings() -> dict: stored.get("theme") if isinstance(stored, dict) else settings.get("theme"), stored.get("skin") if isinstance(stored, dict) else settings.get("skin"), ) + settings["default_model"] = get_effective_default_model() return settings -_SETTINGS_ALLOWED_KEYS = set(_SETTINGS_DEFAULTS.keys()) - {"password_hash"} +_SETTINGS_ALLOWED_KEYS = set(_SETTINGS_DEFAULTS.keys()) - { + "password_hash", + "default_model", +} _SETTINGS_ENUM_VALUES = { "send_key": {"enter", "ctrl+enter"}, "sidebar_density": {"compact", "detailed"}, @@ -1420,16 +1508,16 @@ def save_settings(settings: dict) -> dict: current["default_workspace"] = str( resolve_default_workspace(current.get("default_workspace")) ) + persisted = {k: v for k, v in current.items() if k != "default_model"} SETTINGS_FILE.write_text( - json.dumps(current, ensure_ascii=False, indent=2), + json.dumps(persisted, ensure_ascii=False, indent=2), encoding="utf-8", ) # Update runtime defaults so new sessions use them immediately - global DEFAULT_MODEL, DEFAULT_WORKSPACE - if "default_model" in current: - DEFAULT_MODEL = current["default_model"] + global DEFAULT_WORKSPACE if "default_workspace" in current: DEFAULT_WORKSPACE = resolve_default_workspace(current["default_workspace"]) + current["default_model"] = get_effective_default_model() return current @@ -1444,12 +1532,11 @@ try: except OSError: _settings_file_exists = False if _settings_file_exists: - if _startup_settings.get("default_model"): - DEFAULT_MODEL = _startup_settings["default_model"] if not os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE"): DEFAULT_WORKSPACE = resolve_default_workspace( _startup_settings.get("default_workspace") ) + _startup_settings.pop("default_model", None) # always drop stale value; model comes from config.yaml if _startup_settings.get("default_workspace") != str(DEFAULT_WORKSPACE): _startup_settings["default_workspace"] = str(DEFAULT_WORKSPACE) try: diff --git a/api/models.py b/api/models.py index 22144d6..2f9c989 100644 --- a/api/models.py +++ b/api/models.py @@ -11,7 +11,8 @@ from pathlib import Path import api.config as _cfg from api.config import ( SESSION_DIR, SESSION_INDEX_FILE, SESSIONS, SESSIONS_MAX, - LOCK, DEFAULT_WORKSPACE, DEFAULT_MODEL, PROJECTS_FILE, HOME + LOCK, DEFAULT_WORKSPACE, DEFAULT_MODEL, PROJECTS_FILE, HOME, + get_effective_default_model, ) from api.workspace import get_last_workspace @@ -134,13 +135,18 @@ def get_session(sid): raise KeyError(sid) def new_session(workspace=None, model=None): - # Use _cfg.DEFAULT_MODEL (not the import-time snapshot) so save_settings() changes take effect + # Use the live config-derived default so Hermes config changes apply without restart. try: from api.profiles import get_active_profile_name _profile = get_active_profile_name() except ImportError: _profile = None - s = Session(workspace=workspace or get_last_workspace(), model=model or _cfg.DEFAULT_MODEL, profile=_profile) + effective_model = model or get_effective_default_model() + s = Session( + workspace=workspace or get_last_workspace(), + model=effective_model, + profile=_profile, + ) with LOCK: SESSIONS[s.session_id] = s SESSIONS.move_to_end(s.session_id) diff --git a/api/routes.py b/api/routes.py index c6785a2..9e81fe4 100644 --- a/api/routes.py +++ b/api/routes.py @@ -47,6 +47,7 @@ from api.config import ( CHAT_LOCK, load_settings, save_settings, + set_hermes_default_model, ) from api.helpers import ( require, @@ -881,6 +882,14 @@ def handle_post(handler, parsed) -> bool: s = new_session(workspace=workspace, model=body.get("model")) return j(handler, {"session": s.compact() | {"messages": s.messages}}) + if parsed.path == "/api/default-model": + try: + return j(handler, set_hermes_default_model(body.get("model"))) + except ValueError as e: + return bad(handler, str(e)) + except RuntimeError as e: + return bad(handler, str(e), 500) + if parsed.path == "/api/sessions/cleanup": return _handle_sessions_cleanup(handler, body, zero_only=False) diff --git a/static/onboarding.js b/static/onboarding.js index 85ea8d0..b10de99 100644 --- a/static/onboarding.js +++ b/static/onboarding.js @@ -321,7 +321,8 @@ async function _saveOnboardingDefaults(){ if(!known){ await api('/api/workspaces/add',{method:'POST',body:JSON.stringify({path:workspace})}); } - const body={default_workspace:workspace,default_model:model}; + // Model persisted by /api/onboarding/setup — no /api/default-model call needed here + const body={default_workspace:workspace}; if(password) body._set_password=password; const saved=await api('/api/settings',{method:'POST',body:JSON.stringify(body)}); if(ONBOARDING.status){ diff --git a/static/panels.js b/static/panels.js index 1de1b83..0656df8 100644 --- a/static/panels.js +++ b/static/panels.js @@ -1075,6 +1075,7 @@ document.addEventListener('drop',e=>{e.preventDefault();dragCounter=0;wrap.class let _settingsDirty = false; let _settingsThemeOnOpen = null; // track theme at open time for discard revert let _settingsSkinOnOpen = null; // track skin at open time for discard revert +let _settingsHermesDefaultModelOnOpen = ''; let _settingsSection = 'conversation'; function switchSettingsSection(name){ @@ -1220,9 +1221,10 @@ async function loadSettingsPanel(){ const modelSel=$('settingsModel'); if(modelSel){ modelSel.innerHTML=''; + let models=null; try{ - const models=await api('/api/models'); - for(const g of (models.groups||[])){ + models=await api('/api/models'); + for(const g of ((models||{}).groups||[])){ const og=document.createElement('optgroup'); og.label=g.provider; for(const m of g.models){ @@ -1233,7 +1235,8 @@ async function loadSettingsPanel(){ modelSel.appendChild(og); } }catch(e){} - modelSel.value=settings.default_model||''; + _settingsHermesDefaultModelOnOpen=(models&&models.default_model)||''; + modelSel.value=_settingsHermesDefaultModelOnOpen; modelSel.addEventListener('change',_markSettingsDirty,{once:false}); } // Send key preference @@ -1320,6 +1323,7 @@ function _applySavedSettingsUi(saved, body, opts){ _settingsSkinOnOpen=skin||'default'; const bar=$('settingsUnsavedBar'); if(bar) bar.style.display='none'; + _settingsHermesDefaultModelOnOpen=body.default_model||_settingsHermesDefaultModelOnOpen||''; renderMessages(); if(typeof syncTopbar==='function') syncTopbar(); if(typeof renderSessionList==='function') renderSessionList(); @@ -1327,6 +1331,7 @@ function _applySavedSettingsUi(saved, body, opts){ async function saveSettings(andClose){ const model=($('settingsModel')||{}).value; + const modelChanged=(model||'')!==(_settingsHermesDefaultModelOnOpen||''); const sendKey=($('settingsSendKey')||{}).value; const showTokenUsage=!!($('settingsShowTokenUsage')||{}).checked; const showCliSessions=!!($('settingsShowCliSessions')||{}).checked; @@ -1336,7 +1341,6 @@ async function saveSettings(andClose){ const language=($('settingsLanguage')||{}).value||'en'; const sidebarDensity=($('settingsSidebarDensity')||{}).value==='detailed'?'detailed':'compact'; const body={}; - if(model) body.default_model=model; if(sendKey) body.send_key=sendKey; body.theme=theme; @@ -1357,6 +1361,14 @@ async function saveSettings(andClose){ if(pw && pw.trim()){ try{ const saved=await api('/api/settings',{method:'POST',body:JSON.stringify({...body,_set_password:pw.trim()})}); + if(modelChanged && model){ + try{ + await api('/api/default-model',{method:'POST',body:JSON.stringify({model})}); + body.default_model=model; + }catch(_modelErr){ + if(typeof showToast==='function') showToast('Failed to update default model — settings saved'); + } + } _applySavedSettingsUi(saved, body, {sendKey,showTokenUsage,showCliSessions,theme,skin,language,sidebarDensity}); showToast(t(saved.auth_just_enabled?'settings_saved_pw':'settings_saved_pw_updated')); _hideSettingsPanel(); @@ -1365,6 +1377,14 @@ async function saveSettings(andClose){ } try{ const saved=await api('/api/settings',{method:'POST',body:JSON.stringify(body)}); + if(modelChanged && model){ + try{ + await api('/api/default-model',{method:'POST',body:JSON.stringify({model})}); + body.default_model=model; + }catch(_modelErr){ + if(typeof showToast==='function') showToast('Failed to update default model — settings saved'); + } + } _applySavedSettingsUi(saved, body, {sendKey,showTokenUsage,showCliSessions,theme,skin,language,sidebarDensity}); showToast(t('settings_saved')); _hideSettingsPanel(); diff --git a/tests/test_batch_fixes.py b/tests/test_batch_fixes.py index f0c0f75..1008c29 100644 --- a/tests/test_batch_fixes.py +++ b/tests/test_batch_fixes.py @@ -219,7 +219,8 @@ class TestSystemTheme: def test_panels_hydrates_appearance_before_models_fetch(self): src = read("static/panels.js") skin_idx = src.index("const skinVal=(settings.skin||'default').toLowerCase();") - models_idx = src.index("const models=await api('/api/models');") + # models is now declared as let models=null before the try block + models_idx = src.index("models=await api('/api/models');") assert skin_idx < models_idx, ( "loadSettingsPanel must hydrate theme/skin before awaiting /api/models, " "otherwise a slow model fetch can clobber an in-progress skin selection" diff --git a/tests/test_sprint12.py b/tests/test_sprint12.py index d5ea3a6..7e3ec0a 100644 --- a/tests/test_sprint12.py +++ b/tests/test_sprint12.py @@ -38,27 +38,49 @@ def test_settings_get_returns_defaults(): assert 'default_model' in d assert 'default_workspace' in d -def test_settings_post_persists(): - """POST /api/settings saves and returns merged settings.""" - d, status = post("/api/settings", {"default_model": "test/model-123"}) - assert status == 200 - assert d['default_model'] == 'test/model-123' - # Verify it persisted +def test_default_model_updates_hermes_config(): + """POST /api/default-model updates the effective Hermes default model.""" + original, _ = get("/api/models") + original_model = original.get("default_model") or "" + try: + d, status = post("/api/default-model", {"model": "anthropic/claude-sonnet-4.6"}) + assert status == 200 + assert 'claude-sonnet-4.6' in d['default_model'] + d2, _ = get("/api/settings") + # Both should resolve to the same model (may differ in prefix normalization) + assert 'claude-sonnet-4.6' in d2['default_model'] + finally: + # Always restore — regardless of test ordering or failures + if original_model: + post("/api/default-model", {"model": original_model}) + + +def test_settings_does_not_persist_default_model(): + """POST /api/settings with default_model in body is silently ignored.""" + d1, _ = get("/api/settings") + original_model = d1['default_model'] + # Send default_model via /api/settings — it must be dropped (not persisted) + post("/api/settings", {"default_model": "openai/fake-model-xyz"}) d2, _ = get("/api/settings") - assert d2['default_model'] == 'test/model-123' - # Restore - post("/api/settings", {"default_model": "openai/gpt-5.4-mini"}) + assert d2['default_model'] == original_model, ( + "POST /api/settings must not persist default_model — use /api/default-model instead" + ) + + +def test_default_model_empty_returns_400(): + """POST /api/default-model with empty model returns 400.""" + d, status = post("/api/default-model", {"model": ""}) + assert status == 400 def test_settings_partial_update(): """POST /api/settings with partial data doesn't clobber other fields.""" d1, _ = get("/api/settings") original_ws = d1['default_workspace'] - post("/api/settings", {"default_model": "anthropic/claude-sonnet-4.6"}) + post("/api/settings", {"send_key": "ctrl+enter"}) d2, _ = get("/api/settings") - assert d2['default_model'] == 'anthropic/claude-sonnet-4.6' + assert d2['send_key'] == 'ctrl+enter' assert d2['default_workspace'] == original_ws - # Restore - post("/api/settings", {"default_model": "openai/gpt-5.4-mini"}) + post("/api/settings", {"send_key": "enter"}) # ── Session Pinning ───────────────────────────────────────────────────────