Skip to content

fix(web): config RMW handlers hold the mutation lock — concurrent saves stop dropping writes - #90512

Open
teknium1 wants to merge 1 commit into
mainfrom
fix/config-rmw-lock
Open

fix(web): config RMW handlers hold the mutation lock — concurrent saves stop dropping writes#90512
teknium1 wants to merge 1 commit into
mainfrom
fix/config-rmw-lock

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Model assignment, MoA saves, custom-endpoint create/activate/delete, memory-provider saves, and the profile-dir model write no longer race the desktop's config autosave — every config read-modify-write span in web_server.py now holds _CONFIG_MUTATION_LOCK.

The lock existed precisely for this (its comment: off-loop handlers interleave load→mutate→save and silently drop one another's writes), but only PUT /api/config took it. All the other writers run in worker threads (sync-def endpoints / asyncio.to_thread), and the desktop makes the race real: applyMainModel fires POST /api/model/set while the settings page's debounced PUT /api/config autosave is in flight — whichever save_config landed second dropped the other's mutation (the lost-write flavor of #88913/#89184).

Changes

  • hermes_cli/web_server.py: _CONFIG_MUTATION_LOCK (RLock, worker-thread-only) now wraps the RMW spans in:
    • _apply_model_assignment_sync (main + auxiliary scopes, via a thin locked wrapper)
    • set_moa_models
    • upsert_custom_endpoint / activate_custom_endpoint / delete_custom_endpoint
    • _update_memory_provider_config + the memory-provider activate save
    • the profile-dir model write (set_hermes_home_override path)
  • tests/hermes_cli/test_config_rmw_lock.py: a real two-thread race with a slowed save_config asserting BOTH mutations survive, plus a source-contract tripwire that each flagged handler still holds the lock.

Validation

Result
New tests + moa extra-keys sibling 3/3 pass
Sabotage (web_server.py reverted) race test genuinely loses a write — both tests fail
ruff on web_server.py clean

From the settings-pipeline audit (Bug 4, MED). No event-loop blocking: the lock is only ever held in worker threads.

Infographic

Config writes one at a time

…t saves stop dropping writes

Only PUT /api/config took the span lock; model.set, moa, custom-endpoint
create/activate/delete, memory-provider saves, and the profile-dir model
write ran load→mutate→save unlocked in worker threads. The desktop fires
these concurrently with its debounced autosave — whichever save landed
second silently dropped the other's mutation (#88913/#89184 lost-write
flavor). Race test with slowed save proves both writes now survive.
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on 803e295 — fix(web): config RMW handlers hold _CONFIG_MUTATION_LOCK so

⚠️ Warnings

CI timings · View report · View job

Wall time 20m54s vs 4m33s (+359.3%). 16 job(s) slower, 7 faster, 1 unchanged.

  • Python tests / Run tests slice 2/12: +57.0s
  • Python tests / Run tests slice 6/12: -37.0s
  • Python tests / Run tests slice 12/12: +33.0s
  • Python tests / Run tests slice 8/12: -31.0s
  • Python tests / Run tests slice 9/12: +26.0s

OSV vulnerability scan · View job

7 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/plugins Plugin system and bundled plugins area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #81598 and #81698. This broader patch serializes the shared config mutation surface across their targeted endpoints as well as additional writers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants