From f9da9e83853d8918ff3d23ad2d2b762c6bbf4c6b Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:26:52 -0700 Subject: [PATCH] fix(web): lock the remaining off-loop config RMWs; keep model probes outside the lock local_models._set_runtime_enabled (quickstart/activate/stop job threads) and profiles._disable_unselected_skills ran load_config -> mutate -> save_config without _CONFIG_MUTATION_LOCK, so the dashboard's debounced PUT /api/config autosave could erase their writes exactly like the routers this PR already fixed. Both spans now hold the lock. POST /api/model/set held the global lock across switch_model's catalog fetches / endpoint probes, stalling every other config writer for the duration of a network round-trip. The main-slot validation is split into _prepare_main_assignment (runs under the profile scope only) and the load -> apply -> save half runs under the lock. The Nous entitlement refresh (force_fresh) stays inside the write half: it must read the on-disk config it mutates, and it is bounded by the portal timeout. --- hermes_cli/web_routers/local_models.py | 10 ++++++--- hermes_cli/web_routers/models.py | 19 ++++++++++------ hermes_cli/web_routers/profiles.py | 2 +- hermes_cli/web_server_config.py | 30 +++++++++++++++++++------- 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/hermes_cli/web_routers/local_models.py b/hermes_cli/web_routers/local_models.py index 125ac96adc..907123778a 100644 --- a/hermes_cli/web_routers/local_models.py +++ b/hermes_cli/web_routers/local_models.py @@ -30,6 +30,7 @@ from pydantic import BaseModel from starlette.concurrency import run_in_threadpool from hermes_cli import config as config_mod, web_deps +from hermes_cli.web_routers._common import _CONFIG_MUTATION_LOCK from hermes_cli.local_runtime import ( binaries, bootstrap, catalog, context_policy, estimator, growth, hardware, hf_browse, load_progress, presets, supervisor, @@ -201,9 +202,12 @@ def _runtime_section() -> dict: def _set_runtime_enabled(enabled: bool) -> dict: """Persist ``local_runtime.enabled`` and return the config written.""" - config = config_mod.load_config() - config.setdefault("local_runtime", {})["enabled"] = enabled - config_mod.save_config(config) + # Runs on quickstart/activate/stop job threads; the RMW span races the dashboard's + # debounced PUT /api/config autosave without the lock. + with _CONFIG_MUTATION_LOCK: + config = config_mod.load_config() + config.setdefault("local_runtime", {})["enabled"] = enabled + config_mod.save_config(config) return config diff --git a/hermes_cli/web_routers/models.py b/hermes_cli/web_routers/models.py index bb7d5be163..766e001933 100644 --- a/hermes_cli/web_routers/models.py +++ b/hermes_cli/web_routers/models.py @@ -13,11 +13,12 @@ from fastapi import APIRouter, HTTPException from hermes_cli.web_deps import LateState, late from hermes_cli.web_server_config import ( _AUX_TASK_SLOTS, _UNSET, _apply_model_assignment_sync, _dashboard_code_skew_guard, + _prepare_main_assignment, ) from agent.model_metadata import is_local_endpoint from starlette.concurrency import run_in_threadpool from hermes_cli.web_models import ModelAssignment, MoaConfigPayload, MoaModelSlot -from hermes_cli.web_routers._common import config_write_scope, http_failure +from hermes_cli.web_routers._common import _CONFIG_MUTATION_LOCK, config_write_scope, http_failure _log = logging.getLogger("hermes_cli.web_server") router = APIRouter() @@ -299,10 +300,16 @@ async def set_model_assignment(body: ModelAssignment, profile: Optional[str] = N reasoning_effort = body.reasoning_effort if "reasoning_effort" in body.model_fields_set else _UNSET def _apply_assignment(): - # Same RMW span as PUT /api/config: applyMainModel fires this while - # the settings-page autosave is in flight — hold the mutation lock. - with config_write_scope(body.profile or profile): - return _apply_model_assignment_sync( - scope, provider, model, task, base_url, api_key, reasoning_effort=reasoning_effort) + # Same RMW span as PUT /api/config: applyMainModel fires this while the + # settings-page autosave is in flight — hold the mutation lock. switch_model's + # catalog fetches / endpoint probes are network I/O, so they run BEFORE the lock; + # only load→apply→save holds it. + with _profile_scope(body.profile or profile): + prepared = (_prepare_main_assignment(load_config(), provider, model, base_url, api_key) + if scope == "main" else None) + with _CONFIG_MUTATION_LOCK: + return _apply_model_assignment_sync( + scope, provider, model, task, base_url, api_key, + reasoning_effort=reasoning_effort, prepared=prepared) return await asyncio.to_thread(_apply_assignment) diff --git a/hermes_cli/web_routers/profiles.py b/hermes_cli/web_routers/profiles.py index 80937b177a..f9b682b6d4 100644 --- a/hermes_cli/web_routers/profiles.py +++ b/hermes_cli/web_routers/profiles.py @@ -125,7 +125,7 @@ def _disable_unselected_skills(profile_dir: Path, keep: List[str]) -> int: from hermes_cli.config import load_config from hermes_cli.skills_config import get_disabled_skills, save_disabled_skills keep_set = {s.strip() for s in keep if s and s.strip()} - with _hermes_home_scope(profile_dir): + with _hermes_home_scope(profile_dir), _CONFIG_MUTATION_LOCK: # RMW span skills_root = profile_dir / "skills" installed = ([md.parent.name for md in skills_root.rglob("SKILL.md")] if skills_root.is_dir() else []) diff --git a/hermes_cli/web_server_config.py b/hermes_cli/web_server_config.py index fcdd0f900c..a51ed2defd 100644 --- a/hermes_cli/web_server_config.py +++ b/hermes_cli/web_server_config.py @@ -653,17 +653,30 @@ def _cron_model_impact(cfg: dict, provider: str, model: str) -> Any: return build_cron_model_impact(config=cfg, jobs={}) -def _apply_main_assignment_sync(cfg: dict, provider: str, model: str, base_url: str, api_key: str) -> dict: - from hermes_cli.config import save_config +def _provider_entry(cfg: dict, provider: str) -> Any: + providers_cfg = cfg.get("providers") + return providers_cfg.get(provider) if isinstance(providers_cfg, dict) else None + + +def _prepare_main_assignment(cfg: dict, provider: str, model: str, base_url: str, api_key: str) -> "tuple[str, ModelSwitchResult]": + """Validation half of a main-slot assignment: ``(effective base_url, switch result)``. + ``switch_model`` fetches catalogs / probes endpoints, so callers run this BEFORE taking + ``_CONFIG_MUTATION_LOCK``; it writes nothing.""" if not provider or not model: raise HTTPException(status_code=400, detail="provider and model required for main") provider, model = _normalize_main_model_assignment(provider, model) - providers_cfg = cfg.get("providers") - provider_entry = providers_cfg.get(provider) if isinstance(providers_cfg, dict) else None + provider_entry = _provider_entry(cfg, provider) if not base_url and isinstance(provider_entry, dict) and provider_entry.get("base_url"): base_url = str(provider_entry.get("base_url") or "").strip() - result = _validated_main_model_selection(cfg, provider, model, base_url, api_key) + return base_url, _validated_main_model_selection(cfg, provider, model, base_url, api_key) + + +def _apply_main_assignment_sync(cfg: dict, provider: str, model: str, base_url: str, api_key: str, + prepared: "Optional[tuple[str, ModelSwitchResult]]" = None) -> dict: + from hermes_cli.config import save_config + base_url, result = prepared or _prepare_main_assignment(cfg, provider, model, base_url, api_key) provider, model = result.target_provider, result.new_model + provider_entry = _provider_entry(cfg, provider) model_cfg = _apply_main_model_assignment(cfg.get("model", {}), result, api_key) _resolve_assignment_credentials(model_cfg, provider, provider_entry) cfg["model"] = model_cfg @@ -773,17 +786,18 @@ def _apply_aux_assignment_sync(cfg: dict, provider: str, model: str, task: str, def _apply_model_assignment_sync( scope: str, provider: str, model: str, task: str, base_url: str, api_key: str = "", - reasoning_effort: Optional[str] = _UNSET, + reasoning_effort: Optional[str] = _UNSET, prepared: "Optional[tuple[str, ModelSwitchResult]]" = None, ): """Synchronous body of POST /api/model/set. Runs inside ``_profile_scope`` (worker thread) so every load_config/save_config lands in - the requested profile. Raises HTTPException for validation errors. + the requested profile. Raises HTTPException for validation errors. ``prepared`` is a + ``_prepare_main_assignment`` result computed outside the config lock. """ from hermes_cli.config import load_config cfg = load_config() if scope == "main": - return _apply_main_assignment_sync(cfg, provider, model, base_url, api_key) + return _apply_main_assignment_sync(cfg, provider, model, base_url, api_key, prepared) return _apply_aux_assignment_sync(cfg, provider, model, task, base_url, api_key, reasoning_effort)