From 55babca7832868607f94b599c28ecee99883b321 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 20:47:09 -0700 Subject: [PATCH] fix(desktop,dashboard): single-key config writers send a sparse patch, not the cached snapshot Applying a reasoning/speed default in Desktop Settings -> Model reset an auxiliary slot a user had pinned via CLI back to provider "auto" / model "" while leaving reasoning_effort intact (#95460). POST /api/model/set was never the writer; writeAgentDefault was: it round-tripped the whole default-expanded config record (loaded when Settings opened) through PUT /api/config, so every key another surface changed since the snapshot was echoed back with its stale, default-filled value. The pinned slot's provider/model existed only as defaults in the snapshot; reasoning_effort was already in it, hence the asymmetry the report observed. PUT /api/config deep-merges onto disk, so a writer only needs to send the key it changed. Every desktop single-key writer now does exactly that (the config-settings page already diffed against a baseline): Model defaults (agent.reasoning_effort / service_tier), Appearance resume_last_session, terminal font, session auto-archive, the two browser.use_real_profile toggles, and the Capabilities voice fields (diffConfig against a baseline). The optimistic shared-cache write keeps the full merged record so sibling surfaces repaint without a refetch. The dashboard's ReasoningPicker had the same read-modify-write shape and now sends the sparse patch too. Tests pin the wire contract: only the edited key is sent, a sibling pin that is not in the snapshot cannot be echoed back. --- .../real-profile-consent-dialog.test.tsx | 19 ++++++++----------- .../real-profile-consent-dialog.tsx | 4 +++- .../src/app/settings/appearance-settings.tsx | 4 +++- .../browser-real-profile-panel.test.tsx | 17 +++++++---------- .../settings/browser-real-profile-panel.tsx | 4 +++- .../src/app/settings/model-settings.test.tsx | 15 +++++++++------ .../src/app/settings/model-settings.tsx | 9 ++++++--- .../src/app/settings/sessions-settings.tsx | 4 +++- .../settings/terminal-font-setting.test.tsx | 14 +++++++------- .../app/settings/terminal-font-setting.tsx | 4 +++- .../app/settings/voice-provider-fields.tsx | 15 ++++++++++++--- web/src/components/ReasoningPicker.tsx | 18 +++++------------- 12 files changed, 69 insertions(+), 58 deletions(-) diff --git a/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.test.tsx b/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.test.tsx index 9ad67c549c..4f06db5b0b 100644 --- a/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.test.tsx +++ b/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.test.tsx @@ -84,17 +84,14 @@ describe('RealProfileConsentDialog', () => { fireEvent.click(screen.getByRole('button', { name: promptCopy.enable })) }) - // Saves the WHOLE merged record with only use_real_profile added — the - // same shape the Capabilities toggle writes, through the same cache, so - // the existing toggle flips on without a refetch. - expect(mocks.save).toHaveBeenCalledWith( - { - browser: { allow_private_urls: false, use_real_profile: true }, - model: { provider: 'nous' } - }, - undefined - ) - expect(mocks.cache).toHaveBeenCalledWith(mocks.save.mock.calls[0][0]) + // Saves ONLY the toggled key (PUT deep-merges) — the same shape the + // Capabilities toggle writes — while the shared cache gets the merged + // record so the existing toggle flips on without a refetch. + expect(mocks.save).toHaveBeenCalledWith({ browser: { use_real_profile: true } }, undefined) + expect(mocks.cache).toHaveBeenCalledWith({ + browser: { allow_private_urls: false, use_real_profile: true }, + model: { provider: 'nous' } + }) expect(mocks.notify).toHaveBeenCalled() }) diff --git a/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.tsx b/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.tsx index e8bbe1f469..81b4413a75 100644 --- a/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.tsx +++ b/apps/desktop/src/app/chat/right-rail/real-profile-consent-dialog.tsx @@ -78,7 +78,9 @@ export function RealProfileConsentDialog({ tabId }: RealProfileConsentDialogProp setConfig(next) try { - await saveHermesConfigRecord(next) + // Sparse patch: PUT /api/config deep-merges, and echoing the cached + // snapshot would overwrite keys other surfaces changed since it loaded. + await saveHermesConfigRecord({ browser: { use_real_profile: true } }) notify({ kind: 'info', title: copy.enabledTitle, message: copy.enabledMessage }) } catch (err) { setConfig(config) diff --git a/apps/desktop/src/app/settings/appearance-settings.tsx b/apps/desktop/src/app/settings/appearance-settings.tsx index 17c6bc59aa..aefcf3e249 100644 --- a/apps/desktop/src/app/settings/appearance-settings.tsx +++ b/apps/desktop/src/app/settings/appearance-settings.tsx @@ -89,7 +89,9 @@ function ResumeLastSessionSetting() { const next = setNested(config, 'display.resume_last_session', on) setHermesConfigCache(next) - void saveHermesConfig(next) + // Sparse patch: PUT /api/config deep-merges, and echoing the cached + // snapshot would overwrite keys other surfaces changed since it loaded. + void saveHermesConfig(setNested({}, 'display.resume_last_session', on)) .then(result => { if (!result.ok) { throw new Error(t.settings.config.autosaveFailed) diff --git a/apps/desktop/src/app/settings/browser-real-profile-panel.test.tsx b/apps/desktop/src/app/settings/browser-real-profile-panel.test.tsx index 703b96e501..22ef2d3ac9 100644 --- a/apps/desktop/src/app/settings/browser-real-profile-panel.test.tsx +++ b/apps/desktop/src/app/settings/browser-real-profile-panel.test.tsx @@ -67,16 +67,13 @@ describe('BrowserRealProfilePanel', () => { fireEvent.click(toggle) }) - // Saves the WHOLE merged record with only use_real_profile added — sibling - // browser keys survive. - expect(mocks.save).toHaveBeenCalledWith( - { - browser: { allow_private_urls: false, use_real_profile: true }, - model: { provider: 'nous' } - }, - undefined - ) - expect(mocks.cache).toHaveBeenCalledWith(mocks.save.mock.calls[0][0]) + // Saves ONLY the toggled key (PUT deep-merges); the cache keeps the whole + // merged record so sibling browser keys survive without a refetch. + expect(mocks.save).toHaveBeenCalledWith({ browser: { use_real_profile: true } }, undefined) + expect(mocks.cache).toHaveBeenCalledWith({ + browser: { allow_private_urls: false, use_real_profile: true }, + model: { provider: 'nous' } + }) expect(mocks.notify).toHaveBeenCalled() }) diff --git a/apps/desktop/src/app/settings/browser-real-profile-panel.tsx b/apps/desktop/src/app/settings/browser-real-profile-panel.tsx index e7ada72977..602244a06c 100644 --- a/apps/desktop/src/app/settings/browser-real-profile-panel.tsx +++ b/apps/desktop/src/app/settings/browser-real-profile-panel.tsx @@ -65,7 +65,9 @@ export function BrowserRealProfilePanel({ profile }: BrowserRealProfilePanelProp setConfig(next) try { - await saveHermesConfigRecord(next, profile) + // Sparse patch: PUT /api/config deep-merges, and echoing the cached + // snapshot would overwrite keys other surfaces changed since it loaded. + await saveHermesConfigRecord({ browser: { use_real_profile: on } }, profile) notify({ kind: 'info', title: on ? copy.enabledTitle : copy.disabledTitle, diff --git a/apps/desktop/src/app/settings/model-settings.test.tsx b/apps/desktop/src/app/settings/model-settings.test.tsx index 2a07c323cd..457c8b0ac4 100644 --- a/apps/desktop/src/app/settings/model-settings.test.tsx +++ b/apps/desktop/src/app/settings/model-settings.test.tsx @@ -286,18 +286,21 @@ describe('ModelSettings', () => { ) }) - it('writes the profile default speed (service_tier) when the fast switch is toggled', async () => { + it('writes the profile default speed (service_tier) as a sparse patch, never the cached snapshot', async () => { + // The cached record is a default-expanded snapshot; a CLI pin made after it + // loaded is not in it. Echoing the whole record back would reset that + // auxiliary slot to auto/'' (#95460) — only the edited key may be sent. + getHermesConfigRecord.mockResolvedValue({ + agent: { reasoning_effort: 'medium', service_tier: 'normal' }, + auxiliary: { curator: { provider: 'auto', model: '', reasoning_effort: 'high' } } + }) await renderModelSettings() await waitFor(() => expect(getHermesConfigRecord).toHaveBeenCalled()) const fastSwitch = await screen.findByRole('switch') fireEvent.click(fastSwitch) - await waitFor(() => - expect(saveHermesConfig).toHaveBeenCalledWith( - expect.objectContaining({ agent: expect.objectContaining({ service_tier: 'fast' }) }) - ) - ) + await waitFor(() => expect(saveHermesConfig).toHaveBeenCalledWith({ agent: { service_tier: 'fast' } })) }) it('hides the reasoning/speed defaults when the main model reports no capabilities', async () => { diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index ece3d781b4..5f7ad82ebf 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -566,8 +566,11 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting const fastOn = isFastTier(getNested(config ?? {}, 'agent.service_tier')) - // Persist a single agent.* default by round-tripping the whole config record - // (PUT /api/config replaces it) — optimistic, with rollback on failure. + // Persist a single agent.* default as a sparse patch (PUT /api/config + // deep-merges onto disk). Never send the whole cached record: it is a + // default-expanded snapshot, and echoing it back rewrites every key another + // surface changed meanwhile — a CLI-pinned auxiliary slot came back as + // provider "auto" / model "" (#95460). Optimistic, with rollback on failure. const writeAgentDefault = useCallback( async (key: string, value: string) => { if (!config) { @@ -579,7 +582,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setConfig(next) try { - await saveHermesConfig(next, scopeProfile) + await saveHermesConfig(setNested({}, key, value), scopeProfile) } catch (err) { setConfig(prev) notifyError(err, m.defaultsFailed) diff --git a/apps/desktop/src/app/settings/sessions-settings.tsx b/apps/desktop/src/app/settings/sessions-settings.tsx index a194a6f0a6..d33e37b81f 100644 --- a/apps/desktop/src/app/settings/sessions-settings.tsx +++ b/apps/desktop/src/app/settings/sessions-settings.tsx @@ -239,7 +239,9 @@ function AutoArchiveSetting() { setConfig(updated) try { - await saveHermesConfig(updated) + // Sparse patch: PUT /api/config deep-merges, and echoing the cached + // snapshot would overwrite keys other surfaces changed since it loaded. + await saveHermesConfig({ sessions: { auto_archive: autoArchive, auto_archive_days: archiveDays } }) } catch (err) { notifyError(err, s.autoArchiveFailed) } diff --git a/apps/desktop/src/app/settings/terminal-font-setting.test.tsx b/apps/desktop/src/app/settings/terminal-font-setting.test.tsx index 3bf746526c..b3ba19733a 100644 --- a/apps/desktop/src/app/settings/terminal-font-setting.test.tsx +++ b/apps/desktop/src/app/settings/terminal-font-setting.test.tsx @@ -87,11 +87,13 @@ describe('TerminalFontSetting', () => { await flushAutosave() - expect(mocks.save).toHaveBeenCalledWith({ + // Only the font key goes over the wire (PUT deep-merges); the shared cache + // gets the merged record so sibling terminal keys survive. + expect(mocks.save).toHaveBeenCalledWith({ terminal: { font_family: 'MesloLGS NF' } }) + expect(mocks.cache).toHaveBeenCalledWith({ display: { skin: 'hermes' }, terminal: { backend: 'local', cwd: '/workspace', font_family: 'MesloLGS NF' } }) - expect(mocks.cache).toHaveBeenCalledWith(mocks.save.mock.calls[0][0]) }) it('accepts an arbitrary CSS stack and resets to the bundled default', async () => { @@ -105,8 +107,8 @@ describe('TerminalFontSetting', () => { fireEvent.change(input, { target: { value: "'Custom Powerline', monospace" } }) await flushAutosave() - expect(mocks.save.mock.calls[0][0]).toMatchObject({ - terminal: { backend: 'local', font_family: "'Custom Powerline', monospace" } + expect(mocks.save.mock.calls[0][0]).toEqual({ + terminal: { font_family: "'Custom Powerline', monospace" } }) fireEvent.click(screen.getByRole('button', { name: 'Use default' })) @@ -114,9 +116,7 @@ describe('TerminalFontSetting', () => { expect((screen.getByLabelText('Glyph preview') as HTMLElement).style.fontFamily).toContain('JetBrains Mono') await flushAutosave() - expect(mocks.save.mock.calls[1][0]).toMatchObject({ - terminal: { backend: 'local', font_family: '' } - }) + expect(mocks.save.mock.calls[1][0]).toEqual({ terminal: { font_family: '' } }) }) it('rolls back the optimistic font when autosave fails', async () => { diff --git a/apps/desktop/src/app/settings/terminal-font-setting.tsx b/apps/desktop/src/app/settings/terminal-font-setting.tsx index a1efc8e7d8..52b10bc28f 100644 --- a/apps/desktop/src/app/settings/terminal-font-setting.tsx +++ b/apps/desktop/src/app/settings/terminal-font-setting.tsx @@ -88,7 +88,9 @@ export function TerminalFontSetting() { const timeout = window.setTimeout(() => { const next = setNested(loadedConfig, 'terminal.font_family', value) - void saveHermesConfig(next) + // Sparse patch: PUT /api/config deep-merges, and echoing the cached + // snapshot would overwrite keys other surfaces changed since it loaded. + void saveHermesConfig(setNested({}, 'terminal.font_family', value)) .then(result => { if (!result.ok) { throw new Error(t.settings.config.autosaveFailed) diff --git a/apps/desktop/src/app/settings/voice-provider-fields.tsx b/apps/desktop/src/app/settings/voice-provider-fields.tsx index 62b422016a..e4805e97f3 100644 --- a/apps/desktop/src/app/settings/voice-provider-fields.tsx +++ b/apps/desktop/src/app/settings/voice-provider-fields.tsx @@ -10,7 +10,7 @@ import { setHermesConfigCache, useHermesConfigRecord } from '../hooks/use-config import { ConfigField } from './config-field' import { SECTIONS } from './constants' -import { enumOptionsFor, getNested, inferFieldSchema, setNested } from './helpers' +import { diffConfig, enumOptionsFor, getNested, inferFieldSchema, setNested } from './helpers' // The curated voice keys (Settings → Voice) are the single source of which // per-provider fields exist; both the Voice settings page and the @@ -45,12 +45,18 @@ export function VoiceProviderFields({ section, providerKey }: { section: 'tts' | // refetches must not clobber in-progress edits) — the same shape as // config-settings.tsx's autosave loop. const [config, setConfig] = useState(null) + // Autosave sends only what changed against this baseline (config-settings.tsx + // pattern): the seeded record is a default-expanded snapshot, and echoing it + // whole would overwrite keys other surfaces changed since it loaded. The + // baseline advances to each successfully saved draft. + const [baseline, setBaseline] = useState(null) const seeded = useRef(false) // eslint-disable-next-line no-restricted-syntax -- one-shot config seed flag, not an atom mirror useEffect(() => { if (loadedConfig && !seeded.current) { seeded.current = true + setBaseline(loadedConfig) setConfig(loadedConfig) } }, [loadedConfig]) @@ -64,8 +70,11 @@ export function VoiceProviderFields({ section, providerKey }: { section: 'tts' | } const timeout = window.setTimeout(() => { - void saveHermesConfig(config) - .then(() => setHermesConfigCache(config)) + void saveHermesConfig(diffConfig(baseline ?? {}, config)) + .then(() => { + setBaseline(config) + setHermesConfigCache(config) + }) .catch(err => notifyError(err, t.settings.config.autosaveFailed)) }, 550) diff --git a/web/src/components/ReasoningPicker.tsx b/web/src/components/ReasoningPicker.tsx index cd45986a76..7883f5886a 100644 --- a/web/src/components/ReasoningPicker.tsx +++ b/web/src/components/ReasoningPicker.tsx @@ -77,20 +77,12 @@ export function ReasoningPicker({ const prev = effort; setEffort(next); // optimistic setSaving(true); - // Read-modify-write the whole config — the dashboard's single-key save - // pattern — so we never clobber sibling keys. `saveConfig` PUTs the full - // object the agent boots from. + // Sparse patch: PUT /api/config deep-merges onto disk, so sending only + // the edited key never clobbers sibling keys — and never echoes a + // default-expanded snapshot back over values another surface changed + // meanwhile (a CLI-pinned auxiliary slot would come back as "auto"). void api - .getConfig(profile) - .then((cfg) => { - const base = (cfg ?? {}) as Record; - const agent = - base.agent && typeof base.agent === "object" - ? { ...(base.agent as Record) } - : {}; - agent.reasoning_effort = next; - return api.saveConfig({ ...base, agent }, profile); - }) + .saveConfig({ agent: { reasoning_effort: next } }, profile) .then(() => { onChanged?.(next); })