fix(desktop): stop Settings Workspace from sticking on skeleton under Strict Mode

apps/desktop/src/app/hooks/use-config-record.ts

@@ -1,7 +1,7 @@
 import { useQuery } from '@tanstack/react-query'
 
 import { getHermesConfigRecord } from '@/hermes'
-import { queryClient, writeCache } from '@/lib/query-client'
+import { HERMES_CONFIG_QUERY_KEY, queryClient, writeCache } from '@/lib/query-client'
 import { normalizeProfileKey } from '@/store/profile'
 import type { HermesConfigRecord } from '@/types/hermes'
 
@@ -10,9 +10,13 @@ import type { HermesConfigRecord } from '@/types/hermes'
 // so a save in one shows in the others, and revisiting a tab paints the cache
 // instead of blanking on a fresh fetch.
 //
+// The key itself lives in lib/query-client.ts: on a profile/gateway switch the
+// store-level boundary (invalidateProfileScopedQueries) hard-resets this record
+// so no consumer can seed a draft from the previous profile's data.
+//
 // Distinct from session/hooks/use-hermes-config.ts, which is side-effecting —
 // it pushes personality/cwd/voice/… into the session stores for live chat.
-export const HERMES_CONFIG_KEY = ['hermes-config-record'] as const
+export const HERMES_CONFIG_KEY = HERMES_CONFIG_QUERY_KEY
 
 // Per-profile cache key. The base key (no profile suffix) is the app-wide
 // active profile, unchanged for every caller that passes nothing. An explicit

apps/desktop/src/app/hooks/use-on-profile-switch.test.tsx

@@ -0,0 +1,69 @@
+import { act, renderHook } from '@testing-library/react'
+import { StrictMode } from 'react'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+
+import { $activeGatewayProfile } from '@/store/profile'
+
+import { useOnProfileSwitch } from './use-on-profile-switch'
+
+afterEach(() => {
+  $activeGatewayProfile.set('default')
+})
+
+describe('useOnProfileSwitch', () => {
+  it('does not fire on mount, including under StrictMode double-invoke', () => {
+    const onSwitch = vi.fn()
+
+    renderHook(() => useOnProfileSwitch(onSwitch), {
+      wrapper: StrictMode
+    })
+
+    expect(onSwitch).not.toHaveBeenCalled()
+  })
+
+  it('fires when the active gateway profile actually changes', () => {
+    const onSwitch = vi.fn()
+
+    renderHook(() => useOnProfileSwitch(onSwitch), {
+      wrapper: StrictMode
+    })
+
+    act(() => {
+      $activeGatewayProfile.set('coder')
+    })
+
+    expect(onSwitch).toHaveBeenCalledTimes(1)
+  })
+
+  it('does not fire when the profile atom is set to the same value', () => {
+    const onSwitch = vi.fn()
+
+    renderHook(() => useOnProfileSwitch(onSwitch), {
+      wrapper: StrictMode
+    })
+
+    act(() => {
+      $activeGatewayProfile.set('default')
+    })
+
+    expect(onSwitch).not.toHaveBeenCalled()
+  })
+
+  it('does not fire when the raw value changes but the normalized key does not', () => {
+    const onSwitch = vi.fn()
+
+    renderHook(() => useOnProfileSwitch(onSwitch), {
+      wrapper: StrictMode
+    })
+
+    // '' and ' default ' both normalize to 'default' — not a real switch.
+    act(() => {
+      $activeGatewayProfile.set('')
+    })
+    act(() => {
+      $activeGatewayProfile.set(' default ')
+    })
+
+    expect(onSwitch).not.toHaveBeenCalled()
+  })
+})

apps/desktop/src/app/hooks/use-on-profile-switch.ts

@@ -1,25 +1,31 @@
 import { useStore } from '@nanostores/react'
 import { useEffect, useRef } from 'react'
 
-import { $activeGatewayProfile } from '@/store/profile'
+import { $activeGatewayProfile, normalizeProfileKey } from '@/store/profile'
 
 /** Run `onSwitch` when the active gateway profile changes — never on first
  *  mount. For dropping per-profile view state (probes, cached usage, drafts)
- *  when the backend the app talks to swaps underneath a still-mounted view. */
+ *  when the backend the app talks to swaps underneath a still-mounted view.
+ *
+ *  Guarded by comparing the last seen profile key, not by a one-shot "skip
+ *  first run" flag: React Strict Mode re-invokes effects once after mount, and
+ *  a first-flag treats that second pass as a real switch (it wiped the
+ *  settings draft and left the page on its skeleton forever). Keys are
+ *  normalized with the same rule as the store-level cache invalidation in
+ *  store/profile.ts, so both fire under identical conditions. */
 export function useOnProfileSwitch(onSwitch: () => void): void {
-  const profile = useStore($activeGatewayProfile)
-  const first = useRef(true)
+  const profileKey = normalizeProfileKey(useStore($activeGatewayProfile))
+  const prevProfileKey = useRef(profileKey)
 
   // eslint-disable-next-line no-restricted-syntax -- legitimate non-atom ref write (see eslint rule comment)
   useEffect(() => {
-    if (first.current) {
-      first.current = false
-
+    if (prevProfileKey.current === profileKey) {
       return
     }
 
+    prevProfileKey.current = profileKey
     onSwitch()
     // Fire on profile change only; onSwitch identity is intentionally ignored.
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [profile])
+  }, [profileKey])
 }

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

@@ -0,0 +1,142 @@
+import { QueryClientProvider } from '@tanstack/react-query'
+import { act, cleanup, render, screen, waitFor } from '@testing-library/react'
+import { StrictMode } from 'react'
+import { MemoryRouter } from 'react-router'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { queryClient } from '@/lib/query-client'
+import { $activeGatewayProfile } from '@/store/profile'
+import type { HermesConfigRecord } from '@/types/hermes'
+
+import { HERMES_CONFIG_KEY } from '../hooks/use-config-record'
+
+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) => saveHermesConfig(config),
+  getElevenLabsVoices: () => getElevenLabsVoices(),
+  getProfiles: async () => ({ profiles: [] }),
+  setApiRequestProfile: () => {},
+  STARTUP_REQUEST_TIMEOUT_MS: 1000
+}))
+
+vi.mock('@/store/projects', () => ({
+  repoDiscoveryPolicyFromConfig: (config: HermesConfigRecord) => config?.desktop ?? {},
+  repoDiscoveryPolicySignature: (policy: unknown) => JSON.stringify(policy ?? null),
+  scanAndRecordRepos: vi.fn()
+}))
+
+// Heavy neighbours that aren't under test.
+vi.mock('./model-settings', () => ({
+  ModelSettings: () => null,
+  ModelSettingsSkeleton: () => null
+}))
+vi.mock('./memory/connect', () => ({ MemoryConnect: () => null }))
+vi.mock('./memory/provider-config-panel', () => ({ ProviderConfigPanel: () => null }))
+vi.mock('./quick-entry-settings', () => ({ QuickEntrySettings: () => null }))
+
+const workspaceConfig = (cwd: string): HermesConfigRecord => ({ terminal: { cwd } })
+
+const SCHEMA = {
+  fields: {
+    'terminal.cwd': { type: 'string', description: 'Default project folder.' }
+  }
+}
+
+beforeEach(() => {
+  getHermesConfigRecord.mockImplementation(async () => workspaceConfig('.'))
+  getHermesConfigSchema.mockResolvedValue(structuredClone(SCHEMA))
+  saveHermesConfig.mockResolvedValue({ ok: true })
+  getElevenLabsVoices.mockResolvedValue({ available: false, voices: [] })
+})
+
+afterEach(() => {
+  cleanup()
+  queryClient.clear()
+  $activeGatewayProfile.set('default')
+  vi.clearAllMocks()
+})
+
+async function renderWorkspaceSettings() {
+  const { ConfigSettings } = await import('./config-settings')
+
+  return render(
+    // StrictMode is load-bearing: the app runs under it, and the regression
+    // this file guards (profile-switch hook double-fire wiping the draft)
+    // only reproduces with Strict Mode's second effect pass.
+    <StrictMode>
+      <MemoryRouter>
+        <QueryClientProvider client={queryClient}>
+          <ConfigSettings activeSectionId="workspace" importInputRef={{ current: null }} />
+        </QueryClientProvider>
+      </MemoryRouter>
+    </StrictMode>
+  )
+}
+
+describe('ConfigSettings draft seeding', () => {
+  it('keeps the seeded draft under StrictMode when the record is already cached', async () => {
+    // Warm cache = the live repro: another surface fetched the config before
+    // Settings opened, so the seed happens on mount and the (old) first-flag
+    // profile-switch hook wiped it on Strict Mode's second effect pass —
+    // permanent skeleton, because the refetch structural-shares the same
+    // reference and the seed effect never re-ran.
+    queryClient.setQueryData(HERMES_CONFIG_KEY, workspaceConfig('.'))
+
+    await renderWorkspaceSettings()
+
+    await screen.findByText('Working Directory')
+
+    // Let the staleTime-0 background refetch (deep-equal payload) settle.
+    await waitFor(() => expect(getHermesConfigRecord).toHaveBeenCalled())
+    await act(async () => {
+      await new Promise(resolve => setTimeout(resolve, 25))
+    })
+
+    expect(screen.queryByText('Working Directory')).not.toBeNull()
+    expect(screen.getByDisplayValue('.')).toBeTruthy()
+  })
+
+  it('re-seeds the other profile record after a profile switch, without autosaving', async () => {
+    await renderWorkspaceSettings()
+    await screen.findByText('Working Directory')
+    expect(screen.getByDisplayValue('.')).toBeTruthy()
+
+    getHermesConfigRecord.mockImplementation(async () => workspaceConfig('/profile-b'))
+
+    act(() => {
+      $activeGatewayProfile.set('coder')
+    })
+
+    // Draft cleared + record hard-reset → refetch lands profile B's config and
+    // the empty draft re-seeds from it.
+    await screen.findByDisplayValue('/profile-b')
+
+    // The switch itself must never write config (that would cross-contaminate).
+    expect(saveHermesConfig).not.toHaveBeenCalled()
+  })
+
+  it('re-seeds after a profile switch even when both profiles have deep-equal configs', async () => {
+    await renderWorkspaceSettings()
+    await screen.findByText('Working Directory')
+
+    const callsBeforeSwitch = getHermesConfigRecord.mock.calls.length
+
+    // Profile B's record has identical content. Without a hard cache reset,
+    // React Query structural sharing keeps the SAME object reference across
+    // the refetch, a reference-keyed seed effect never re-runs, and the page
+    // is a skeleton forever. The state-derived seed + resetQueries must
+    // recover regardless of reference identity.
+    act(() => {
+      $activeGatewayProfile.set('coder')
+    })
+
+    await waitFor(() => expect(getHermesConfigRecord.mock.calls.length).toBeGreaterThan(callsBeforeSwitch))
+    await waitFor(() => expect(screen.getByDisplayValue('.')).toBeTruthy())
+  })
+})

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

@@ -101,25 +101,27 @@ export function ConfigSettings({
   const savedDiscoverySignatureRef = useRef<string | undefined>(undefined)
   const [saveVersion, setSaveVersion] = useState(0)
 
-  // 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)
-
+  // Seed the local draft whenever it is empty and the shared record is
+  // available. The guard is the draft state itself (not a one-shot ref), so any
+  // path that clears the draft re-seeds automatically once data lands — there
+  // is no "cleared but never re-seeded" state to get stuck in. Background
+  // refetches while an edit is in progress still can't clobber the draft,
+  // because a non-null draft blocks the seed.
   // eslint-disable-next-line no-restricted-syntax -- legitimate non-atom ref write (see eslint rule comment)
   useEffect(() => {
-    if (loadedConfig && !configSeeded.current) {
-      configSeeded.current = true
+    if (loadedConfig && config === null) {
       savedDiscoverySignatureRef.current = repoDiscoveryPolicySignature(repoDiscoveryPolicyFromConfig(loadedConfig))
       setConfig(loadedConfig)
     }
-  }, [loadedConfig])
-
-  // A profile switch invalidates (but doesn't clear) the shared config query, so
-  // the local draft would otherwise keep profile A's data and autosave it into
-  // B. Drop the seed + draft (re-seeds from B's refetch) and zero saveVersion so
-  // the pending debounced autosave is cancelled by its effect cleanup.
+  }, [loadedConfig, config])
+
+  // A profile switch swaps the backend under the mounted panel. Drop the draft
+  // and zero saveVersion so the pending debounced autosave is cancelled by its
+  // effect cleanup — profile A's draft must never be saved into profile B. The
+  // shared record itself is hard-reset centrally at the switch boundary
+  // (invalidateProfileScopedQueries), so the seed effect re-seeds from B's
+  // fresh fetch rather than A's cached record.
   useOnProfileSwitch(() => {
-    configSeeded.current = false
     savedDiscoverySignatureRef.current = undefined
     setConfig(null)
     saveVersionRef.current = 0
@@ -165,11 +167,14 @@ export function ConfigSettings({
             throw new Error(c.autosaveFailed)
           }
 
-          // Mirror the saved record into the shared cache so MCP/model surfaces
-          // reflect the edit without their own refetch.
-          setHermesConfigCache(config)
-
           if (saveVersionRef.current === v) {
+            // Mirror the saved record into the shared cache so MCP/model
+            // surfaces reflect the edit without their own refetch. Inside the
+            // version guard: a profile switch zeroes saveVersion while this
+            // save is in flight, and mirroring then would write profile A's
+            // record over profile B's freshly-reset cache.
+            setHermesConfigCache(config)
+
             const discoverySignature = repoDiscoveryPolicySignature(repoDiscoveryPolicyFromConfig(config))
 
             if (savedDiscoverySignatureRef.current !== discoverySignature) {

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

@@ -522,14 +522,21 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
         return
       }
 
+      const epoch = profileEpoch.current
       const prev = config
       const next = setNested(config, key, value)
       setConfig(next)
 
       try {
         await saveHermesConfig(next)
       } catch (err) {
-        setConfig(prev)
+        // Roll back only within the same profile epoch: after a switch the
+        // shared cache holds (or is fetching) profile B's record, and writing
+        // A's `prev` back would stomp it.
+        if (profileEpoch.current === epoch) {
+          setConfig(prev)
+        }
+
         notifyError(err, m.defaultsFailed)
       }
     },