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
This commit is contained in:
pyr0ball 2026-03-22 12:01:55 -07:00
parent 0d17b20831
commit acda1e8f5a
4 changed files with 37 additions and 13 deletions

View file

@ -1149,6 +1149,9 @@ def suggest_search(body: dict):
class ByokAckPayload(BaseModel): class ByokAckPayload(BaseModel):
backends: List[str] = [] backends: List[str] = []
class LlmConfigPayload(BaseModel):
backends: List[dict] = []
LLM_CONFIG_PATH = Path("config/llm.yaml") LLM_CONFIG_PATH = Path("config/llm.yaml")
@app.get("/api/settings/system/llm") @app.get("/api/settings/system/llm")
@ -1165,13 +1168,13 @@ def get_llm_config():
raise HTTPException(status_code=500, detail=str(e)) raise HTTPException(status_code=500, detail=str(e))
@app.put("/api/settings/system/llm") @app.put("/api/settings/system/llm")
def save_llm_config(payload: dict): def save_llm_config(payload: LlmConfigPayload):
try: try:
data = {} data = {}
if LLM_CONFIG_PATH.exists(): if LLM_CONFIG_PATH.exists():
with open(LLM_CONFIG_PATH) as f: with open(LLM_CONFIG_PATH) as f:
data = yaml.safe_load(f) or {} 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) LLM_CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True)
with open(LLM_CONFIG_PATH, "w") as f: with open(LLM_CONFIG_PATH, "w") as f:
yaml.dump(data, f, allow_unicode=True, default_flow_style=False) yaml.dump(data, f, allow_unicode=True, default_flow_style=False)

View file

@ -47,6 +47,17 @@ describe('useSystemStore — BYOK gate', () => {
expect(mockFetch).toHaveBeenCalledWith('/api/settings/system/llm', expect.anything()) 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 () => { it('cancelByok() clears pending and restores backends to pre-save state', async () => {
mockFetch.mockResolvedValue({ data: { ok: true }, error: null }) mockFetch.mockResolvedValue({ data: { ok: true }, error: null })
const store = useSystemStore() const store = useSystemStore()

View file

@ -40,18 +40,26 @@ export const useSystemStore = defineStore('settings/system', () => {
async function confirmByok() { async function confirmByok() {
saving.value = true saving.value = true
saveError.value = null
const { error } = await useApiFetch('/api/settings/system/llm/byok-ack', { const { error } = await useApiFetch('/api/settings/system/llm/byok-ack', {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ backends: byokPending.value }), 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 = [] byokPending.value = []
await _commitSave() await _commitSave()
} }
function cancelByok() { function cancelByok() {
backends.value = JSON.parse(JSON.stringify(_preSaveSnapshot)) if (_preSaveSnapshot.length > 0) {
backends.value = JSON.parse(JSON.stringify(_preSaveSnapshot))
}
byokPending.value = [] byokPending.value = []
_preSaveSnapshot = [] _preSaveSnapshot = []
} }

View file

@ -98,16 +98,18 @@ function dragStart(idx: number) {
dragIdx.value = idx dragIdx.value = idx
} }
function dragOver(idx: number) { function dragOver(toFilteredIdx: number) {
if (dragIdx.value === null || dragIdx.value === idx) return if (dragIdx.value === null || dragIdx.value === toFilteredIdx) return
// Reorder store.backends (immutable swap) const fromId = visibleBackends.value[dragIdx.value].id
const toId = visibleBackends.value[toFilteredIdx].id
const arr = [...store.backends] const arr = [...store.backends]
const [moved] = arr.splice(dragIdx.value, 1) const fromFull = arr.findIndex(b => b.id === fromId)
arr.splice(idx, 0, moved) const toFull = arr.findIndex(b => b.id === toId)
store.backends = arr if (fromFull === -1 || toFull === -1) return
// Update priorities const [moved] = arr.splice(fromFull, 1)
store.backends = store.backends.map((b, i) => ({ ...b, priority: i + 1 })) arr.splice(toFull, 0, moved)
dragIdx.value = idx store.backends = arr.map((b, i) => ({ ...b, priority: i + 1 }))
dragIdx.value = toFilteredIdx
} }
function drop() { function drop() {