From 5afb752be6d8cc5d5897e2a4fbce5575f1aa3c6b Mon Sep 17 00:00:00 2001 From: pyr0ball Date: Sun, 22 Mar 2026 12:01:55 -0700 Subject: [PATCH] fix(settings): system tab review fixes - guard confirmByok() against byok-ack POST failure (leave modal open on error) - fix drag reorder to use ID-based index lookup (not filtered-list index) - guard cancelByok() against empty snapshot - add LlmConfigPayload Pydantic model for PUT endpoint - add test for confirmByok() failure path --- dev-api.py | 7 +++++-- web/src/stores/settings/system.test.ts | 11 ++++++++++ web/src/stores/settings/system.ts | 12 +++++++++-- web/src/views/settings/SystemSettingsView.vue | 20 ++++++++++--------- 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/dev-api.py b/dev-api.py index d74c738..d4c1502 100644 --- a/dev-api.py +++ b/dev-api.py @@ -1149,6 +1149,9 @@ def suggest_search(body: dict): class ByokAckPayload(BaseModel): backends: List[str] = [] +class LlmConfigPayload(BaseModel): + backends: List[dict] = [] + LLM_CONFIG_PATH = Path("config/llm.yaml") @app.get("/api/settings/system/llm") @@ -1165,13 +1168,13 @@ def get_llm_config(): raise HTTPException(status_code=500, detail=str(e)) @app.put("/api/settings/system/llm") -def save_llm_config(payload: dict): +def save_llm_config(payload: LlmConfigPayload): try: data = {} if LLM_CONFIG_PATH.exists(): with open(LLM_CONFIG_PATH) as f: data = yaml.safe_load(f) or {} - data["backends"] = payload.get("backends", []) + data["backends"] = payload.backends LLM_CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) with open(LLM_CONFIG_PATH, "w") as f: yaml.dump(data, f, allow_unicode=True, default_flow_style=False) diff --git a/web/src/stores/settings/system.test.ts b/web/src/stores/settings/system.test.ts index c4ececc..665f080 100644 --- a/web/src/stores/settings/system.test.ts +++ b/web/src/stores/settings/system.test.ts @@ -47,6 +47,17 @@ describe('useSystemStore — BYOK gate', () => { expect(mockFetch).toHaveBeenCalledWith('/api/settings/system/llm', expect.anything()) }) + it('confirmByok() sets saveError and leaves modal open when ack POST fails', async () => { + mockFetch.mockResolvedValue({ data: null, error: 'Network error' }) + const store = useSystemStore() + store.byokPending = ['anthropic'] + store.backends = [{ id: 'anthropic', enabled: true, priority: 1 }] + await store.confirmByok() + expect(store.saveError).toBeTruthy() + expect(store.byokPending).toContain('anthropic') // modal stays open + expect(mockFetch).not.toHaveBeenCalledWith('/api/settings/system/llm', expect.anything()) + }) + it('cancelByok() clears pending and restores backends to pre-save state', async () => { mockFetch.mockResolvedValue({ data: { ok: true }, error: null }) const store = useSystemStore() diff --git a/web/src/stores/settings/system.ts b/web/src/stores/settings/system.ts index f3fea40..9a46998 100644 --- a/web/src/stores/settings/system.ts +++ b/web/src/stores/settings/system.ts @@ -40,18 +40,26 @@ export const useSystemStore = defineStore('settings/system', () => { async function confirmByok() { saving.value = true + saveError.value = null const { error } = await useApiFetch('/api/settings/system/llm/byok-ack', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ backends: byokPending.value }), }) - if (!error) byokAcknowledged.value = [...byokAcknowledged.value, ...byokPending.value] + if (error) { + saving.value = false + saveError.value = 'Failed to save acknowledgment — please try again.' + return // leave modal open, byokPending intact + } + byokAcknowledged.value = [...byokAcknowledged.value, ...byokPending.value] byokPending.value = [] await _commitSave() } function cancelByok() { - backends.value = JSON.parse(JSON.stringify(_preSaveSnapshot)) + if (_preSaveSnapshot.length > 0) { + backends.value = JSON.parse(JSON.stringify(_preSaveSnapshot)) + } byokPending.value = [] _preSaveSnapshot = [] } diff --git a/web/src/views/settings/SystemSettingsView.vue b/web/src/views/settings/SystemSettingsView.vue index e0e4433..e7cf73f 100644 --- a/web/src/views/settings/SystemSettingsView.vue +++ b/web/src/views/settings/SystemSettingsView.vue @@ -98,16 +98,18 @@ function dragStart(idx: number) { dragIdx.value = idx } -function dragOver(idx: number) { - if (dragIdx.value === null || dragIdx.value === idx) return - // Reorder store.backends (immutable swap) +function dragOver(toFilteredIdx: number) { + if (dragIdx.value === null || dragIdx.value === toFilteredIdx) return + const fromId = visibleBackends.value[dragIdx.value].id + const toId = visibleBackends.value[toFilteredIdx].id const arr = [...store.backends] - const [moved] = arr.splice(dragIdx.value, 1) - arr.splice(idx, 0, moved) - store.backends = arr - // Update priorities - store.backends = store.backends.map((b, i) => ({ ...b, priority: i + 1 })) - dragIdx.value = idx + const fromFull = arr.findIndex(b => b.id === fromId) + const toFull = arr.findIndex(b => b.id === toId) + if (fromFull === -1 || toFull === -1) return + const [moved] = arr.splice(fromFull, 1) + arr.splice(toFull, 0, moved) + store.backends = arr.map((b, i) => ({ ...b, priority: i + 1 })) + dragIdx.value = toFilteredIdx } function drop() {