fix(desktop): open setup for missing provider rows (#66767)
* fix(desktop): open setup for missing provider rows * fix(desktop): keep provider recovery profile-safe * fix(desktop): satisfy model settings hook deps
This commit is contained in:
Gille
GitHub
parent
ad0ddfb15d
commit
48e36a5370
2 changed files with 150 additions and 12 deletions
|
|
@ -1,5 +1,5 @@
|
|||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
|
||||
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// Radix Select calls scrollIntoView on its items when the content opens; jsdom
|
||||
|
|
@ -21,7 +21,10 @@ const saveMoaModels = vi.fn()
|
|||
const setEnvVar = vi.fn()
|
||||
const getHermesConfigRecord = vi.fn()
|
||||
const saveHermesConfig = vi.fn()
|
||||
const startManualLocalEndpoint = vi.fn()
|
||||
const startManualOnboarding = vi.fn()
|
||||
const startManualProviderOAuth = vi.fn()
|
||||
let profileSwitchHandler: (() => void) | null = null
|
||||
|
||||
vi.mock('@/hermes', () => ({
|
||||
getGlobalModelInfo: () => getGlobalModelInfo(),
|
||||
|
|
@ -38,9 +41,17 @@ vi.mock('@/hermes', () => ({
|
|||
}))
|
||||
|
||||
vi.mock('@/store/onboarding', () => ({
|
||||
startManualLocalEndpoint: () => startManualLocalEndpoint(),
|
||||
startManualOnboarding: () => startManualOnboarding(),
|
||||
startManualProviderOAuth: (slug: string) => startManualProviderOAuth(slug)
|
||||
}))
|
||||
|
||||
vi.mock('../hooks/use-on-profile-switch', () => ({
|
||||
useOnProfileSwitch: (handler: () => void) => {
|
||||
profileSwitchHandler = handler
|
||||
}
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
getGlobalModelInfo.mockResolvedValue({ provider: 'nous', model: 'hermes-4' })
|
||||
getGlobalModelOptions.mockResolvedValue({
|
||||
|
|
@ -69,6 +80,7 @@ beforeEach(() => {
|
|||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.clearAllMocks()
|
||||
profileSwitchHandler = null
|
||||
})
|
||||
|
||||
async function renderModelSettings() {
|
||||
|
|
@ -98,6 +110,103 @@ describe('ModelSettings', () => {
|
|||
expect(screen.queryByText(/DeepSeek/)).toBeNull()
|
||||
})
|
||||
|
||||
it.each(['custom', 'local', 'custom:lab'])(
|
||||
'opens local endpoint setup when %s has no inventory row',
|
||||
async provider => {
|
||||
getGlobalModelInfo.mockResolvedValueOnce({ provider, model: '' })
|
||||
getGlobalModelOptions.mockResolvedValueOnce({ providers: [] })
|
||||
|
||||
await renderModelSettings()
|
||||
|
||||
const providerSelect = (await screen.findAllByRole('combobox'))[0]
|
||||
|
||||
expect(providerSelect.textContent).toContain(provider)
|
||||
expect(screen.queryByText(/undefined/)).toBeNull()
|
||||
expect(screen.queryByText(/signs in through your browser/)).toBeNull()
|
||||
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Set up provider' }))
|
||||
|
||||
expect(startManualLocalEndpoint).toHaveBeenCalledOnce()
|
||||
expect(startManualOnboarding).not.toHaveBeenCalled()
|
||||
expect(startManualProviderOAuth).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
it('opens the generic provider picker for an unknown provider with no inventory row', async () => {
|
||||
getGlobalModelInfo.mockResolvedValueOnce({ provider: 'retired-provider', model: '' })
|
||||
getGlobalModelOptions.mockResolvedValueOnce({ providers: [] })
|
||||
|
||||
await renderModelSettings()
|
||||
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Set up provider' }))
|
||||
|
||||
expect(startManualOnboarding).toHaveBeenCalledOnce()
|
||||
expect(startManualLocalEndpoint).not.toHaveBeenCalled()
|
||||
expect(startManualProviderOAuth).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('deep-links a known OAuth provider row into its setup flow', async () => {
|
||||
getGlobalModelInfo.mockResolvedValueOnce({ provider: 'anthropic', model: '' })
|
||||
getGlobalModelOptions.mockResolvedValueOnce({
|
||||
providers: [
|
||||
{
|
||||
name: 'Anthropic',
|
||||
slug: 'anthropic',
|
||||
models: [],
|
||||
authenticated: false,
|
||||
auth_type: 'oauth'
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
await renderModelSettings()
|
||||
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Set up Anthropic' }))
|
||||
|
||||
expect(startManualProviderOAuth).toHaveBeenCalledWith('anthropic')
|
||||
expect(startManualLocalEndpoint).not.toHaveBeenCalled()
|
||||
expect(startManualOnboarding).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('replaces the selected provider and model when the active profile changes', async () => {
|
||||
getGlobalModelInfo
|
||||
.mockResolvedValueOnce({ provider: 'custom', model: 'local-a' })
|
||||
.mockResolvedValueOnce({ provider: 'nous', model: 'hermes-4' })
|
||||
getGlobalModelOptions
|
||||
.mockResolvedValueOnce({
|
||||
providers: [
|
||||
{
|
||||
name: 'Custom A',
|
||||
slug: 'custom',
|
||||
models: ['local-a'],
|
||||
authenticated: true
|
||||
}
|
||||
]
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
providers: [
|
||||
{
|
||||
name: 'Nous',
|
||||
slug: 'nous',
|
||||
models: ['hermes-4'],
|
||||
authenticated: true,
|
||||
capabilities: { 'hermes-4': { reasoning: true, fast: true } }
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
await renderModelSettings()
|
||||
expect((await screen.findAllByRole('combobox'))[0].textContent).toContain('Custom A')
|
||||
|
||||
await act(async () => {
|
||||
profileSwitchHandler?.()
|
||||
})
|
||||
|
||||
await waitFor(() => expect(getGlobalModelInfo).toHaveBeenCalledTimes(2))
|
||||
await waitFor(() => expect(screen.getAllByRole('combobox')[0].textContent).toContain('Nous'))
|
||||
expect(screen.queryByRole('button', { name: 'Set up provider' })).toBeNull()
|
||||
})
|
||||
|
||||
it('writes the profile default speed (service_tier) when the fast switch is toggled', async () => {
|
||||
await renderModelSettings()
|
||||
await waitFor(() => expect(getHermesConfigRecord).toHaveBeenCalled())
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ import { useI18n } from '@/i18n'
|
|||
import { AlertTriangle, Cpu, Loader2 } from '@/lib/icons'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { notifyError } from '@/store/notifications'
|
||||
import { startManualLocalEndpoint, startManualProviderOAuth } from '@/store/onboarding'
|
||||
import { startManualLocalEndpoint, startManualOnboarding, startManualProviderOAuth } from '@/store/onboarding'
|
||||
|
||||
import { invalidateHermesConfig, setHermesConfigCache, useHermesConfigRecord } from '../hooks/use-config-record'
|
||||
import { useOnProfileSwitch } from '../hooks/use-on-profile-switch'
|
||||
|
|
@ -220,7 +220,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
// A's models/providers into profile B (or fire onMainModelChanged for A).
|
||||
const profileEpoch = useRef(0)
|
||||
|
||||
const refresh = useCallback(async () => {
|
||||
const refresh = useCallback(async ({ replaceSelection = false }: { replaceSelection?: boolean } = {}) => {
|
||||
const epoch = profileEpoch.current
|
||||
setLoading(true)
|
||||
setError('')
|
||||
|
|
@ -239,8 +239,15 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
|
||||
setMainModel({ model: modelInfo.model, provider: modelInfo.provider })
|
||||
setProviders(modelOptions.providers || [])
|
||||
setSelectedProvider(prev => prev || modelInfo.provider)
|
||||
setSelectedModel(prev => prev || modelInfo.model)
|
||||
|
||||
if (replaceSelection) {
|
||||
setSelectedProvider(modelInfo.provider)
|
||||
setSelectedModel(modelInfo.model)
|
||||
} else {
|
||||
setSelectedProvider(prev => prev || modelInfo.provider)
|
||||
setSelectedModel(prev => prev || modelInfo.model)
|
||||
}
|
||||
|
||||
setAuxiliary(auxiliaryModels)
|
||||
setMoa(moaModels)
|
||||
|
||||
|
|
@ -270,11 +277,28 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
// new profile (bumping the epoch first so any in-flight A request is discarded).
|
||||
useOnProfileSwitch(() => {
|
||||
profileEpoch.current += 1
|
||||
void refresh()
|
||||
// The panel stays mounted across profile switches, so clear the previous
|
||||
// profile's draft selection before loading the new profile's source of
|
||||
// truth. Ordinary same-profile refreshes still preserve in-progress edits.
|
||||
setSelectedProvider('')
|
||||
setSelectedModel('')
|
||||
setApiKeyDraft('')
|
||||
void refresh({ replaceSelection: true })
|
||||
})
|
||||
|
||||
const providerOptions = providers.length ? providers : NO_PROVIDERS
|
||||
|
||||
// Radix renders a blank trigger when the controlled value has no matching
|
||||
// item. Keep a missing saved provider visible in the main selector while
|
||||
// leaving it out of the real inventory used for readiness/setup metadata.
|
||||
const mainProviderOptions = useMemo(
|
||||
() =>
|
||||
selectedProvider && !providers.some(provider => provider.slug === selectedProvider)
|
||||
? [{ name: selectedProvider, slug: selectedProvider, models: [] }, ...providers]
|
||||
: providerOptions,
|
||||
[providerOptions, providers, selectedProvider]
|
||||
)
|
||||
|
||||
// MoA reference/aggregator slots must never be the moa virtual provider —
|
||||
// that would create a recursive MoA tree (the backend rejects it on save).
|
||||
// Hide it from the slot selectors so it isn't offered as a dead choice.
|
||||
|
|
@ -504,7 +528,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
notifyError(err, m.defaultsFailed)
|
||||
}
|
||||
},
|
||||
[config, m.defaultsFailed]
|
||||
[config, m.defaultsFailed, setConfig]
|
||||
)
|
||||
|
||||
// Paste an API key for the selected `api_key` provider, persist it, then
|
||||
|
|
@ -561,7 +585,8 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
// local-endpoint form (URL + optional API key) instead of being dead-ended
|
||||
// on the OAuth picker (the original "booted back to the first screen" loop).
|
||||
const startProviderSetup = useCallback(() => {
|
||||
const slug = selectedProviderRow?.slug
|
||||
const rowSlug = selectedProviderRow?.slug.trim() ?? ''
|
||||
const slug = rowSlug || selectedProvider.trim()
|
||||
|
||||
if (!slug) {
|
||||
return
|
||||
|
|
@ -571,10 +596,14 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
|
||||
if (lower === 'custom' || lower === 'local' || lower.startsWith('custom:')) {
|
||||
startManualLocalEndpoint()
|
||||
} else if (rowSlug) {
|
||||
startManualProviderOAuth(rowSlug)
|
||||
} else {
|
||||
startManualProviderOAuth(slug)
|
||||
// An absent row has no trustworthy auth metadata. Open the generic
|
||||
// provider picker instead of deep-linking an unknown or stale slug.
|
||||
startManualOnboarding()
|
||||
}
|
||||
}, [selectedProviderRow])
|
||||
}, [selectedProvider, selectedProviderRow])
|
||||
|
||||
const applyMainModel = useCallback(async () => {
|
||||
if (!selectedProvider || !selectedModel) {
|
||||
|
|
@ -700,7 +729,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
<SelectValue placeholder={m.provider} />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{providerOptions.map(provider => (
|
||||
{mainProviderOptions.map(provider => (
|
||||
<SelectItem key={provider.slug || 'none'} value={provider.slug || 'none'}>
|
||||
{provider.name}
|
||||
</SelectItem>
|
||||
|
|
@ -762,7 +791,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
</>
|
||||
)}
|
||||
</div>
|
||||
{needsSetup && !setupIsApiKey && (
|
||||
{needsSetup && !setupIsApiKey && selectedProviderRow && (
|
||||
<p className="mt-2 text-xs text-muted-foreground">
|
||||
{selectedProviderRow?.auth_type === 'api_key'
|
||||
? `${selectedProviderRow?.name} needs an API key — set it up to choose a model.`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue