diff --git a/apps/desktop/src/app/settings/fallback-models-field.test.tsx b/apps/desktop/src/app/settings/fallback-models-field.test.tsx index a514396b17..f30869ab75 100644 --- a/apps/desktop/src/app/settings/fallback-models-field.test.tsx +++ b/apps/desktop/src/app/settings/fallback-models-field.test.tsx @@ -78,12 +78,15 @@ describe('FallbackModelsField', () => { await waitFor(() => expect(getGlobalModelOptions).toHaveBeenCalled()) }) - it('removing a row emits the remaining entries', async () => { - const onChange = await renderField(CHAIN) + it('removing a row emits the remaining entries with their routing keys intact', async () => { + // #89184: a hand-written local-gateway chain carries base_url/api_key per + // entry; the editor only owns provider/model and must not strip the rest. + const routed = { provider: 'custom', model: 'glm-5.08', base_url: 'http://gw:8080/v1', api_key: '${GW_KEY}' } + const onChange = await renderField([...CHAIN, routed]) fireEvent.click(screen.getAllByLabelText('Remove')[0]) - expect(onChange.mock.calls.at(-1)?.[0]).toEqual([{ provider: 'openai-codex', model: 'gpt-5.4-mini' }]) + expect(onChange.mock.calls.at(-1)?.[0]).toEqual([{ provider: 'openai-codex', model: 'gpt-5.4-mini' }, routed]) }) it('adding a blank row does not persist a partial entry', async () => { diff --git a/apps/desktop/src/app/settings/fallback-models-field.tsx b/apps/desktop/src/app/settings/fallback-models-field.tsx index d7aa7839df..f9185d95a8 100644 --- a/apps/desktop/src/app/settings/fallback-models-field.tsx +++ b/apps/desktop/src/app/settings/fallback-models-field.tsx @@ -10,14 +10,19 @@ import { cn } from '@/lib/utils' import { CONTROL_TEXT } from './constants' -interface FallbackEntry { +// An entry is `{provider, model}` plus whatever routing the user hand-wrote +// (`base_url`, `api_key`, `key_env`, `api_mode`, ...). The editor only edits +// the two selects; every other key rides along untouched, or an autosave +// would rewrite a local-gateway chain as bare provider/model pairs and route +// fallbacks to the public provider (#89184). +interface FallbackEntry extends Record { provider: string model: string } // Normalize the raw config value (`fallback_providers`: a list of -// `{provider, model}` dicts) into editor rows. Defensive against legacy string -// entries ("provider/model") so the editor never crashes on odd data. +// `{provider, model, ...}` dicts) into editor rows. Defensive against legacy +// string entries ("provider/model") so the editor never crashes on odd data. function normalizeEntries(value: unknown): FallbackEntry[] { if (!Array.isArray(value)) { return [] @@ -27,7 +32,7 @@ function normalizeEntries(value: unknown): FallbackEntry[] { if (item && typeof item === 'object') { const record = item as Record - return { provider: String(record.provider ?? ''), model: String(record.model ?? '') } + return { ...record, provider: String(record.provider ?? ''), model: String(record.model ?? '') } } if (typeof item === 'string') { @@ -47,10 +52,7 @@ function completeEntries(rows: FallbackEntry[]): FallbackEntry[] { } function entriesEqual(a: FallbackEntry[], b: FallbackEntry[]): boolean { - return ( - a.length === b.length && - a.every((entry, index) => entry.provider === b[index]?.provider && entry.model === b[index]?.model) - ) + return a.length === b.length && a.every((entry, index) => JSON.stringify(entry) === JSON.stringify(b[index])) } /** diff --git a/hermes_cli/web_routers/models.py b/hermes_cli/web_routers/models.py index 262340bbc0..e35f528b37 100644 --- a/hermes_cli/web_routers/models.py +++ b/hermes_cli/web_routers/models.py @@ -255,9 +255,13 @@ def set_moa_models(body: MoaConfigPayload, profile: Optional[str] = None): raise HTTPException(status_code=422, detail="Invalid MoA config: " + "; ".join(problems)) normalized = normalize_moa_config(raw) # Merge, don't overwrite: hand-edited keys not in MoaConfigPayload (save_traces, trace_dir) survive. - # See issue #58819. - cfg.setdefault("moa", {}).update(normalized) - save_config(cfg) + # See issue #58819. Write ONLY the moa section (merge_existing deep-merges it over the + # on-disk raw file): saving the whole default-expanded ``cfg`` snapshot re-persisted + # every other section too, so a Desktop MoA autosave could wipe a chain another + # surface wrote meanwhile (#89184, ``fallback_providers: []``). + moa_section = dict(cfg.get("moa") or {}) + moa_section.update(normalized) + save_config({"moa": moa_section}, merge_existing=True) return {"ok": True, **normalized} diff --git a/tests/hermes_cli/test_moa_set_models_preserves_extra_keys.py b/tests/hermes_cli/test_moa_set_models_preserves_extra_keys.py index f5b719d0ba..4c7aa6dd44 100644 --- a/tests/hermes_cli/test_moa_set_models_preserves_extra_keys.py +++ b/tests/hermes_cli/test_moa_set_models_preserves_extra_keys.py @@ -61,7 +61,7 @@ class TestSetMoaModelsPreservesUndeclaredKeys: def fake_load_config(): return dict(existing_cfg) # shallow copy - def fake_save_config(cfg): + def fake_save_config(cfg, **_kwargs): saved_cfg.update(cfg) payload = _base_payload() @@ -82,3 +82,36 @@ class TestSetMoaModelsPreservesUndeclaredKeys: ) + + +def test_moa_save_writes_only_the_moa_section(tmp_path, monkeypatch): + """#89184: a MoA autosave must not re-persist the rest of the effective-config snapshot. + + ``load_config()`` is a default-expanded snapshot; saving it whole after a chain was written + out-of-band (or was simply stale) rewrote ``fallback_providers`` too. Real config pipeline, + temp HERMES_HOME. + """ + import yaml + from hermes_cli.config import get_config_path, load_config, read_raw_config + + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + get_config_path().write_text(yaml.safe_dump({"model": {"default": "m1", "provider": "custom"}}), encoding="utf-8") + stale = load_config() # snapshot BEFORE the chain exists + stale["fallback_providers"] = [] # what the default-expanded snapshot carries + chain = [{"provider": "custom", "model": "glm-5.08", "base_url": "http://gw:8080/v1"}] + raw = read_raw_config() + raw["fallback_providers"] = chain + get_config_path().write_text(yaml.safe_dump(raw), encoding="utf-8") + + with ( + patch("hermes_cli.config.load_config", return_value=stale), + patch("hermes_cli.web_server_profiles._profile_scope"), + ): + set_moa_models(_base_payload()) + + on_disk = read_raw_config() + assert on_disk["fallback_providers"] == chain, "MoA save clobbered fallback_providers" + # The MoA edit itself landed (a non-default value, so default stripping leaves it on disk). + assert on_disk["moa"]["presets"]["default"]["reference_models"][0]["model"] == "gpt-5.5"