hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 32 Packages Projects Releases Wiki Activity

test(web): deterministic lost-write race through TestClient; drop the source-text tripwire

Browse Source 
The PR's race test slowed save_config and hoped for an interleave, and
called the six handlers as web_server attributes that no longer exist
(web_server.py is a facade over web_routers/). Its second test read
inspect.getsource() for the lock name — a change-detector on source text.

Replace both with two invariant tests that drive the real endpoints via
TestClient and FORCE the interleaving: writer 1 is held inside save_config
until writer 2 has run load_config (or the lock kept it out). Unlocked the
second save erases the first mutation; locked the writes serialize.
Verified: reverting hermes_cli/web_routers/ to main fails both tests
(KeyError 'providers' / 'aggregator' = the lost write), the fix passes.
This commit is contained in:
teknium1
2026-09-14 19:01:40 -07:00
committed by Teknium
parent eaf0dd0970
commit 131ea24026
1 changed files with 70 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

121
tests/hermes_cli/test_config_rmw_lock.py
View File

@@ -1,15 +1,15 @@
"""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::
Only ``PUT /api/config`` (and the ``config_write_scope`` routers) held ``_CONFIG_MUTATION_LOCK``.
The custom-endpoint handlers, the profile-create model write, ``POST /api/model/set`` and
``PUT /api/model/moa`` ran their load→mutate→save cycles on worker threads without it, so a
writer 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
T1 load (providers={}) T2 load (providers={})
T1 mutate providers.box T2 mutate display.x
T1 save (providers.box) T2 save (providers={} + display.x) <- T1's write erased
Both tests provoke that interleaving with a slowed ``save_config`` and assert both writes land.
Both tests force exactly that interleaving and assert both writes land.
"""
from __future__ import annotations
@@ -38,74 +38,93 @@ def client(monkeypatch, _isolate_hermes_home):
return client
def _slow_saves(monkeypatch, delay: float = 0.2) -> None:
"""Widen the load→save window so an unlocked pair reliably loses a write."""
def _race_second_writer_into_first_writers_save(monkeypatch, first, second, timeout: float = 5.0):
"""Deterministic lost-write interleaving: ``first`` runs until it reaches ``save_config``,
then ``second`` is started and ``first`` waits (up to ``timeout``) for ``second`` to
``load_config`` before saving. Unlocked, ``second`` loads the stale document and its save
erases ``first``'s mutation. With the RMW lock ``second`` blocks before its load, ``first``'s
wait times out, and the two writes serialize. Returns ``(first_response, second_response)``."""
import hermes_cli.config as cfg_mod
real_save = cfg_mod.save_config
real_save, real_load = cfg_mod.save_config, cfg_mod.load_config
first_at_save, second_loaded = threading.Event(), threading.Event()
gate = threading.Lock()
def slow_save(config, *args, **kwargs):
time.sleep(delay)
def gated_save(config, *args, **kwargs):
# The first save_config call in the test is the first writer's (the second has not
# started yet). Hold it until the second writer has loaded — or the lock kept it out.
if not first_at_save.is_set():
first_at_save.set()
second_loaded.wait(timeout)
with gate:
return real_save(config, *args, **kwargs)
return real_save(config, *args, **kwargs)
monkeypatch.setattr(cfg_mod, "save_config", slow_save)
def spied_load(*args, **kwargs):
cfg = real_load(*args, **kwargs)
# Any load while the first writer sits blocked in save_config is the second writer's.
if first_at_save.is_set() and not gate.locked():
second_loaded.set()
return cfg
monkeypatch.setattr(cfg_mod, "save_config", gated_save)
monkeypatch.setattr(cfg_mod, "load_config", spied_load)
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)]
results: list = [None, None]
threads = [threading.Thread(target=lambda: results.__setitem__(0, first())),
threading.Thread(target=lambda: results.__setitem__(1, second()))]
threads[0].start()
assert first_at_save.wait(30), "first writer never reached save_config"
threads[1].start()
for t in threads:
t.start()
for t in threads:
t.join(timeout=30)
t.join(timeout=60)
return results
def _model_block(home) -> dict:
return yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8"))
def _on_disk() -> dict:
from hermes_constants import get_hermes_home
return yaml.safe_load((get_hermes_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"})
def test_custom_endpoint_upsert_racing_config_autosave_keeps_both_writes(client, monkeypatch):
"""Saving a custom endpoint (sync-def handler on a worker thread) while the settings-page
autosave (PUT /api/config) is in flight: the new ``providers`` entry AND the autosaved field
both survive."""
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})
r1, r2 = _race_second_writer_into_first_writers_save(monkeypatch, autosave, upsert)
assert r1.status_code == 200, r1.text
assert r2.status_code == 200, r2.text
on_disk = _on_disk()
assert "racebox" in on_disk["providers"]
assert on_disk["display"]["personality"] == "canary"
def test_custom_endpoint_activate_racing_moa_save_keeps_both_writes(client, monkeypatch):
"""Two worker-thread writers (custom-endpoint activate vs MoA save) serialize through the
same lock — the ``model`` switch and the ``moa`` section are both on disk afterwards."""
from hermes_cli.config import load_config, save_config
cfg = load_config()
cfg["providers"] = {"racebox": {"base_url": "http://racebox:8000/v1", "model": "race-model", "api_key": "k"}}
save_config(cfg)
activate = lambda: client.post("/api/providers/custom-endpoints/racebox/activate") # noqa: E731
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)
r1, r2 = _race_second_writer_into_first_writers_save(monkeypatch, activate, 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"]
on_disk = _on_disk()
assert on_disk["model"]["provider"] == "racebox"
assert on_disk["moa"]["aggregator"]["model"] == "openai/gpt-5.5"