fix(config): Desktop fallback editor keeps per-entry routing; MoA save writes only the moa section
Two remaining halves of #89184 (Desktop Settings saves rewriting unrelated config): - The `fallback_providers` structured editor normalized every entry down to `{provider, model}`, so any edit (remove a row, pick a model) re-emitted a hand-written local-gateway chain without its `base_url` / `api_key` / `key_env` / `api_mode` — the next autosave persisted bare pairs and the fallbacks silently routed to the public provider. Entries now carry every key through; the editor only owns the two selects. - `PUT /api/model/moa` did `cfg = load_config(); cfg["moa"].update(...); save_config(cfg)`: the whole default-expanded snapshot went back to disk, so a Desktop MoA autosave re-persisted every other section too (the 2026-09-10 repro: `fallback_providers: []` written alongside the MoA block the user had just edited). It now saves `{"moa": ...}` with `merge_existing=True`, the same section-scoped write every other sparse writer uses since #110535. Hand-edited moa keys (#58819) still survive. The `model.default not persisted / base_url cleared` symptom from the 0.20.4 report no longer reproduces on main through the real REST path (Config page diffs against a baseline since 5361867c6d32; `_denormalize_config_from_web` keeps the on-disk `model:` block).
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
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<string, unknown>
|
||||
|
||||
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]))
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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}
|
||||
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user