Skip to content

fix(desktop): prevent MoA autosave defaults explosion from half-filled slots - #64158

Closed
DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/moa-autosave-defaults-explosion
Closed

fix(desktop): prevent MoA autosave defaults explosion from half-filled slots#64158
DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/moa-autosave-defaults-explosion

Conversation

@DavidMetcalfe

Copy link
Copy Markdown
Contributor

Summary

Fixes #64156: MoA preset editor silently replaces user config with hardcoded defaults when a reference model's provider is changed.

What this PR does

Three changes in apps/desktop/src/app/settings/model-settings.tsx:

  1. Filter incomplete reference slots before autosave (sanitizeMoaRefsForSave): when a user changes a reference model's provider, updateMoaSlot clears the model to "" and the 600ms debounced autosave was sending this half-filled slot to the backend. The backend's _clean_slot() rejects empty-model slots, and _normalize_preset() falls back to hardcoded DEFAULT_MOA_REFERENCE_MODELS — silently replacing the entire preset. The sanitizer strips slots with empty model before any save and skips presets that would end up with zero valid references.

  2. Add withActive() to MoA provider dropdowns: the reference and aggregator provider Selects filtered to authenticated providers only, so unauthenticated current values (e.g. openai-codex, which requires OAuth) rendered as blank triggers. The parallel model dropdown already used withActive(). Mirror that pattern.

  3. Add generation counter to scheduleMoaSave(): stale save responses could overwrite newer state (classic async race). Bump a counter on each save and skip setMoa if a newer save was scheduled in the meantime.

Test plan

  • Open Settings → Model → Mixture of Agents
  • Change a reference model's provider to any authenticated provider (Kilo, OpenRouter, etc.)
  • Model dropdown should populate with that provider's models
  • Wait >600ms without selecting a model — the autosave should NOT trigger the defaults explosion
  • The reference should remain at the selected provider (with cleared model, waiting for model selection)
  • Select a model — the autosave should persist the complete slot correctly

Notes

  • The hardcoded defaults in moa_config.py:13-16 reference openai-codex (OAuth, unauthenticated by default), which practically guarantees the blank-provider symptom on fresh installs or wiped configs. This PR mitigates the explosion but a follow-up could make the fallback dynamically select from authenticated providers.

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Jul 14, 2026
…d slots

Three fixes in model-settings.tsx:

1. Filter incomplete slots before autosave: sanitizeMoaRefsForSave() strips
   reference slots with empty model before any autosave (the 600ms debounce
   was sending half-filled provider-but-no-model slots to the backend, where
   _clean_slot rejects them and _normalize_preset falls back to hardcoded
   defaults). Presets with zero valid refs keep their empty reference_models
   array rather than being silently dropped. The aggregator slot is also
   sanitized when its model is empty.

2. Add withActive() to MoA provider dropdowns: the reference and aggregator
   provider Selects filtered to authenticated-only, so unauthenticated
   current values (e.g. openai-codex) rendered blank. Mirror the existing
   pattern from the model Select.

3. Add generation counter to scheduleMoaSave(): stale save responses could
   overwrite newer state. Bump a counter on each save and skip setMoa/setError
   if a newer save was scheduled in the meantime.
@DavidMetcalfe
DavidMetcalfe force-pushed the fix/moa-autosave-defaults-explosion branch from 6ee56ac to 60f359e Compare July 14, 2026 05:35
teknium1 added a commit that referenced this pull request Jul 15, 2026
…esktop autosave until slots are complete

Follow-up hardening on top of #64158 (@DavidMetcalfe):

Backend (the root-cause fix):
- hermes_cli/moa_config.py: add validate_moa_payload() — strict write-time
  counterpart to the deliberately tolerant normalize_moa_config(). Flags
  half-filled slots, empty reference lists, recursive moa slots, naming the
  exact preset/slot.
- hermes_cli/web_server.py: PUT /api/model/moa validates before normalizing
  and returns 422 with the specific problems instead of silently swapping the
  user's preset for hardcoded defaults (#64156). Also declares
  fanout / reference_max_tokens / reasoning_effort on the Pydantic payload so
  client round-trips no longer erase hand-set values.

Desktop:
- Replace sanitize-then-send with hold-while-incomplete: the debounced
  autosave is deferred (not repaired) while any slot is half-filled, and
  flushes once the model pick completes the edit. Mid-edit UI state is never
  repainted by a save response (generation guard covers held edits too).
- updateMoaSlot only clears the model when the provider actually changed.
- Explicit preset ops (set default / add / delete) cancel the pending
  autosave and invalidate in-flight responses so the two writers can't race.
- Stable row keys (preset+index) so mid-edit rows don't remount; cleared
  model shows the 'Model' placeholder instead of vanishing.

Both TS clients' MoaConfigResponse types now declare the round-tripped
fields (fanout, reference_max_tokens, reasoning_effort).

Tests: 12 new backend unit tests (validate_moa_payload contract incl.
validate/normalize agreement), 3 new web_server endpoint tests (422 on
half-filled ref/aggregator, fanout round-trip), 3 new desktop vitest cases
(autosave held while half-filled, flush on completion, same-provider
reselect no-op). E2E validated against a live TestClient with isolated
HERMES_HOME: bug sequence now 422s with config untouched.

Fixes #64156
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #64976 — your commit was cherry-picked onto current main with your authorship preserved in git history (a61a0bc). Thanks for the sharp diagnosis: the half-filled-slot autosave → _clean_slot() rejection → defaults fallback chain was exactly right.

On top of your fix we hardened both sides of the wire:

  • The backend PUT /api/model/moa now validates before normalizing and returns 422 naming the broken preset/slot, so no client (desktop, dashboard, or future) can trigger the silent defaults swap again — addressing the follow-up you suggested in your PR notes.
  • The desktop autosave now holds while a slot is half-filled and flushes when the model pick completes, instead of stripping the mid-edit slot from the payload (which still repainted the row out from under the user).
  • The payload schema also now round-trips fanout / reference_max_tokens / reasoning_effort, fixing a sibling data-loss hole where any client save erased those hand-set values.

Your generation-counter guard and withActive() provider-dropdown fix landed as-is. Much appreciated!

@teknium1 teknium1 closed this Jul 15, 2026
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…esktop autosave until slots are complete

Follow-up hardening on top of NousResearch#64158 (@DavidMetcalfe):

Backend (the root-cause fix):
- hermes_cli/moa_config.py: add validate_moa_payload() — strict write-time
  counterpart to the deliberately tolerant normalize_moa_config(). Flags
  half-filled slots, empty reference lists, recursive moa slots, naming the
  exact preset/slot.
- hermes_cli/web_server.py: PUT /api/model/moa validates before normalizing
  and returns 422 with the specific problems instead of silently swapping the
  user's preset for hardcoded defaults (NousResearch#64156). Also declares
  fanout / reference_max_tokens / reasoning_effort on the Pydantic payload so
  client round-trips no longer erase hand-set values.

Desktop:
- Replace sanitize-then-send with hold-while-incomplete: the debounced
  autosave is deferred (not repaired) while any slot is half-filled, and
  flushes once the model pick completes the edit. Mid-edit UI state is never
  repainted by a save response (generation guard covers held edits too).
- updateMoaSlot only clears the model when the provider actually changed.
- Explicit preset ops (set default / add / delete) cancel the pending
  autosave and invalidate in-flight responses so the two writers can't race.
- Stable row keys (preset+index) so mid-edit rows don't remount; cleared
  model shows the 'Model' placeholder instead of vanishing.

Both TS clients' MoaConfigResponse types now declare the round-tripped
fields (fanout, reference_max_tokens, reasoning_effort).

Tests: 12 new backend unit tests (validate_moa_payload contract incl.
validate/normalize agreement), 3 new web_server endpoint tests (422 on
half-filled ref/aggregator, fanout round-trip), 3 new desktop vitest cases
(autosave held while half-filled, flush on completion, same-provider
reselect no-op). E2E validated against a live TestClient with isolated
HERMES_HOME: bug sequence now 422s with config untouched.

Fixes NousResearch#64156
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…esktop autosave until slots are complete

Follow-up hardening on top of NousResearch#64158 (@DavidMetcalfe):

Backend (the root-cause fix):
- hermes_cli/moa_config.py: add validate_moa_payload() — strict write-time
  counterpart to the deliberately tolerant normalize_moa_config(). Flags
  half-filled slots, empty reference lists, recursive moa slots, naming the
  exact preset/slot.
- hermes_cli/web_server.py: PUT /api/model/moa validates before normalizing
  and returns 422 with the specific problems instead of silently swapping the
  user's preset for hardcoded defaults (NousResearch#64156). Also declares
  fanout / reference_max_tokens / reasoning_effort on the Pydantic payload so
  client round-trips no longer erase hand-set values.

Desktop:
- Replace sanitize-then-send with hold-while-incomplete: the debounced
  autosave is deferred (not repaired) while any slot is half-filled, and
  flushes once the model pick completes the edit. Mid-edit UI state is never
  repainted by a save response (generation guard covers held edits too).
- updateMoaSlot only clears the model when the provider actually changed.
- Explicit preset ops (set default / add / delete) cancel the pending
  autosave and invalidate in-flight responses so the two writers can't race.
- Stable row keys (preset+index) so mid-edit rows don't remount; cleared
  model shows the 'Model' placeholder instead of vanishing.

Both TS clients' MoaConfigResponse types now declare the round-tripped
fields (fanout, reference_max_tokens, reasoning_effort).

Tests: 12 new backend unit tests (validate_moa_payload contract incl.
validate/normalize agreement), 3 new web_server endpoint tests (422 on
half-filled ref/aggregator, fanout round-trip), 3 new desktop vitest cases
(autosave held while half-filled, flush on completion, same-provider
reselect no-op). E2E validated against a live TestClient with isolated
HERMES_HOME: bug sequence now 422s with config untouched.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: MoA preset editor — changing reference provider triggers defaults explosion from half-filled autosave

3 participants