diff --git a/apps/desktop/src/app/shell/model-menu-panel.test.tsx b/apps/desktop/src/app/shell/model-menu-panel.test.tsx index 7ae098d0f673..64532b2541a0 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.test.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.test.tsx @@ -4,7 +4,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vite import { DropdownMenu, DropdownMenuContent } from '@/components/ui/dropdown-menu' import { $activeSessionId, $currentModel, $currentProvider } from '@/store/session' -import { $collapsedProviders } from '@/store/provider-collapse' +import { $collapsedProviders, toggleCollapsedProvider } from '@/store/provider-collapse' import { ModelMenuPanel } from './model-menu-panel' @@ -197,4 +197,55 @@ describe('ModelMenuPanel provider collapse', () => { expect(content.queryByText('Deepseek V4 Pro')).toBeNull() }) + + // The collapsed-providers set is a global presentation preference + // (`hermes.desktop.collapsed-providers`), but the catalog the picker renders + // is profile-scoped (`getGlobalModelOptions` routes through + // `profileScoped()`). Pruning the global set against only the active catalog + // would silently delete a user's collapse preference on every profile switch + // whose configured providers don't include the slug — the bug the maintainer + // flagged. The set must survive catalog changes; if the same provider shows + // up again later, the previous collapse is preserved. + it('preserves the collapsed set across a profile switch whose catalog lacks the slug', async () => { + toggleCollapsedProvider('deepseek') + toggleCollapsedProvider('google') + expect($collapsedProviders.get()).toEqual(['deepseek', 'google']) + + // Profile A: both providers present, render + unmount. + getGlobalModelOptions.mockResolvedValueOnce({ providers: MOCK_PROVIDERS }) + const a = renderPanel() + await a.content.findByText('DeepSeek') + a.content.unmount() + + // Profile B: google is not in the catalog (simulates a profile whose + // configured providers differ). The previously-collapsed 'google' slug + // must survive — pruning it would lose state across a profile switch. + getGlobalModelOptions.mockResolvedValueOnce({ providers: [DEEPSEEK_PROVIDER, MOA_PROVIDER] }) + const b = renderPanel() + await b.content.findByText('DeepSeek') + + expect($collapsedProviders.get()).toEqual(['deepseek', 'google']) + }) + + it('preserves the collapsed set when Refresh Models drops a provider', async () => { + toggleCollapsedProvider('deepseek') + toggleCollapsedProvider('google') + + // First load: both providers present. + getGlobalModelOptions.mockResolvedValueOnce({ providers: MOCK_PROVIDERS }) + const a = renderPanel() + await a.content.findByText('DeepSeek') + a.content.unmount() + + // Refresh Models returns a catalog that drops google (revoked key, + // plugin disabled, backend policy change). 'google' must survive — the + // user explicitly collapsed it, and the global set is not tied to any + // single refresh. + getGlobalModelOptions.mockResolvedValueOnce({ providers: [DEEPSEEK_PROVIDER, MOA_PROVIDER] }) + const b = renderPanel() + await b.content.findByText('DeepSeek') + + expect($collapsedProviders.get()).toContain('google') + expect($collapsedProviders.get()).toContain('deepseek') + }) }) diff --git a/apps/desktop/src/app/shell/model-menu-panel.tsx b/apps/desktop/src/app/shell/model-menu-panel.tsx index 6dd816191ffa..6a600f7b8c38 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.tsx @@ -1,6 +1,6 @@ import { useStore } from '@nanostores/react' import { useQuery, useQueryClient } from '@tanstack/react-query' -import { createContext, useContext, useEffect, useMemo, useState } from 'react' +import { createContext, useContext, useMemo, useState } from 'react' import { useSessionView } from '@/app/chat/session-view' import { Codicon } from '@/components/ui/codicon' @@ -38,7 +38,7 @@ import { modelVisibilityKey, setModelVisibilityOpen } from '@/store/model-visibility' -import { $collapsedProviders, pruneStaleCollapsed, toggleCollapsedProvider } from '@/store/provider-collapse' +import { $collapsedProviders, toggleCollapsedProvider } from '@/store/provider-collapse' import type { ModelOptionProvider, ModelOptionsResponse } from '@/types/hermes' import { ModelEditSubmenu, resolveFastControl } from './model-edit-submenu' @@ -123,14 +123,6 @@ export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: Model [providers] ) - // Drop stale collapsed provider slugs when the provider list changes - // (e.g. after Refresh Models, plugin install/uninstall). - useEffect(() => { - if (pickerProviders.length > 0) { - pruneStaleCollapsed(pickerProviders.map(p => p.slug)) - } - }, [pickerProviders]) - const effectiveVisibleModels = useMemo( () => effectiveVisibleKeys(visibleModels, pickerProviders), [visibleModels, pickerProviders] diff --git a/apps/desktop/src/store/provider-collapse.ts b/apps/desktop/src/store/provider-collapse.ts index a139003c77f3..bd484ac008a7 100644 --- a/apps/desktop/src/store/provider-collapse.ts +++ b/apps/desktop/src/store/provider-collapse.ts @@ -4,7 +4,21 @@ const STORAGE_KEY = 'hermes.desktop.collapsed-providers' /** Set of provider slugs whose model groups are currently collapsed in the * model picker dropdown. Persisted to localStorage globally — this is a - * presentation-layer preference, not a per-profile setting. */ + * presentation-layer preference, not a per-profile setting. + * + * We deliberately do NOT prune this set when the active provider catalog + * changes (e.g. profile switch, Refresh Models, API key revoked). The catalog + * the picker renders is profile-scoped (`getGlobalModelOptions` routes through + * `profileScoped()`), so pruning against only the active catalog would delete + * a user's collapse preference every time they switch to a profile whose + * configured providers don't include it — silently losing state across what + * is otherwise a pure presentational toggle. + * + * Provider slugs come from a small bounded configured set (not user input), + * so dead entries in the array cost a few bytes and have no observable effect: + * the render loop only visits providers present in the active `groups`, and + * `collapsedProviders.includes(slug)` against an absent slug is a no-op. + */ export const $collapsedProviders = persistentAtom( STORAGE_KEY, [], @@ -20,15 +34,3 @@ export function toggleCollapsedProvider(slug: string): void { : [...current, slug] ) } - -/** Remove slugs from the collapsed set that no longer match any current - * provider. Call this after provider list changes (e.g. Refresh Models, - * plugin install/uninstall) to prevent unbounded growth. */ -export function pruneStaleCollapsed(slugs: string[]): void { - const valid = new Set(slugs) - const next = $collapsedProviders.get().filter(s => valid.has(s)) - - if (next.length !== $collapsedProviders.get().length) { - $collapsedProviders.set(next) - } -}