fix(desktop): stop Settings autosave from clobbering out-of-band config edits

apps/desktop/src/app/settings/config-settings.test.tsx

@@ -0,0 +1,97 @@
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
+import { cleanup, render, screen, waitFor } from '@testing-library/react'
+import { atom } from 'nanostores'
+import { createRef } from 'react'
+import { MemoryRouter } from 'react-router'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+const getHermesConfigRecord = vi.fn()
+const getHermesConfigSchema = vi.fn()
+const saveHermesConfig = vi.fn()
+const getElevenLabsVoices = vi.fn()
+
+vi.mock('@/hermes', () => ({
+  getHermesConfigRecord: () => getHermesConfigRecord(),
+  getHermesConfigSchema: () => getHermesConfigSchema(),
+  saveHermesConfig: (config: unknown, profile?: string) => saveHermesConfig(config, profile),
+  getElevenLabsVoices: () => getElevenLabsVoices(),
+  setApiRequestProfile: () => {}
+}))
+
+vi.mock('../hooks/use-on-profile-switch', () => ({
+  useOnProfileSwitch: () => {}
+}))
+
+// The real stores pull in the gateway/profile stack, which needs a live
+// backend connection. This page only reads the "applies to" scope override
+// and the repo-discovery signature, neither of which this test touches.
+vi.mock('@/store/settings-scope', () => ({
+  $settingsScopeOverride: atom<null | string>(null)
+}))
+
+vi.mock('@/store/projects', () => ({
+  repoDiscoveryPolicyFromConfig: () => ({ enabled: true, roots: [], exclude_paths: [] }),
+  repoDiscoveryPolicySignature: (policy: unknown) => JSON.stringify(policy),
+  scanAndRecordRepos: vi.fn().mockResolvedValue(undefined)
+}))
+
+beforeEach(() => {
+  getElevenLabsVoices.mockResolvedValue({ available: false })
+  getHermesConfigSchema.mockResolvedValue({ fields: {} })
+  saveHermesConfig.mockResolvedValue({ ok: true })
+})
+
+afterEach(() => {
+  cleanup()
+  vi.clearAllMocks()
+})
+
+async function renderConfigSettings() {
+  const { ConfigSettings } = await import('./config-settings')
+  const client = new QueryClient({ defaultOptions: { queries: { retry: false } } })
+  const importInputRef = createRef<HTMLInputElement>()
+
+  render(
+    <MemoryRouter>
+      <QueryClientProvider client={client}>
+        <ConfigSettings activeSectionId="safety" importInputRef={importInputRef} />
+      </QueryClientProvider>
+    </MemoryRouter>
+  )
+
+  return { importInputRef }
+}
+
+describe('ConfigSettings autosave', () => {
+  it('sends a later revert instead of diffing it away against the stale page-load baseline', async () => {
+    getHermesConfigRecord.mockResolvedValue({ checkpoints: { enabled: false }, other: 'untouched' })
+
+    vi.useFakeTimers({ shouldAdvanceTime: true })
+
+    try {
+      await renderConfigSettings()
+
+      const toggle = await screen.findByRole('switch')
+
+      // Edit: flip checkpoints.enabled on, let the debounced autosave fire.
+      toggle.click()
+      await vi.advanceTimersByTimeAsync(700)
+
+      await waitFor(() => expect(saveHermesConfig).toHaveBeenCalledTimes(1))
+      expect(saveHermesConfig.mock.calls[0][0]).toEqual({ checkpoints: { enabled: true } })
+
+      // Revert: flip it back to its original value and let autosave fire again.
+      toggle.click()
+      await vi.advanceTimersByTimeAsync(700)
+
+      await waitFor(() => expect(saveHermesConfig).toHaveBeenCalledTimes(2))
+      // Must still explicitly send the reverted value — diffing against the
+      // never-advanced page-load baseline would produce an empty patch here
+      // (the field is back to its original value) and leave disk stuck at
+      // `enabled: true` from the first save.
+      expect(saveHermesConfig.mock.calls[1][0]).toEqual({ checkpoints: { enabled: false } })
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+})

apps/desktop/src/app/settings/config-settings.tsx

@@ -33,6 +33,7 @@ import { PanelEmpty } from '../overlays/panel'
 import { ConfigField } from './config-field'
 import {
   clearsEnabledToolsets,
+  diffConfig,
   enumOptionsFor,
   getNested,
   isExternalMemoryProvider,
@@ -121,11 +122,21 @@ function ConfigSettingsInner({
   // Seed the local draft once, the first time the shared record lands.
   // Background refetches thereafter must not clobber in-progress edits.
   const configSeeded = useRef(false)
+  // Snapshot of the record as it was when the draft was seeded. Autosave
+  // diffs the draft against this (not against disk) so a field the user
+  // never touched — possibly changed out-of-band by `hermes config set`
+  // while this page sat open — is never resent with its stale value.
+  const configBaselineRef = useRef<HermesConfigRecord | null>(null)
+  // Serializes autosave requests so an older save that's still in flight can't
+  // resolve after a newer one and re-advance the baseline / cache with stale
+  // data — each save's diff+request only starts once the previous one lands.
+  const saveQueueRef = useRef<Promise<void>>(Promise.resolve())
 
   // eslint-disable-next-line no-restricted-syntax -- legitimate non-atom ref write (see eslint rule comment)
   useEffect(() => {
     if (loadedConfig && !configSeeded.current) {
       configSeeded.current = true
+      configBaselineRef.current = loadedConfig
       savedDiscoverySignatureRef.current = repoDiscoveryPolicySignature(repoDiscoveryPolicyFromConfig(loadedConfig))
       setConfig(loadedConfig)
     }
@@ -137,10 +148,12 @@ function ConfigSettingsInner({
   // the pending debounced autosave is cancelled by its effect cleanup.
   useOnProfileSwitch(() => {
     configSeeded.current = false
+    configBaselineRef.current = null
     savedDiscoverySignatureRef.current = undefined
     setConfig(null)
     saveVersionRef.current = 0
     setSaveVersion(0)
+    saveQueueRef.current = Promise.resolve()
   })
 
   useEffect(() => {
@@ -173,25 +186,37 @@ function ConfigSettingsInner({
     }
 
     const v = saveVersion
+    const snapshot = config
 
     const t = window.setTimeout(() => {
-      void (async () => {
+      // Chained onto the queue (not fired directly) so an older save that's
+      // still awaiting its response can't land after this one and undo its
+      // baseline advance — each save's diff is computed once its predecessor
+      // has fully resolved.
+      saveQueueRef.current = saveQueueRef.current.then(async () => {
         try {
-          const result = await saveHermesConfig(config, scopeProfile ?? undefined)
+          const patch = diffConfig(configBaselineRef.current ?? {}, snapshot)
+          const result = await saveHermesConfig(patch, scopeProfile ?? undefined)
 
           if (!result.ok) {
             throw new Error(c.autosaveFailed)
           }
 
+          // The saved snapshot becomes the new baseline, so the next autosave
+          // diffs against what's actually on disk instead of the page-load
+          // (or last-baseline) copy — otherwise reverting a field to its
+          // pre-save value diffs to nothing and the revert never reaches disk.
+          configBaselineRef.current = snapshot
+
           // Mirror the saved record into the shared cache so MCP/model surfaces
           // reflect the edit without their own refetch.
-          writeConfigCache(config)
+          writeConfigCache(snapshot)
 
           if (saveVersionRef.current === v) {
             // The repo-discovery scan reads the ACTIVE profile's workspace
             // policy; skip it when this page is editing another profile.
             if (scopeProfile == null) {
-              const discoverySignature = repoDiscoveryPolicySignature(repoDiscoveryPolicyFromConfig(config))
+              const discoverySignature = repoDiscoveryPolicySignature(repoDiscoveryPolicyFromConfig(snapshot))
 
               if (savedDiscoverySignatureRef.current !== discoverySignature) {
                 savedDiscoverySignatureRef.current = discoverySignature
@@ -206,7 +231,7 @@ function ConfigSettingsInner({
             notifyError(err, c.autosaveFailed)
           }
         }
-      })()
+      })
     }, 550)
 
     return () => window.clearTimeout(t)

apps/desktop/src/app/settings/helpers.test.ts

@@ -6,6 +6,7 @@ import { FIELD_DESCRIPTIONS, FIELD_LABELS, SECTIONS } from './constants'
 import { defineFieldCopy, fieldCopyForSchemaKey, schemaKeyToFieldCopyKey } from './field-copy'
 import {
   clearsEnabledToolsets,
+  diffConfig,
   enumOptionsFor,
   getNested,
   isExternalMemoryProvider,
@@ -404,4 +405,44 @@ describe('settings helpers', () => {
       expect(clearsEnabledToolsets(prev, next)).toBe(false)
     })
   })
+
+  describe('diffConfig', () => {
+    it('omits a top-level key the draft never touched', () => {
+      // The autosave baseline is a snapshot taken when Settings opened. A key
+      // an agent set via `hermes config set` while the page sat open must not
+      // come back in the patch just because it's still present in the draft.
+      const baseline: HermesConfigRecord = { fallback_providers: ['nara1'], timezone: 'UTC' }
+      const draft: HermesConfigRecord = { fallback_providers: ['nara1'], timezone: 'America/New_York' }
+
+      expect(diffConfig(baseline, draft)).toEqual({ timezone: 'America/New_York' })
+    })
+
+    it('includes a nested key only when it actually changed, leaving siblings out', () => {
+      const baseline: HermesConfigRecord = { display: { personality: 'default', show_reasoning: true } }
+      const draft: HermesConfigRecord = { display: { personality: 'default', show_reasoning: false } }
+
+      expect(diffConfig(baseline, draft)).toEqual({ display: { show_reasoning: false } })
+    })
+
+    it('sends a new key that was absent from the baseline', () => {
+      const baseline: HermesConfigRecord = {}
+      const draft: HermesConfigRecord = { timezone: 'UTC' }
+
+      expect(diffConfig(baseline, draft)).toEqual({ timezone: 'UTC' })
+    })
+
+    it('returns an empty object when the draft matches the baseline exactly', () => {
+      const baseline: HermesConfigRecord = { toolsets: ['memory'], display: { personality: 'default' } }
+      const draft: HermesConfigRecord = { toolsets: ['memory'], display: { personality: 'default' } }
+
+      expect(diffConfig(baseline, draft)).toEqual({})
+    })
+
+    it('treats an array as a whole value, not diffed element by element', () => {
+      const baseline: HermesConfigRecord = { toolsets: ['memory', 'terminal'] }
+      const draft: HermesConfigRecord = { toolsets: ['memory'] }
+
+      expect(diffConfig(baseline, draft)).toEqual({ toolsets: ['memory'] })
+    })
+  })
 })

apps/desktop/src/app/settings/helpers.ts

@@ -97,6 +97,43 @@ export function getNested(obj: HermesConfigRecord, path: string): unknown {
   return cur
 }
 
+const isPlainObject = (value: unknown): value is Record<string, unknown> =>
+  typeof value === 'object' && value !== null && !Array.isArray(value)
+
+/**
+ * Structural diff between two config snapshots: an object holding only the
+ * branches of `next` that changed relative to `base`. Plain-object values are
+ * compared key by key so editing one field doesn't drag its untouched
+ * siblings back into the result; arrays and scalars are compared as whole
+ * values.
+ *
+ * The autosave path sends this instead of the full draft so a field the user
+ * never touched — one an agent may have changed via `hermes config set`
+ * while Settings was open with a stale snapshot — is never resent with its
+ * now-stale value. `PUT /api/config` deep-merges onto disk, so an omitted
+ * key keeps whatever is currently there.
+ */
+export function diffConfig(base: HermesConfigRecord, next: HermesConfigRecord): HermesConfigRecord {
+  const patch: HermesConfigRecord = {}
+
+  for (const key of Object.keys(next)) {
+    const baseValue = base[key]
+    const nextValue = next[key]
+
+    if (isPlainObject(baseValue) && isPlainObject(nextValue)) {
+      const nested = diffConfig(baseValue, nextValue)
+
+      if (Object.keys(nested).length > 0) {
+        patch[key] = nested
+      }
+    } else if (JSON.stringify(baseValue) !== JSON.stringify(nextValue)) {
+      patch[key] = nextValue
+    }
+  }
+
+  return patch
+}
+
 /**
  * True when an edit clears the entire "Enabled Toolsets" list — i.e. the
  * previous config had a non-empty toolsets array and the next one is an