fix(desktop): key the vault panel by owner instead of syncing state in an effect

CI lint: the owner-change wipe was a useEffect that reset state from a derived value (no-restricted-syntax) — replace it with what the guide prescribes: the mount site keys <VaultSettings> by (connection, profile), so an owner change remounts the panel and drafts/dialogs are gone by construction. Owner test mirrors the keyed mount; curly-brace lint in the test fixture.
2026-09-09 05:43:16 -07:00
parent dedc99ec6c
commit de551dcd3a
4 changed files with 40 additions and 22 deletions
@@ -31,6 +31,7 @@ import { typeToFocusChar } from '@/lib/keybinds/composer-focus-keys'
 import { cn } from '@/lib/utils'
 import { $commandPaletteOpen, openCommandPalettePage } from '@/store/command-palette'
 import { confirm } from '@/store/confirm'
+import { $activeConnectionId } from '@/store/connections'
 import { bindingsFor } from '@/store/keybinds'
 import { $localModelsEnabled } from '@/store/local-models-flag'
 import { notifyError } from '@/store/notifications'
@@ -54,7 +55,7 @@ import { NotificationsSettings } from './notifications-settings'
 import { PROVIDER_VIEWS, ProvidersSettings, type ProviderView } from './providers-settings'
 import { SessionsSettings } from './sessions-settings'
 import type { SettingsPageProps, SettingsView as SettingsViewId } from './types'
-import { VaultSettings } from './vault-settings'
+import { vaultOwnerKey, VaultSettings } from './vault-settings'

 const SETTINGS_VIEWS: readonly SettingsViewId[] = [
  ...SECTIONS.map(s => `config:${s.id}` as SettingsViewId),
@@ -74,6 +75,7 @@ const SETTINGS_VIEWS: readonly SettingsViewId[] = [

 export function SettingsView({ onClose, onConfigSaved, onMainModelChanged }: SettingsPageProps) {
  const scopeProfile = useStore($settingsScopeProfile)
+  const activeConnectionId = useStore($activeConnectionId)
  const { t } = useI18n()
  const navigate = useNavigate()
  const { hash, pathname, search } = useLocation()
@@ -431,7 +433,7 @@ export function SettingsView({ onClose, onConfigSaved, onMainModelChanged }: Set
    ) : activeView === 'billing' ? (
      <BillingSettings />
    ) : activeView === 'vault' ? (
-      <VaultSettings />
+      <VaultSettings key={vaultOwnerKey(activeConnectionId, scopeProfile)} />
    ) : (
      <SessionsSettings />
    )
@@ -21,11 +21,14 @@ vi.mock('@/store/gateway', async importActual => ({
 vi.mock('@/lib/haptics', () => ({ triggerHaptic: vi.fn() }))
 vi.mock('@/store/notifications', () => ({ notify: vi.fn(), notifyError: vi.fn() }))

+import { useStore } from '@nanostores/react'
+
 import { queryClient } from '@/lib/query-client'
 import { $activeGatewayProfile } from '@/store/profile'
 import { $gatewayState } from '@/store/session'
+import { $settingsScopeProfile } from '@/store/settings-scope'

-import { VaultSettings } from './vault-settings'
+import { vaultOwnerKey, VaultSettings } from './vault-settings'

 stubResizeObserver()

@@ -33,11 +36,19 @@ const sources = [
  { name: 'bitwarden', display_name: 'Bitwarden', enabled: true, needs_unlock: true, unlocked: false, installed: true }
 ]

+// Mirrors the production mount site (settings/index.tsx): the panel is keyed by its owner, so an
+// owner change remounts it and every dialog/draft is gone by construction.
+function KeyedVault() {
+  const profile = useStore($settingsScopeProfile)
+
+  return <VaultSettings key={vaultOwnerKey(null, profile)} />
+}
+
 function mount() {
  return render(
    <MemoryRouter>
      <QueryClientProvider client={queryClient}>
-        <VaultSettings />
+        <KeyedVault />
      </QueryClientProvider>
    </MemoryRouter>
  )
@@ -72,11 +83,19 @@ it('a master-password draft is wiped on a profile switch and never submitted to
 it("a late list response from profile A never paints under profile B", async () => {
  let resolveA!: (value: unknown) => void
  const held = new Promise(r => (resolveA = r))
+
  respond = async (profile, method) => {
-    if (method === 'vault.sources') return { sources }
-    if (profile === 'default' && method === 'vault.list') return held
+    if (method === 'vault.sources') {
+      return { sources }
+    }
+
+    if (profile === 'default' && method === 'vault.list') {
+      return held
+    }
+
    return { items: [] }
  }
+
  mount()
  await waitFor(() => expect(calls.some(c => c.profile === 'default' && c.method === 'vault.list')).toBe(true))

@@ -94,9 +113,11 @@ it('vault.add secrets never enter the mutation cache', async () => {
  respond = async (_profile, method) => (method === 'vault.sources' ? { sources } : method === 'vault.list' ? { items: [] } : { id: 'created' })
  const view = mount()
  fireEvent.click(await screen.findByRole('button', { name: 'Add credential' }))
+
  for (const [label, value] of [['Label', 'fixture'], ['Site origin', 'https://example.com'], ['Identifier', 'fixture@example.com'], ['Password', 'fixture-retained-password']] as const) {
    fireEvent.change(screen.getByLabelText(label), { target: { value } })
  }
+
  fireEvent.click(screen.getByRole('button', { name: 'Save to vault' }))
  await waitFor(() => expect(calls.some(c => c.method === 'vault.add')).toBe(true))
  expect((calls.find(c => c.method === 'vault.add')!.params.secret as Record<string, string>).password).toBe('fixture-retained-password')
@@ -146,14 +146,17 @@ describe('VaultSettings', () => {
      { name: 'onepassword', display_name: '1Password', enabled: true, needs_unlock: true, unlocked: false, installed: true },
      { name: 'bitwarden', display_name: 'Bitwarden', enabled: false, needs_unlock: true, unlocked: false, installed: false }
    ]
+
    requestGateway.mockImplementation(async (method: string) => {
      if (method === 'vault.list') {
        // An external item has no delete affordance; its manager is shown as a source badge instead.
        return { items: [{ ...LOGIN_ITEM, id: 'op:xyz', label: 'GitHub via 1Password', backend: 'onepassword' }] }
      }
+
      if (method === 'vault.sources') {
        return { sources: sources.map(source => ({ ...source })) }
      }
+
      if (method === 'vault.unlock') {
        sources[0] = { ...sources[0], unlocked: true }

@@ -32,6 +32,7 @@ import { ListRow, Pill, SectionHeading, SettingsContent } from './primitives'

 // Vault data is private to one (connection, profile); the cache key carries that owner so a
 // late response from profile A can never paint under profile B.
+export const vaultOwnerKey = (connectionId: null | string, profile: string) => `${connectionId ?? ''}::${profile}`
 const vaultQueryKey = (owner: string) => ['vault-items', owner] as const
 const vaultSourcesQueryKey = (owner: string) => ['vault-sources', owner] as const

@@ -151,17 +152,20 @@ export function VaultSettings() {
  const v = t.settings.vault
  const gatewayState = useStore($gatewayState)
  const queryClient = useQueryClient()
-  // The owner this panel edits: pinned per render, and every RPC below goes through the owner's
-  // socket with an explicit profile — never the ambient foreground gateway. Changing owner
-  // (profile switch, connection swap) closes every dialog and wipes drafts (see the effect below).
+  // The owner this panel edits: every RPC below goes through the owner's socket with an explicit
+  // profile — never the ambient foreground gateway. The mount site keys the panel by this same
+  // owner, so a profile switch / connection swap remounts it: dialogs close and drafts (including a
+  // typed master password) are gone by construction rather than by cleanup code.
  const scopeProfile = useStore($settingsScopeProfile)
  const connectionId = useStore($activeConnectionId)
-  const owner = `${connectionId ?? ''}::${scopeProfile}`
+  const owner = vaultOwnerKey(connectionId, scopeProfile)
+
  const requestGateway = useCallback(
    <T,>(method: string, params: Record<string, unknown> = {}) =>
      requestGatewayForProfile<T>(scopeProfile, method, params),
    [scopeProfile]
  )
+
  const VAULT_QUERY_KEY = useMemo(() => vaultQueryKey(owner), [owner])
  const VAULT_SOURCES_QUERY_KEY = useMemo(() => vaultSourcesQueryKey(owner), [owner])
  const [searchParams, setSearchParams] = useSearchParams()
@@ -178,18 +182,6 @@ export function VaultSettings() {
  const pendingMasterPassword = useRef('')
  const pendingSecret = useRef<null | Record<string, string>>(null)

-  useEffect(() => {
-    // A draft typed for one owner must not be submitted to another.
-    setAddOpen(false)
-    setForm(EMPTY_FORM)
-    setFormError(null)
-    setPendingDelete(null)
-    setUnlockTarget(null)
-    setMasterPassword('')
-    setUnlockError(null)
-    pendingMasterPassword.current = ''
-    pendingSecret.current = null
-  }, [owner])

  const { data: sourcesData } = useQuery({
    enabled: gatewayState === 'open',