diff --git a/hermes_cli/web_routers/config_env.py b/hermes_cli/web_routers/config_env.py index 467c603991..ad50514065 100644 --- a/hermes_cli/web_routers/config_env.py +++ b/hermes_cli/web_routers/config_env.py @@ -543,7 +543,10 @@ def list_custom_endpoints(profile: Optional[str] = None): def upsert_custom_endpoint(body: CustomEndpointUpdate, profile: Optional[str] = None): """Create or update a v12+ ``providers`` custom endpoint entry.""" with http_failure("POST /api/providers/custom-endpoints failed", 500, detail="Failed to save custom endpoint"): - with _config_profile_scope(profile): + # Sync-def endpoints run on worker threads: the load→mutate→save span + # holds _CONFIG_MUTATION_LOCK so a concurrent config autosave cannot + # drop this write (or vice versa). + with _config_profile_scope(profile), _CONFIG_MUTATION_LOCK: cfg = load_config() endpoint_id, _entry = _write_custom_endpoint(cfg, body) save_config(cfg) @@ -560,7 +563,7 @@ def activate_custom_endpoint(endpoint_id: str, profile: Optional[str] = None): f"POST /api/providers/custom-endpoints/{endpoint_id}/activate failed", 500, detail="Failed to activate custom endpoint", ): - with _config_profile_scope(profile): + with _config_profile_scope(profile), _CONFIG_MUTATION_LOCK: # RMW span cfg = load_config() provider_key = _custom_endpoint_id(endpoint_id) _stored, entry = find_provider_entry(cfg.get("providers"), provider_key) @@ -601,7 +604,7 @@ def delete_custom_endpoint(endpoint_id: str, profile: Optional[str] = None): f"DELETE /api/providers/custom-endpoints/{endpoint_id} failed", 500, detail="Failed to delete custom endpoint", ): - with _config_profile_scope(profile): + with _config_profile_scope(profile), _CONFIG_MUTATION_LOCK: # RMW span cfg = load_config() provider_key = _custom_endpoint_id(endpoint_id) providers = cfg.get("providers") diff --git a/hermes_cli/web_routers/memory_providers.py b/hermes_cli/web_routers/memory_providers.py index 83c71ca573..777326cae8 100644 --- a/hermes_cli/web_routers/memory_providers.py +++ b/hermes_cli/web_routers/memory_providers.py @@ -22,7 +22,7 @@ from hermes_cli.web_server_memory import ( _coerce_bool, _field_default, _field_is_set, _field_value, _field_visible, _load_memory_provider, _memory_provider_manifest, _memory_provider_setup_info, _memory_provider_setup_manifest, _normalize_memory_provider_schema, _read_memory_provider_existing_values, _require_memory_provider_ready, _run_setup_command, ) from hermes_cli.web_models import MemoryProviderConfigUpdate, MemoryProviderSetupRequest -from hermes_cli.web_routers._common import scoped_to_thread +from hermes_cli.web_routers._common import _CONFIG_MUTATION_LOCK, scoped_to_thread from plugins.memory.config_schema import ( STORAGE_HONCHO_HOST_BLOCK, ProviderConfigSchema, ProviderField, get_provider_config_schema, ) @@ -304,11 +304,12 @@ def _memory_section(config: Dict[str, Any]) -> Dict[str, Any]: def _update_memory_provider_config(provider: ProviderConfigSchema, values: Dict[str, str]) -> None: writer = _write_provider_honcho if provider.storage == STORAGE_HONCHO_HOST_BLOCK else _write_provider_flat writer(provider, values) - config = load_config() - memory_config = _memory_section(config) - if memory_config.get("provider") != provider.name: - memory_config["provider"] = provider.name - save_config(config) + with _CONFIG_MUTATION_LOCK: # RMW span vs. the dashboard's config autosave + config = load_config() + memory_config = _memory_section(config) + if memory_config.get("provider") != provider.name: + memory_config["provider"] = provider.name + save_config(config) # ── Setup: dependency installation ──────────────────────────────────────────── @@ -468,11 +469,12 @@ def _save_memory_provider_native_config(name: str, provider: Any, values: Dict[s if _BaseMemoryProvider is None or type(provider).save_config is not _BaseMemoryProvider.save_config: provider.save_config(values, str(get_hermes_home())) return - cfg = load_config() - memory_cfg = _memory_section(cfg) - current = memory_cfg.get(name) - memory_cfg[name] = {**(current if isinstance(current, dict) else {}), **values} - save_config(cfg) + with _CONFIG_MUTATION_LOCK: # RMW span vs. the dashboard's config autosave + cfg = load_config() + memory_cfg = _memory_section(cfg) + current = memory_cfg.get(name) + memory_cfg[name] = {**(current if isinstance(current, dict) else {}), **values} + save_config(cfg) def _write_memory_provider_config_values(name: str, provider: Any, values: Dict[str, Any]) -> None: @@ -567,9 +569,10 @@ async def update_memory_provider_config( raise _unknown_provider(name) _write_memory_provider_config_values(name, provider, values) _require_memory_provider_ready(name) - config = load_config() - _memory_section(config)["provider"] = name - save_config(config) + with _CONFIG_MUTATION_LOCK: # RMW span vs. the dashboard's config autosave + config = load_config() + _memory_section(config)["provider"] = name + save_config(config) _invalidate_plugins_hub_cache() return {"ok": True, "active": name} diff --git a/hermes_cli/web_routers/models.py b/hermes_cli/web_routers/models.py index e35f528b37..bb7d5be163 100644 --- a/hermes_cli/web_routers/models.py +++ b/hermes_cli/web_routers/models.py @@ -17,7 +17,7 @@ from hermes_cli.web_server_config import ( 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 http_failure +from hermes_cli.web_routers._common import config_write_scope, http_failure _log = logging.getLogger("hermes_cli.web_server") router = APIRouter() @@ -234,7 +234,10 @@ def set_moa_models(body: MoaConfigPayload, profile: Optional[str] = None): with http_failure("PUT /api/model/moa failed", 500, detail="Failed to save MoA config"): from hermes_cli.moa_config import normalize_moa_config, validate_moa_payload - with _profile_scope(body.profile or profile): + # load→mutate→save runs on a worker thread (sync-def endpoint); the + # desktop's debounced PUT /api/config autosave races it, so the whole + # span holds _CONFIG_MUTATION_LOCK or one of the two saves is dropped. + with config_write_scope(body.profile or profile): cfg = load_config() if body.presets: raw = { @@ -296,7 +299,9 @@ 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(): - with _profile_scope(body.profile or profile): + # 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) diff --git a/hermes_cli/web_routers/profiles.py b/hermes_cli/web_routers/profiles.py index c3bfed728d..80937b177a 100644 --- a/hermes_cli/web_routers/profiles.py +++ b/hermes_cli/web_routers/profiles.py @@ -33,6 +33,7 @@ from hermes_cli.web_server_config import ( _apply_main_model_assignment, _normalize_main_model_assignment, _validated_main_model_selection, ) from hermes_cli.web_server_gateway import _strip_session_list_rows +from hermes_cli.web_routers._common import _CONFIG_MUTATION_LOCK from hermes_cli.web_server_profiles import ( _fallback_profile_dicts, _hub_action_name, _write_profile_mcp_servers, ) @@ -110,7 +111,7 @@ def _write_profile_model(profile_dir: Path, provider: str, model: str, validate_ with _hermes_home_scope(validate_in or profile_dir): provider, model = _normalize_main_model_assignment(provider, model) result = _validated_main_model_selection(load_config(), provider, model) - with _hermes_home_scope(profile_dir): + with _hermes_home_scope(profile_dir), _CONFIG_MUTATION_LOCK: # RMW span cfg = load_config() cfg["model"] = _apply_main_model_assignment(cfg.get("model", {}), result) save_config(cfg) diff --git a/tests/hermes_cli/test_config_rmw_lock.py b/tests/hermes_cli/test_config_rmw_lock.py new file mode 100644 index 0000000000..76fd64948f --- /dev/null +++ b/tests/hermes_cli/test_config_rmw_lock.py @@ -0,0 +1,111 @@ +"""Concurrent dashboard config writers must not drop each other's mutations. + +Only ``PUT /api/config`` used to hold ``_CONFIG_MUTATION_LOCK``. ``POST /api/model/set``, +``PUT /api/model/moa``, the custom-endpoint handlers and the memory-provider saves ran their +load→mutate→save cycles unlocked on worker threads, so a model assignment racing the desktop's +debounced whole-record autosave interleaved as:: + + T1 load (model=A) T2 load (model=A) + T1 mutate model=B T2 mutate display.x + T1 save (model=B) T2 save (model=A + display.x) <- T1's write erased + +Both tests provoke that interleaving with a slowed ``save_config`` and assert both writes land. +""" + +from __future__ import annotations + +import threading +import time + +import pytest +import yaml + + +@pytest.fixture +def client(monkeypatch, _isolate_hermes_home): + from starlette.testclient import TestClient + + from hermes_cli.config import load_config, save_config + from hermes_cli.web_server import _SESSION_HEADER_NAME, _SESSION_TOKEN, app + + monkeypatch.setattr("hermes_cli.model_cost_guard.expensive_model_warning", lambda *_a, **_k: None) + cfg = load_config() + cfg["model"] = {"provider": "openrouter", "default": "openai/gpt-5.5"} + save_config(cfg) + + client = TestClient(app) + client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN + return client + + +def _slow_saves(monkeypatch, delay: float = 0.2) -> None: + """Widen the load→save window so an unlocked pair reliably loses a write.""" + import hermes_cli.config as cfg_mod + + real_save = cfg_mod.save_config + + def slow_save(config, *args, **kwargs): + time.sleep(delay) + return real_save(config, *args, **kwargs) + + monkeypatch.setattr(cfg_mod, "save_config", slow_save) + + +def _race(*calls): + results: list = [None] * len(calls) + + def run(i, fn): + results[i] = fn() + + threads = [threading.Thread(target=run, args=(i, fn)) for i, fn in enumerate(calls)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=30) + return results + + +def _model_block(home) -> dict: + return yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8")) + + +def test_model_set_racing_config_autosave_keeps_both_writes(client, monkeypatch, _isolate_hermes_home): + """``applyMainModel`` (POST /api/model/set) while the settings autosave (PUT /api/config) is + in flight: the model assignment AND the autosaved field both survive.""" + _slow_saves(monkeypatch) + + set_model = lambda: client.post( # noqa: E731 + "/api/model/set", json={"scope": "main", "provider": "openrouter", "model": "anthropic/claude-sonnet-4"}) + autosave = lambda: client.put( # noqa: E731 + "/api/config", json={"config": {"display": {"personality": "canary"}}}) + + r1, r2 = _race(set_model, autosave) + assert r1.status_code == 200, r1.text + assert r2.status_code == 200, r2.text + + on_disk = _model_block(_isolate_hermes_home) + assert on_disk["model"]["default"] == "anthropic/claude-sonnet-4" + assert on_disk["display"]["personality"] == "canary" + + +def test_custom_endpoint_upsert_racing_moa_save_keeps_both_writes(client, monkeypatch, _isolate_hermes_home): + """Two sync-def writers on worker threads (custom-endpoint upsert vs MoA save) serialize + through the same lock — neither top-level section is lost.""" + _slow_saves(monkeypatch) + + upsert = lambda: client.post( # noqa: E731 + "/api/providers/custom-endpoints", + json={"id": "racebox", "name": "racebox", "base_url": "http://racebox:8000/v1", "model": "race-model", + "discover_models": False}) + moa = lambda: client.put( # noqa: E731 + "/api/model/moa", + json={"reference_models": [{"provider": "openrouter", "model": "openai/gpt-5.5"}], + "aggregator": {"provider": "openrouter", "model": "openai/gpt-5.5"}}) + + r1, r2 = _race(upsert, moa) + assert r1.status_code == 200, r1.text + assert r2.status_code == 200, r2.text + + on_disk = _model_block(_isolate_hermes_home) + assert "racebox" in on_disk["providers"] + assert on_disk["moa"]["aggregator"]["model"] == "openai/gpt-5.5"