From 18abc46162ed813b217fb77ac876d3924014e350 Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Sat, 25 Jul 2026 23:50:34 +0700 Subject: [PATCH] fix(settings): stop a background refetch from wiping unsaved agent edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit keepDirtyValues covers reset(), and the create form seeds its agents with setValue, which ignores it. So on /settings/providers/new?type=… every field under agents.* snapped back to the type's defaults whenever a settingsProviders result landed that differed from the rendered one — a reconnect refetch, another session touching a provider, a changed model catalogue — while the dirty name and type survived, making the loss look arbitrary. Skip the re-seed when the type has not changed and the agents subtree is already dirty; a real type switch still re-seeds, since the previous type's models and defaults are wrong for it. The create-path twin of the existing edit-path test fails on the old code with the edited temperature back at the default; a second case pins the type switch, which a plain dirty-guard would have broken. Co-Authored-By: Claude Opus 4.8 --- .../pages/settings/settings-provider.test.tsx | 59 +++++++++++++++++-- .../src/pages/settings/settings-provider.tsx | 14 ++++- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/frontend/src/pages/settings/settings-provider.test.tsx b/frontend/src/pages/settings/settings-provider.test.tsx index 3af4c6a3..ce5f3313 100644 --- a/frontend/src/pages/settings/settings-provider.test.tsx +++ b/frontend/src/pages/settings/settings-provider.test.tsx @@ -26,8 +26,6 @@ const enabled = { qwen: true, }; -const emptyProvider = { agents: {} }; - const userDefined = [ { agents: {}, @@ -47,13 +45,23 @@ const userDefined = [ }, ]; +// The create form seeds agents from the type's defaults, and bails when the type has no models — +// so a fixture with an empty catalogue would make the seeding tests vacuously green. +const agentDefaults = (model: string, temperature: number) => ({ + simple: { maxTokens: 1000, model, temperature }, +}); + const settingsProviders = { default: { - anthropic: emptyProvider, - openai: emptyProvider, + anthropic: { agents: agentDefaults('claude-e2e', 0.2) }, + openai: { agents: agentDefaults('gpt-e2e', 0.7) }, }, enabled, - models: { anthropic: [], minimax: [], openai: [] }, + models: { + anthropic: [{ name: 'claude-e2e' }], + minimax: [], + openai: [{ name: 'gpt-e2e' }], + }, userDefined, }; @@ -159,6 +167,47 @@ describe('SettingsProvider create-form type guards', () => { expect(navigate).not.toHaveBeenCalled(); }); + const expandAgent = () => { + if (!screen.queryByLabelText('Temperature')) { + fireEvent.click(screen.getByRole('button', { name: /Simple/ })); + } + }; + + const temperatureInput = () => screen.getByLabelText('Temperature') as HTMLInputElement; + + it('preserves an in-flight agent edit on the create form across a background refetch', () => { + setSearch('type=openai'); + const { rerender } = render(); + + expandAgent(); + expect(temperatureInput().value).toBe('0.7'); + + fireEvent.change(temperatureInput(), { target: { value: '1.5' } }); + + // A refetch that carries no change leaves `data` referentially equal and is inert, so the + // payload has to actually differ for this to exercise the seeding effect. + queryResult.data = { + settingsProviders: { ...settingsProviders, userDefined: [...userDefined] }, + }; + rerender(); + + expect(temperatureInput().value).toBe('1.5'); + }); + + it('still re-seeds the agents when the type changes mid-edit', () => { + setSearch('type=openai'); + const { rerender } = render(); + + expandAgent(); + fireEvent.change(temperatureInput(), { target: { value: '1.5' } }); + + setSearch('type=anthropic'); + rerender(); + expandAgent(); + + expect(temperatureInput().value).toBe('0.2'); + }); + it('preserves an in-flight edit across a background refetch', () => { state.providerId = 'edit-1'; const { rerender } = render(); diff --git a/frontend/src/pages/settings/settings-provider.tsx b/frontend/src/pages/settings/settings-provider.tsx index 85f04b48..856949ef 100644 --- a/frontend/src/pages/settings/settings-provider.tsx +++ b/frontend/src/pages/settings/settings-provider.tsx @@ -12,7 +12,7 @@ import { Trash2, XCircle, } from 'lucide-react'; -import { type ComponentProps, useEffect, useMemo, useState } from 'react'; +import { type ComponentProps, useEffect, useMemo, useRef, useState } from 'react'; import { type Control, type FieldPath, @@ -1213,6 +1213,7 @@ function SettingsProvider() { const { control, formState, handleSubmit: handleFormSubmit, reset, setValue, trigger, watch } = form; const { isDirty } = useFormState({ control }); + const seededTypeRef = useRef(null); useEffect(() => { if (submitError) { @@ -1283,6 +1284,17 @@ function SettingsProvider() { return; } + // setValue is outside the form's keepDirtyValues, so a background refetch re-runs this effect + // with a fresh `data` identity and overwrites agent edits the user has not saved. Re-seed only + // when the type actually changed — that is the case where the previous type's agents are wrong. + const isSameType = seededTypeRef.current === selectedType; + + seededTypeRef.current = selectedType; + + if (isSameType && form.getFieldState('agents').isDirty) { + return; + } + const defaultProvider = data.settingsProviders.default[selectedType as keyof typeof data.settingsProviders.default];