fix(dashboard): serialize the memory-provider config read-modify-write

[0xGr1mm]

What does this PR do?

965a54878 / 4ecdee38a moved the dashboard's config handlers off the event loop and introduced _CONFIG_MUTATION_LOCK to replace the serialization the loop used to provide for free. Its docstring states the contract:

Serializes read-modify-write cycles over config.yaml for handlers that run in worker threads (asyncio.to_thread). config.py's _CONFIG_LOCK covers each load_config()/save_config() call individually, not the span between them [...] off-loop two concurrent updates could interleave load->mutate->save and silently drop one another's writes.

PUT /api/memory/providers/{name}/config runs off-loop and does exactly that span, without the lock:

def _run():
    with _profile_scope(profile):
        ...
        _write_memory_provider_config_values(name, provider, values)
        _require_memory_provider_ready(name)
        config = load_config()          # read
        memory_config = config.get("memory")
        ...
        memory_config["provider"] = name  # mutate
        save_config(config)               # save
...
return await asyncio.to_thread(_run)

Any concurrent writer that commits between the load_config() and the save_config() is erased by the stale save. Measured against a concurrent PUT /api/dashboard/theme:

theme write lost to a concurrent memory-provider write
assert 'default' == 'midnight'

The declared-surface branch mutates config too, through _update_memory_provider_config, so the lock wraps both branches rather than just the visible span.

Correction: my first scan was wrong, and the gap is wider

The original description said a scan found "exactly one" site. That claim was
wrong, and it was wrong because the scan keyed on _run closures rather than on
what actually matters, which is whether the span runs off the event loop.

Triage caught the first miss: _apply_model_assignment_sync is dispatched
through asyncio.to_thread under a differently named closure. Rescanning by
reachability surfaced four more, and they are a shape I had not considered at
all:

Site	Why it runs off-loop
_apply_model_assignment_sync	asyncio.to_thread(_apply_assignment)
set_moa_models (PUT /api/model/moa)	plain def FastAPI route
upsert_custom_endpoint (POST /api/providers/custom-endpoints)	plain def FastAPI route
activate_custom_endpoint (POST .../{id}/activate)	plain def FastAPI route
delete_custom_endpoint (DELETE .../{id})	plain def FastAPI route

FastAPI runs non-async path operations in its own threadpool, so a sync route
handler is exactly as off-loop as a to_thread call and gets none of the loop's
free serialization. All five do load_config() -> mutate -> save_config() with
no lock.

Two candidates the rescan flagged are deliberately not wrapped, and the
reasons are worth stating: _save_memory_provider_native_config is only reached
from inside the span the first commit already locks, and the lock is an RLock,
so it is covered; _write_profile_model and _write_profile_mcp_servers have no
call sites at all and are referenced only from docstrings.

The shape of the second commit

A _serialized_config_write decorator rather than five re-indented bodies. It
keeps the diff readable, and functools.wraps preserves __wrapped__, which is
what FastAPI follows when it builds the request signature; a test pins that the
parameters are still visible.

The last new test rescans the module by reachability rather than trusting a
hand-kept list, so the next sync @app.* handler that reads and writes config
without the lock fails a test instead of waiting for another manual sweep. On
the unpatched module it reports all four route handlers by name.

Related Issue

No issue; found while reading the off-loop sweep after it landed. Searched open and merged PRs first, per CONTRIBUTING's search-first section:

gh search prs --repo NousResearch/hermes-agent "config mutation lock"  --limit 20
gh search prs --repo NousResearch/hermes-agent "config RMW"            --limit 20
gh search prs --repo NousResearch/hermes-agent "memory provider config" --limit 20

I also listed the most recently opened PRs by creation time rather than relying on search alone, because the index lags new PRs by minutes. Nothing covers this handler. This is the same defect class 4ecdee38a ("close config-RMW gaps left by the off-loop sweep") set out to close; this is one it did not reach.

Type of Change

Bug fix (non-breaking). No behaviour change on the success path beyond serialization; no config keys, no API shape change.

Changes Made

• hermes_cli/web_server.py - update_memory_provider_config's worker body takes _CONFIG_MUTATION_LOCK around both branches, matching the idiom used by the handlers the sweep already covered.
• tests/hermes_cli/test_memory_provider_config_rmw_lock.py - new, 1 test.

How to Test

pytest tests/hermes_cli/test_memory_provider_config_rmw_lock.py -q

1 passed. It races a real PUT /api/memory/providers/{name}/config against a real PUT /api/dashboard/theme through Starlette's TestClient, delaying only the memory writer's save so the theme write lands inside its span. Modelled on the existing TestConfigMutationLock::test_plugin_providers_put_serialized_against_other_writers.

Reverting only hermes_cli/web_server.py and rerunning:

E  AssertionError: theme write lost to a concurrent memory-provider write -
E    PUT /api/memory/providers/{name}/config is not holding
E    _CONFIG_MUTATION_LOCK around its read-modify-write span
E  assert 'default' == 'midnight'

Two details in that test are load-bearing and worth flagging, because a naive version of it passes on unpatched code and proves nothing. web_server binds save_config at module import, so the slow wrapper has to replace web_server.save_config, not hermes_cli.config.save_config. And the delay is keyed on the payload so only the memory writer is slowed; slowing both writers hides the interleaving. My first attempt got both of these wrong and passed against the unpatched handler.

Full tests/hermes_cli/ suite, this branch vs main, same environment, run serially:

main:   169 failed, 4323 passed, 19 skipped
branch: 169 failed, 4324 passed, 19 skipped

Comparing the failing sets rather than the counts: zero regressions, the two sets are identical. The +1 is this PR's new test. Those 169 are pre-existing in a non-hermetic single-process run of this suite; CI's per-file isolation via run_tests_parallel.py is the supported path.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(dashboard): …)
• I searched for existing PRs to make sure this isn't a duplicate (queries above)
• My PR contains only changes related to this fix
• I've run the affected suites and all tests pass
• I've added tests for my changes
• I've tested on my platform: macOS 15 (Darwin 25.5), Python 3.11

Documentation & Housekeeping

• I've updated relevant documentation; the new block carries a comment naming both mutating branches and why the span needs the lock
• cli-config.yaml.example is N/A, no config keys
• CONTRIBUTING.md / AGENTS.md are N/A
• Cross-platform impact is N/A, in-process locking
• Tool descriptions/schemas are N/A

Notes for the reviewer

The lock is an RLock, so wrapping the whole worker body is safe even though _write_memory_provider_config_values and _update_memory_provider_config may themselves reach code that takes it. I wrapped the body rather than just the three-line span so the declared-surface branch is covered too; if you would rather keep the critical section minimal and add a second with inside that branch, that reads fine as well.

The scan I described is reproducible and cheap. If it would help, the same shape could be turned into a lint-style test that fails when a new to_thread closure pairs a config read with a config save and no lock, so the next sweep does not have to be done by hand.

[spfcraze]

This was generated by AI during triage.

Summary:
A second off-loop config read-modify-write span of the class this PR locks is live at POST /api/model/set: _apply_model_assignment_sync (hermes_cli/web_server.py:6645) loads at 6654 and saves at 6709/6788/6823 via asyncio.to_thread(_apply_assignment) (6637), outside every _CONFIG_MUTATION_LOCK site.

Problems:

• _apply_model_assignment_sync (hermes_cli/web_server.py:6645) runs cfg = load_config() at 6654 and save_config(cfg) at 6709 (main scope), 6788 (auxiliary reset) and 6823 (auxiliary assign), dispatched through asyncio.to_thread(_apply_assignment) at 6637. The file's with _CONFIG_MUTATION_LOCK: sites (6972, 12938, 13252, 13296, 16700, 16750, 17291, 17311) all sit outside that body — the same off-loop load→mutate→save shape the lock's docstring (2969-2976) says can "silently drop one another's writes".
• The description's scan claim ("exactly one came back, and it is this handler") covered _run-named closures and the load/save pair inside each closure body; _apply_assignment (hermes_cli/web_server.py:6631) matches neither — its RMW pair lives in the helper _apply_model_assignment_sync, which this diff does not touch.

Solution:
Wrap _apply_model_assignment_sync's span in the same _CONFIG_MUTATION_LOCK the covered handlers use — with _CONFIG_MUTATION_LOCK: around the body of _apply_assignment (hermes_cli/web_server.py:6631) — so the class of off-loop config RMW spans is serialized at this handler too.

---

Checked against e2d2ad0 — the tip of fix/memory-provider-config-rmw-lock when this was written — and 81413f0, main at the same moment.