From 43915b391dcc463fcfcb66a21e6b5cd8a81c28e0 Mon Sep 17 00:00:00 2001 From: Josh Stevenson Date: Sun, 4 Oct 2026 02:24:23 -0700 Subject: [PATCH] fix(desktop): preserve model picker source and receipt ownership --- apps/desktop/src/api/models.ts | 5 +- .../src/app/hooks/use-composer-model-owner.ts | 98 +++++++++ .../src/app/model-picker-overlay.test.tsx | 132 ++++++++++++ apps/desktop/src/app/model-picker-overlay.tsx | 21 +- .../model-selection-composition.test.tsx | 8 + .../custom-endpoints-settings.test.tsx | 93 ++++++++- .../settings/custom-endpoints-settings.tsx | 74 +++++-- .../src/app/settings/model-settings.test.tsx | 89 +++++++- .../src/app/settings/model-settings.tsx | 139 ++++++++++--- .../app/shell/model-menu-panel-owner.test.tsx | 129 ++++++++++++ .../src/app/shell/model-menu-panel.test.tsx | 4 +- .../src/app/shell/model-menu-panel.tsx | 25 ++- .../src/components/model-picker.test.tsx | 18 ++ apps/desktop/src/components/model-picker.tsx | 5 +- apps/desktop/src/lib/model-options.test.ts | 192 +++++++++++++++++- apps/desktop/src/lib/model-options.ts | 5 +- .../src/lib/model-status-label.test.ts | 8 + apps/desktop/src/lib/model-status-label.ts | 7 +- 18 files changed, 968 insertions(+), 84 deletions(-) create mode 100644 apps/desktop/src/app/hooks/use-composer-model-owner.ts create mode 100644 apps/desktop/src/app/model-picker-overlay.test.tsx create mode 100644 apps/desktop/src/app/shell/model-menu-panel-owner.test.tsx diff --git a/apps/desktop/src/api/models.ts b/apps/desktop/src/api/models.ts index b484c3f2faa5e..2d12ef561817b 100644 --- a/apps/desktop/src/api/models.ts +++ b/apps/desktop/src/api/models.ts @@ -50,7 +50,10 @@ export function getGlobalModelOptions( return window.hermesDesktop.api({ ...capabilityScoped(profile), - ...(profile && typeof profile === 'object' ? { connectionId: profile.connectionId || 'local' } : {}), + // Explicit null preserves the captured legacy route, which may be remote. + ...(profile && typeof profile === 'object' && profile.connectionId !== null + ? { connectionId: profile.connectionId || 'local' } + : {}), path: params.size > 0 ? `/api/model/options?${params.toString()}` : '/api/model/options', timeoutMs: STARTUP_REQUEST_TIMEOUT_MS }) diff --git a/apps/desktop/src/app/hooks/use-composer-model-owner.ts b/apps/desktop/src/app/hooks/use-composer-model-owner.ts new file mode 100644 index 0000000000000..ede992fe406c4 --- /dev/null +++ b/apps/desktop/src/app/hooks/use-composer-model-owner.ts @@ -0,0 +1,98 @@ +import { useStore } from '@nanostores/react' +import { useCallback, useLayoutEffect, useRef, useState } from 'react' + +import { + captureDraftComposerOwner, + captureModelRequestOwner, + composerOwnerKey, + type ComposerSelectionOwner +} from '@/app/session/hooks/composer-model-selection-owner' +import { $activeGatewayProfile, $newChatConnectionId, $newChatProfile, $newChatRoute } from '@/store/profile' +import { $connection } from '@/store/session' + +// These atoms publish rehomes; canonical capture still owns route precedence. +// Do not infer model authority from the mirrored model/provider atoms here. +function useModelOwnerChanges() { + useStore($connection) + useStore($activeGatewayProfile) + useStore($newChatConnectionId) + useStore($newChatProfile) + useStore($newChatRoute) +} + +export function useModelRequestOwner(scopeProfile?: string): ComposerSelectionOwner { + // Canonical capture reads imperative stores; it must run on every rehome. + 'use no memo' + + useModelOwnerChanges() + + return captureModelRequestOwner(scopeProfile) +} + +export function useDraftComposerOwner(): ComposerSelectionOwner { + 'use no memo' + + useModelOwnerChanges() + + return captureDraftComposerOwner() +} + +/** Form lifecycle only: a batched round trip still needs a fresh form. */ +export function useModelFormKey(owner: ComposerSelectionOwner, scopeProfile?: string): string { + const [revision, setRevision] = useState(0) + useLayoutEffect(() => { + let previousKey = composerOwnerKey(captureModelRequestOwner(scopeProfile)) + + const rehome = () => { + const nextKey = composerOwnerKey(captureModelRequestOwner(scopeProfile)) + + if (nextKey !== previousKey) { + previousKey = nextKey + setRevision(value => value + 1) + } + } + + const unlisten = [$connection, $activeGatewayProfile, $newChatConnectionId, $newChatProfile, $newChatRoute] + .map(store => store.listen(rehome)) + + return () => { unlisten.forEach(stop => stop()) } + }, [scopeProfile]) + + return JSON.stringify([composerOwnerKey(owner), revision]) +} + +/** A remounted form cannot regain permission when the user returns A→B→A. */ +export function useModelOwnerIsCurrent(owner: ComposerSelectionOwner, scopeProfile?: string): () => boolean { + const ownerKey = composerOwnerKey(owner) + const mounted = useRef(true) + useLayoutEffect(() => { + mounted.current = true + + const invalidate = () => { + if (composerOwnerKey(captureModelRequestOwner(scopeProfile)) !== ownerKey) { + mounted.current = false + } + } + + // Observe the transition itself, even when React batches A→B→A into one + // paint. This lease cannot become valid again before a fresh form mount. + const unlisten = [$connection, $activeGatewayProfile, $newChatConnectionId, $newChatProfile, $newChatRoute] + .map(store => store.listen(invalidate)) + + return () => { + mounted.current = false + unlisten.forEach(stop => stop()) + } + }, [ownerKey, scopeProfile]) + + return useCallback( + () => mounted.current && composerOwnerKey(captureModelRequestOwner(scopeProfile)) === ownerKey, + [ownerKey, scopeProfile] + ) +} + +export function requireCurrentModelOwner(isCurrent: () => boolean): void { + if (!isCurrent()) { + throw new Error('Model settings target changed. Reopen the selection before saving.') + } +} diff --git a/apps/desktop/src/app/model-picker-overlay.test.tsx b/apps/desktop/src/app/model-picker-overlay.test.tsx new file mode 100644 index 0000000000000..b0c3448bdb10f --- /dev/null +++ b/apps/desktop/src/app/model-picker-overlay.test.tsx @@ -0,0 +1,132 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeAll, beforeEach, expect, it, vi } from 'vitest' + +import { setApiRequestConnection, setApiRequestProfile } from '@/api/client' +import type * as Gateway from '@/store/gateway' +import { $newChatConnectionId, $newChatProfile, $newChatRoute } from '@/store/profile' +import { + $activeSessionId, $currentModel, $currentProvider, $gatewayState, $modelPickerOpen, $sessions, + _resetComposerModelSelectionsForTests, _resetSessionOwnerHintsForTests, captureComposerModelSelection, + recordComposerModelSelection, setComposerModelSelectionOwner, setSessionOwnerHint +} from '@/store/session' +import { knownOwnerForSession } from '@/store/session-states' + +import { ModelPickerOverlay } from './model-picker-overlay' + +const calls = vi.hoisted(() => ({ agent: vi.fn(), profile: vi.fn(), rest: vi.fn() })) +vi.mock('@/store/gateway', async original => ({ + ...await original(), + requestGatewayForAgent: (...args: unknown[]) => calls.agent(...args), + requestGatewayForProfile: (...args: unknown[]) => calls.profile(...args) +})) +vi.mock('@/hermes', () => ({ getGlobalModelOptions: (...args: unknown[]) => calls.rest(...args), setApiRequestProfile: vi.fn() })) +beforeAll(() => { + Element.prototype.scrollIntoView = vi.fn() + vi.stubGlobal('ResizeObserver', class { observe() {} unobserve() {} disconnect() {} }) +}) +const owner = { connectionId: 'source-b', profile: 'same-name', targetProfile: 'backend-b' } +const options = { model: 'catalog-b', provider: 'custom:b', providers: [{ name: 'B', slug: 'custom:b', models: ['catalog-b', 'pinned-b'] }] } + +beforeEach(() => { + vi.clearAllMocks() + calls.agent.mockReset().mockResolvedValue(options) + calls.profile.mockReset().mockResolvedValue(options) + calls.rest.mockReset().mockResolvedValue(options) + _resetComposerModelSelectionsForTests() + _resetSessionOwnerHintsForTests() + $sessions.set([]) + $activeSessionId.set(null) + $gatewayState.set('open') + $modelPickerOpen.set(true) + $currentModel.set('ambient-a') + $currentProvider.set('provider-a') + setApiRequestConnection('source-a') + setApiRequestProfile('default') + $newChatRoute.set(owner) + $newChatProfile.set(owner.profile) + $newChatConnectionId.set(owner.connectionId) + setComposerModelSelectionOwner(owner) +}) +afterEach(() => { cleanup(); setApiRequestConnection(null); setApiRequestProfile('default'); $newChatRoute.set(null); $newChatProfile.set(null); $newChatConnectionId.set(null); $modelPickerOpen.set(false); $gatewayState.set('idle'); $sessions.set([]); _resetSessionOwnerHintsForTests() }) + +function mount() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const ambient = vi.fn(async () => options) + const select = vi.fn() + const view = render() + + return { client, view, ambient, select } +} + +it('uses the captured fresh draft source and backend target in the actual overlay catalog', async () => { + const { ambient } = mount() + await screen.findByText('catalog-b') + expect(calls.agent).toHaveBeenCalledWith('source-b', 'same-name', 'model.options', { profile: 'backend-b', explicit_only: true }) + expect(ambient).not.toHaveBeenCalled() +}) + +it('does not mark a stale catalog default current when a valid scalar receipt owns the draft', async () => { + recordComposerModelSelection(captureComposerModelSelection(owner), { model: 'scalar-b', provider: '', source: 'default' }) + $currentModel.set('scalar-b') + $currentProvider.set('') + const { select } = mount() + const row = await screen.findByText('catalog-b') + expect(row.closest('[cmdk-item]')?.className).not.toContain('bg-primary text-primary-foreground') + expect(select).not.toHaveBeenCalled() +}) + +it('preserves a deliberate draft pin through a late catalog reply', async () => { + let resolve!: (value: typeof options) => void + calls.agent.mockReturnValueOnce(new Promise(r => { resolve = r })) + mount() + await waitFor(() => expect(calls.agent).toHaveBeenCalledOnce()) + await act(async () => { + recordComposerModelSelection(captureComposerModelSelection(owner), { model: 'pinned-b', provider: 'custom:b', source: 'manual' }) + $currentModel.set('pinned-b') + $currentProvider.set('custom:b') + resolve(options) + }) + const row = await screen.findByText('pinned-b') + expect(row.closest('[cmdk-item]')?.className).toContain('bg-primary text-primary-foreground') +}) + +it.each([ + ['legacy', 'rejected'], ['legacy', 'empty'], ['local', 'rejected'], ['local', 'empty'] +] as const)('preserves a live %s owner through %s RPC recovery in the overlay', async (source, rpcUnavailable) => { + const runtimeId = 'owned-runtime' + const connectionId = source === 'legacy' ? null : 'local' + $sessions.set([{ id: runtimeId, profile: 'default' }] as never) + + if (connectionId) { + setSessionOwnerHint(runtimeId, { connectionId, profile: 'default', mode: 'local' }) + } + + expect(knownOwnerForSession(runtimeId)).toEqual(connectionId ? { connectionId, profile: 'default', mode: 'local' } : 'default') + $activeSessionId.set(runtimeId) + $currentModel.set('') + $currentProvider.set('') + const rpc = source === 'legacy' ? calls.profile : calls.agent + + if (rpcUnavailable === 'empty') { + rpc.mockResolvedValue({ providers: [] }) + } else { + rpc.mockRejectedValue(new Error('Offline catalog RPC')) + } + + const model = source === 'legacy' ? 'legacy-a' : 'local-b' + const expected = { ...options, model, providers: [{ ...options.providers[0], models: [model] }] } + calls.rest.mockImplementation(async (_opts, scope) => scope.connectionId === connectionId ? expected : options) + const { client, ambient } = mount() + await waitFor(() => expect(calls.rest).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId, profile: 'default' })) + await screen.findByText(model) + expect(ambient).not.toHaveBeenCalled() + + if (source === 'legacy') { + expect(calls.profile).toHaveBeenCalledWith('default', 'model.options', { profile: 'default', session_id: runtimeId, explicit_only: true }, undefined, undefined) + } else { + expect(calls.agent).toHaveBeenCalledWith('local', 'default', 'model.options', { profile: 'default', session_id: runtimeId, explicit_only: true }) + } + + expect(client.getQueryData(['model-options', source === 'legacy' ? 'default' : 'local::default', runtimeId])).toEqual(expected) +}) diff --git a/apps/desktop/src/app/model-picker-overlay.tsx b/apps/desktop/src/app/model-picker-overlay.tsx index 626327b75edbb..e9670636c4126 100644 --- a/apps/desktop/src/app/model-picker-overlay.tsx +++ b/apps/desktop/src/app/model-picker-overlay.tsx @@ -4,17 +4,21 @@ import type { ModelSelection } from '@/app/shell/model-menu-panel' import { ModelPickerDialog } from '@/components/model-picker' import type { HermesGateway } from '@/hermes' import { useStoreSelector } from '@/lib/use-session-slice' +import { requestGatewayForAgent } from '@/store/gateway' import { $activeSessionId, $currentModel, $currentProvider, $gatewayState, $modelPickerOpen, + getComposerModelSelection, setModelPickerOpen } from '@/store/session' import { knownOwnerForSession, requestForOwnedSession } from '@/store/session-states' import { $focusedRuntimeId, $focusedSessionState } from '@/store/session-states' +import { useDraftComposerOwner } from './hooks/use-composer-model-owner' + interface ModelPickerOverlayProps { gateway?: HermesGateway onSelect: (selection: ModelSelection) => void @@ -36,6 +40,7 @@ export function ModelPickerOverlay({ gateway, onSelect, profile }: ModelPickerOv const focusedProvider = useStoreSelector($focusedSessionState, state => state?.provider ?? null) const gatewayOpen = useStore($gatewayState) === 'open' const open = useStore($modelPickerOpen) + const draftOwner = useDraftComposerOwner() // Prefer the focused tile's runtime when the overlay opens from a tile that // lacked a live menu (gateway closed → fallback path). @@ -48,20 +53,24 @@ export function ModelPickerOverlay({ gateway, onSelect, profile }: ModelPickerOv } const owner = knownOwnerForSession(sessionId) + const draftSelection = !sessionId ? getComposerModelSelection(draftOwner) : null const ownerProfile = typeof owner === 'string' ? owner : (owner?.targetProfile || owner?.profile) - const ownerConnection = owner && typeof owner === 'object' ? owner.connectionId : owner ? 'local' : undefined + const ownerConnection = owner && typeof owner === 'object' ? owner.connectionId : owner ? null : undefined return ( onSelect({ ...selection, sessionId })} open={open} - profile={ownerProfile || profile} - request={gateway && sessionId ? (method, params) => requestForOwnedSession(sessionId, gateway.request.bind(gateway), method, params) : undefined} + profile={ownerProfile || (!sessionId ? draftOwner.targetProfile || draftOwner.profile : profile)} + request={!sessionId + ? (method, params) => requestGatewayForAgent(draftOwner.connectionId, draftOwner.profile, method, params) + : gateway ? (method, params) => requestForOwnedSession(sessionId, gateway.request.bind(gateway), method, params) : undefined} + selectionIsAuthoritative={Boolean(draftSelection)} sessionId={sessionId} /> ) diff --git a/apps/desktop/src/app/session/hooks/model-selection-composition.test.tsx b/apps/desktop/src/app/session/hooks/model-selection-composition.test.tsx index d65205fc5f940..242895b0ef57b 100644 --- a/apps/desktop/src/app/session/hooks/model-selection-composition.test.tsx +++ b/apps/desktop/src/app/session/hooks/model-selection-composition.test.tsx @@ -513,6 +513,14 @@ describe('actual producer/store/admission composition', () => { }) it('refuses the actual Settings confirmation retry after A changes to B with the same logical profile', async () => { + vi.mocked(getGlobalModelOptions).mockImplementation(async (_opts, scope) => { + const connectionId = scope && typeof scope === 'object' ? scope.connectionId : getApiRequestConnection() + const pair = connectionId === b.connectionId ? defaultB : defaultA + + return { + providers: [{ slug: pair.provider, name: 'Offline owner', authenticated: true, models: [pair.model] }] + } + }) const value = await setup(1) await seed(value) const assignmentSources: (string | null)[] = [] diff --git a/apps/desktop/src/app/settings/custom-endpoints-settings.test.tsx b/apps/desktop/src/app/settings/custom-endpoints-settings.test.tsx index 5477ab4008f72..dd9a48d9ffcc4 100644 --- a/apps/desktop/src/app/settings/custom-endpoints-settings.test.tsx +++ b/apps/desktop/src/app/settings/custom-endpoints-settings.test.tsx @@ -1,10 +1,10 @@ -import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { setApiRequestConnection, setApiRequestProfile } from '@/api/client' +import { getApiRequestConnection, setApiRequestConnection, setApiRequestProfile } from '@/api/client' import { confirm } from '@/store/confirm' import { $activeGatewayProfile, $newChatProfile, $newChatRoute } from '@/store/profile' -import { _resetComposerModelSelectionsForTests } from '@/store/session' +import { $connection, _resetComposerModelSelectionsForTests } from '@/store/session' import type { CustomEndpoint, CustomEndpointsResponse } from '@/types/hermes' import { deferred } from '../../test/deferred' @@ -57,6 +57,7 @@ beforeEach(() => { deleteCustomEndpoint.mockReset() vi.mocked(confirm).mockReset().mockResolvedValue(true) _resetComposerModelSelectionsForTests() + $connection.set(null) $newChatRoute.set(owner) $newChatProfile.set(owner.profile) $activeGatewayProfile.set(owner.profile) @@ -82,6 +83,92 @@ async function renderSettings(changed = vi.fn()) { } describe('custom endpoint save ownership', () => { + it('clears source A form authority and key draft when the same profile rehomes to B and back', async () => { + const second = { ...endpoint, name: 'Source B endpoint', model: 'model-b', models: ['model-b'], base_url: 'https://b.invalid/v1' } + + const rehome = (connectionId: string) => { + setApiRequestConnection(connectionId) + $connection.set({ connectionId } as never) + $newChatRoute.set({ connectionId, profile: owner.profile, targetProfile: connectionId === 'source-a' ? 'backend-a' : 'backend-b' }) + } + + getCustomEndpoints.mockImplementation(async () => ({ endpoints: [getApiRequestConnection() === 'source-b' ? second : endpoint] })) + saveCustomEndpoint.mockResolvedValue({ id: second.id, endpoints: [second] }) + await renderSettings() + fireEvent.change(screen.getByPlaceholderText('Leave blank to keep current key'), { target: { value: 'fake-source-a-key' } }) + await act(async () => rehome('source-b')) + await screen.findByDisplayValue(second.base_url) + expect((screen.getByPlaceholderText('Leave blank to keep current key') as HTMLInputElement).value).toBe('') + fireEvent.click(screen.getByRole('button', { name: 'Save' })) + await waitFor(() => expect(saveCustomEndpoint).toHaveBeenCalledWith(expect.objectContaining({ + base_url: second.base_url, model: second.model, api_key: undefined + }))) + await act(async () => rehome('source-a')) + await screen.findByDisplayValue(endpoint.base_url) + }) + + it('rejects a delayed Delete confirmation from an unmounted owner even after A → B → A', async () => { + const confirmation = deferred() + vi.mocked(confirm).mockReturnValueOnce(confirmation.promise) + deleteCustomEndpoint.mockResolvedValue({ endpoints: [] }) + await renderSettings() + fireEvent.click(screen.getByRole('button', { name: 'Delete endpoint' })) + await waitFor(() => expect(confirm).toHaveBeenCalledOnce()) + + const rehome = (connectionId: string) => { + setApiRequestConnection(connectionId) + $connection.set({ connectionId } as never) + $newChatRoute.set({ connectionId, profile: owner.profile, targetProfile: connectionId === 'source-a' ? 'backend-a' : 'backend-b' }) + } + + await act(async () => rehome('source-b')) + await screen.findByRole('button', { name: 'Save' }) + await act(async () => rehome('source-a')) + await screen.findByRole('button', { name: 'Save' }) + await act(async () => confirmation.resolve(true)) + expect(deleteCustomEndpoint).not.toHaveBeenCalled() + }) + + it('rejects a stale Delete confirmation when React batches the source A → B → A round trip', async () => { + const confirmation = deferred() + vi.mocked(confirm).mockReturnValueOnce(confirmation.promise) + deleteCustomEndpoint.mockResolvedValue({ endpoints: [] }) + saveCustomEndpoint.mockResolvedValue(savedResponse()) + await renderSettings() + fireEvent.click(screen.getByRole('button', { name: 'Delete endpoint' })) + await waitFor(() => expect(confirm).toHaveBeenCalledOnce()) + await act(async () => { + setApiRequestConnection('source-b') + $connection.set({ connectionId: 'source-b' } as never) + setApiRequestConnection(owner.connectionId) + $connection.set({ connectionId: owner.connectionId } as never) + confirmation.resolve(true) + }) + expect(deleteCustomEndpoint).not.toHaveBeenCalled() + expect(notifyError).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringMatching(/target changed/i) }), 'Delete failed') + await waitFor(() => expect(getCustomEndpoints).toHaveBeenCalledTimes(2)) + await screen.findByDisplayValue(endpoint.base_url) + fireEvent.click(await screen.findByRole('button', { name: 'Save' })) + await waitFor(() => expect(saveCustomEndpoint).toHaveBeenCalledOnce()) + }) + + it('discards a late source A inventory after the settings owner has rehomed to B', async () => { + const pending = deferred() + const second = { ...endpoint, name: 'Source B endpoint', base_url: 'https://b.invalid/v1' } + getCustomEndpoints.mockReturnValueOnce(pending.promise).mockResolvedValue({ endpoints: [second] }) + const { CustomEndpointsSettings } = await import('./custom-endpoints-settings') + render() + await waitFor(() => expect(getCustomEndpoints).toHaveBeenCalledOnce()) + await act(async () => { + setApiRequestConnection('source-b') + $connection.set({ connectionId: 'source-b' } as never) + $newChatRoute.set({ connectionId: 'source-b', profile: owner.profile }) + }) + await act(async () => pending.resolve({ endpoints: [endpoint], current: savedResponse().current })) + await screen.findByDisplayValue(second.base_url) + expect(screen.queryByDisplayValue(endpoint.base_url)).toBeNull() + }) + it('carries the original owner through a delayed save callback', async () => { const pending = deferred() saveCustomEndpoint.mockReturnValueOnce(pending.promise) diff --git a/apps/desktop/src/app/settings/custom-endpoints-settings.tsx b/apps/desktop/src/app/settings/custom-endpoints-settings.tsx index 35e9254578564..e4d9252555636 100644 --- a/apps/desktop/src/app/settings/custom-endpoints-settings.tsx +++ b/apps/desktop/src/app/settings/custom-endpoints-settings.tsx @@ -1,7 +1,7 @@ import { useEffect, useRef, useState } from 'react' import { beginMainModelSave, ownsMainModelSave } from '@/app/session/hooks/composer-model-selection-owner' -import type { OnMainModelChanged } from '@/app/session/hooks/composer-model-selection-owner' +import type { ComposerSelectionOwner, OnMainModelChanged } from '@/app/session/hooks/composer-model-selection-owner' import { Button } from '@/components/ui/button' import { Checkbox } from '@/components/ui/checkbox' import { Input } from '@/components/ui/input' @@ -19,6 +19,8 @@ import { confirm } from '@/store/confirm' import { notify, notifyError } from '@/store/notifications' import type { CustomEndpoint, CustomEndpointUpdate } from '@/types/hermes' +import { requireCurrentModelOwner, useModelFormKey, useModelOwnerIsCurrent, useModelRequestOwner } from '../hooks/use-composer-model-owner' + import { EmptyState, Pill, SectionHeading, SettingsContent, SettingsSkeleton } from './primitives' interface CustomEndpointsSettingsProps { @@ -78,6 +80,14 @@ function toPayload(form: EndpointForm, models?: string[]): CustomEndpointUpdate } export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: CustomEndpointsSettingsProps) { + const owner = useModelRequestOwner() + const formKey = useModelFormKey(owner) + + return +} + +function CustomEndpointsForm({ onConfigSaved, onMainModelChanged, owner }: CustomEndpointsSettingsProps & { owner: ComposerSelectionOwner }) { + const isOwnerCurrent = useModelOwnerIsCurrent(owner) const [loading, setLoading] = useState(true) const [saving, setSaving] = useState(false) const [testing, setTesting] = useState(false) @@ -91,8 +101,15 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C const modelWritePendingRef = useRef(false) async function refresh() { + if (!isOwnerCurrent()) { + return + } + const data = await getCustomEndpoints() - setEndpoints(data.endpoints) + + if (isOwnerCurrent()) { + setEndpoints(data.endpoints) + } } useEffect(() => { @@ -102,7 +119,7 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C try { const data = await getCustomEndpoints() - if (cancelled) { + if (cancelled || !isOwnerCurrent()) { return } @@ -114,9 +131,11 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C setDiscoveredModels(current.models) } } catch (err) { - notifyError(err, 'Could not load custom endpoints') + if (!cancelled && isOwnerCurrent()) { + notifyError(err, 'Could not load custom endpoints') + } } finally { - if (!cancelled) { + if (!cancelled && isOwnerCurrent()) { setLoading(false) } } @@ -127,7 +146,7 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C return () => { cancelled = true } - }, []) + }, [isOwnerCurrent]) async function handleSave() { if (modelWritePendingRef.current) { @@ -135,9 +154,10 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C } modelWritePendingRef.current = true - const origin = beginMainModelSave() try { + requireCurrentModelOwner(isOwnerCurrent) + const origin = beginMainModelSave() setSaving(true) const response = await saveCustomEndpoint(toPayload(form, discoveredModels)) @@ -145,20 +165,26 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C return } - setEndpoints(response.endpoints) const saved = response.endpoints.find(endpoint => endpoint.id === response.id) + if (saved && saved.is_current) { + onMainModelChanged?.({ ...origin, provider: saved.id, model: saved.model }) + } + + onConfigSaved?.() + + if (!isOwnerCurrent()) { + return + } + + setEndpoints(response.endpoints) + if (saved) { setForm(formFromEndpoint(saved)) setDiscoveredModels(saved.models) } - if (saved && saved.is_current) { - onMainModelChanged?.({ ...origin, provider: saved.id, model: saved.model }) - } - triggerHaptic('success') - onConfigSaved?.() notify({ kind: 'success', message: 'Custom endpoint saved.' }) } catch (err) { notifyError(err, 'Save failed') @@ -170,8 +196,14 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C async function handleValidate() { try { + requireCurrentModelOwner(isOwnerCurrent) setTesting(true) const response = await validateCustomEndpoint(toPayload(form)) + + if (!isOwnerCurrent()) { + return + } + setDiscoveredModels(response.models) if (response.ok) { @@ -204,9 +236,10 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C } modelWritePendingRef.current = true - const origin = beginMainModelSave() try { + requireCurrentModelOwner(isOwnerCurrent) + const origin = beginMainModelSave() setActivating(endpoint.id) const response = await activateCustomEndpoint(endpoint.id) @@ -218,6 +251,11 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C // the committed default from the composer or report activation failure. onConfigSaved?.() onMainModelChanged?.({ ...origin, provider: response.provider, model: response.model }) + + if (!isOwnerCurrent()) { + return + } + triggerHaptic('success') try { @@ -250,8 +288,15 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C modelWritePendingRef.current = true try { + requireCurrentModelOwner(isOwnerCurrent) setDeleting(endpoint.id) const response = await deleteCustomEndpoint(endpoint.id) + onConfigSaved?.() + + if (!isOwnerCurrent()) { + return + } + setEndpoints(response.endpoints) if (form.id === endpoint.id) { @@ -259,7 +304,6 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C setDiscoveredModels([]) } - onConfigSaved?.() triggerHaptic('success') } catch (err) { notifyError(err, 'Delete failed') diff --git a/apps/desktop/src/app/settings/model-settings.test.tsx b/apps/desktop/src/app/settings/model-settings.test.tsx index 8d80c37523c7f..125df1f7c67a9 100644 --- a/apps/desktop/src/app/settings/model-settings.test.tsx +++ b/apps/desktop/src/app/settings/model-settings.test.tsx @@ -4,8 +4,9 @@ import { MemoryRouter } from 'react-router' import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import { getApiRequestConnection, setApiRequestConnection, setApiRequestProfile } from '@/api/client' +import { getHermesConfigRecord as readConfigOverBridge, saveHermesConfig as saveConfigOverBridge } from '@/api/config' import { $activeGatewayProfile, $newChatProfile, $newChatRoute } from '@/store/profile' -import { _resetComposerModelSelectionsForTests } from '@/store/session' +import { $connection, _resetComposerModelSelectionsForTests } from '@/store/session' import { deferred } from '../../test/deferred' @@ -39,13 +40,13 @@ vi.mock('@/hermes', () => ({ getAuxiliaryModels: (profile?: null | string) => getAuxiliaryModels(profile), getApiRequestProfile: () => 'default', getMoaModels: (profile?: null | string) => getMoaModels(profile), - profileScopeKey: (scope?: null | string) => (scope ?? '').trim() || 'default', + profileScopeKey: (scope?: null | string | { connectionId?: string | null; profile?: string | null }) => typeof scope === 'object' && scope ? `${scope.connectionId || 'local'}::${scope.profile || 'default'}` : (scope ?? '').trim() || 'default', setModelAssignment: (body: unknown) => setModelAssignment(body), getRecommendedDefaultModel: (slug: string) => getRecommendedDefaultModel(slug), saveMoaModels: (body: unknown) => saveMoaModels(body), setEnvVar: (key: string, value: string) => setEnvVar(key, value), - getHermesConfigRecord: () => getHermesConfigRecord(), - saveHermesConfig: (config: unknown) => saveHermesConfig(config), + getHermesConfigRecord: (profile?: null | string) => getHermesConfigRecord(profile), + saveHermesConfig: (config: unknown, profile?: null | string) => saveHermesConfig(config, profile), setApiRequestProfile: () => {} })) @@ -63,6 +64,7 @@ vi.mock('../hooks/use-on-profile-switch', () => ({ beforeEach(() => { _resetComposerModelSelectionsForTests() + $connection.set(null) $newChatRoute.set(null) $newChatProfile.set(null) $activeGatewayProfile.set('default') @@ -114,6 +116,77 @@ async function renderModelSettings(scopeProfile?: string, onMainModelChanged = v } describe('ModelSettings profile scope', () => { + it('reloads the form for same-named source owners A → B → A before saving', async () => { + const rehome = (connectionId: string) => { + setApiRequestConnection(connectionId) + $connection.set({ connectionId } as never) + $newChatRoute.set({ connectionId, profile: 'default' }) + } + + rehome('source-a') + getGlobalModelInfo.mockImplementation(async () => ({ + provider: getApiRequestConnection() === 'source-b' ? 'custom:b' : 'nous', + model: getApiRequestConnection() === 'source-b' ? 'model-b' : 'hermes-4' + })) + getGlobalModelOptions.mockImplementation(async () => ({ + providers: getApiRequestConnection() === 'source-b' + ? [{ name: 'Source B', slug: 'custom:b', models: ['model-b'], authenticated: true, api_url: 'https://b.invalid/v1' }] + : [{ name: 'Nous', slug: 'nous', models: ['hermes-4'], authenticated: true }] + })) + await renderModelSettings() + await screen.findByRole('button', { name: 'Apply' }) + await act(async () => rehome('source-b')) + await waitFor(() => expect(screen.getAllByRole('combobox')[0].textContent).toContain('Source B')) + fireEvent.click(screen.getByRole('button', { name: 'Apply' })) + await waitFor(() => expect(setModelAssignment).toHaveBeenCalledWith(expect.objectContaining({ + provider: 'custom:b', model: 'model-b', base_url: 'https://b.invalid/v1' + }))) + await act(async () => rehome('source-a')) + await waitFor(() => expect(screen.getAllByRole('combobox')[0].textContent).toContain('Nous')) + }) + + it.each(['source-b', 'local'])('keeps config GET/PUT routing and record authority together on %s', async destination => { + const previousBridge = window.hermesDesktop + + const api = vi.fn(async (request: { connectionId?: string; method?: string; body?: unknown }) => + request.method === 'PUT' ? { ok: true } : { + fixture_source: request.connectionId, + agent: { reasoning_effort: 'medium', service_tier: 'normal' }, + memory: { enabled: false }, governor: { semantic_memory_enabled: true } + }) + + window.hermesDesktop = { api } as never + getHermesConfigRecord.mockImplementation(readConfigOverBridge) + saveHermesConfig.mockImplementation(saveConfigOverBridge) + + const rehome = (connectionId: string) => { + setApiRequestConnection(connectionId) + $connection.set({ connectionId } as never) + $newChatRoute.set({ connectionId, profile: 'default' }) + } + + try { + rehome('source-a') + await renderModelSettings() + await screen.findByRole('switch') + await act(async () => rehome(destination)) + await waitFor(() => expect(api).toHaveBeenCalledWith(expect.objectContaining({ connectionId: destination, path: '/api/config' }))) + fireEvent.click(await screen.findByRole('switch')) + await waitFor(() => expect(api).toHaveBeenCalledWith(expect.objectContaining({ + connectionId: destination, method: 'PUT', body: { config: expect.objectContaining({ + fixture_source: destination, memory: { enabled: false }, governor: { semantic_memory_enabled: true } + }) + } + }))) + expect(getHermesConfigRecord).toHaveBeenCalledWith(undefined) + expect(saveHermesConfig).toHaveBeenCalledWith(expect.objectContaining({ fixture_source: destination }), undefined) + } finally { + window.hermesDesktop = previousBridge + getHermesConfigRecord.mockReset() + saveHermesConfig.mockReset() + } + }) + it('carries the original source/profile/target through an asynchronous save callback', async () => { const owner = { connectionId: 'source-a', profile: 'specialist', targetProfile: 'backend-a' } $newChatRoute.set(owner) @@ -128,7 +201,10 @@ describe('ModelSettings profile scope', () => { fireEvent.click(await screen.findByRole('button', { name: 'Apply' })) await waitFor(() => expect(setModelAssignment).toHaveBeenCalledOnce()) const originalSource = getApiRequestConnection() - setApiRequestConnection('source-b') + await act(async () => { + setApiRequestConnection('source-b') + $connection.set({ connectionId: 'source-b' } as never) + }) pending.resolve({ ok: true, provider: 'nous', model: 'hermes-4', gateway_tools: [] }) await waitFor(() => expect(changed).toHaveBeenCalledOnce()) expect(changed).toHaveBeenCalledWith( @@ -345,7 +421,8 @@ describe('ModelSettings', () => { await waitFor(() => expect(saveHermesConfig).toHaveBeenCalledWith( - expect.objectContaining({ agent: expect.objectContaining({ service_tier: 'fast' }) }) + expect.objectContaining({ agent: expect.objectContaining({ service_tier: 'fast' }) }), + undefined ) ) }) diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index c26af6000e3ab..34b0e3b692a7f 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -1,11 +1,13 @@ +import { useQuery, useQueryClient } from '@tanstack/react-query' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { beginMainModelSave, + composerOwnerKey, isMainModelSaveOriginCurrent, ownsMainModelSave } from '@/app/session/hooks/composer-model-selection-owner' -import type { OnMainModelChanged } from '@/app/session/hooks/composer-model-selection-owner' +import type { ComposerSelectionOwner, OnMainModelChanged } from '@/app/session/hooks/composer-model-selection-owner' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' @@ -15,6 +17,7 @@ import { getAuxiliaryModels, getGlobalModelInfo, getGlobalModelOptions, + getHermesConfigRecord, getMoaModels, getRecommendedDefaultModel, saveHermesConfig, @@ -24,6 +27,7 @@ import { } from '@/hermes' import type { AuxiliaryModelsResponse, + HermesConfigRecord, MoaConfigResponse, MoaModelSlot, ModelOptionProvider, @@ -37,7 +41,8 @@ import { setMainModelAssignment } from '@/store/cron-model-impact' import { notifyError } from '@/store/notifications' import { startManualLocalEndpoint, startManualOnboarding, startManualProviderOAuth } from '@/store/onboarding' -import { hermesConfigCacheWriter, invalidateHermesConfig, useHermesConfigRecord } from '../hooks/use-config-record' +import { requireCurrentModelOwner, useModelFormKey, useModelOwnerIsCurrent, useModelRequestOwner } from '../hooks/use-composer-model-owner' +import { HERMES_CONFIG_KEY } from '../hooks/use-config-record' import { useOnProfileSwitch } from '../hooks/use-on-profile-switch' import { CONTROL_TEXT } from './constants' @@ -195,6 +200,22 @@ interface ModelSettingsProps { } export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSettingsProps) { + const owner = useModelRequestOwner(scopeProfile) + const formKey = useModelFormKey(owner, scopeProfile) + + return +} + +function ModelSettingsForm({ onMainModelChanged, owner, scopeProfile }: ModelSettingsProps & { owner: ComposerSelectionOwner }) { + const isOwnerCurrent = useModelOwnerIsCurrent(owner, scopeProfile) + const ownerKey = composerOwnerKey(owner) + const queryClient = useQueryClient() + + const configKey = useMemo( + () => [...HERMES_CONFIG_KEY, ownerKey] as const, + [ownerKey] + ) + const { t } = useI18n() const m = t.settings.model const [loading, setLoading] = useState(true) @@ -207,10 +228,26 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting const [moa, setMoa] = useState(null) const [selectedMoaPreset, setSelectedMoaPreset] = useState('') const [newMoaPresetName, setNewMoaPresetName] = useState('') + // agent.* defaults round-trip through the shared config cache (read → write // back the whole record), so a save here shows in the MCP/model surfaces. - const { data: config } = useHermesConfigRecord(scopeProfile) - const setConfig = useMemo(() => hermesConfigCacheWriter(scopeProfile), [scopeProfile]) + const { data: config } = useQuery({ + queryKey: configKey, + queryFn: () => { + requireCurrentModelOwner(isOwnerCurrent) + + // Keep GET and PUT on the same legacy routing seam. In particular an + // ambient explicit 'local' tag must bypass legacy remote overrides. + return getHermesConfigRecord(scopeProfile) + }, + staleTime: 0 + }) + + const setConfig = useCallback( + (value: HermesConfigRecord) => queryClient.setQueryData(configKey, value), + [configKey, queryClient] + ) + const [applying, setApplying] = useState(false) const [editingAuxTask, setEditingAuxTask] = useState(null) const [auxDraft, setAuxDraft] = useState<{ model: string; provider: string }>({ model: '', provider: '' }) @@ -232,11 +269,15 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting // Every profile-scoped async here captures this and bails before writing back, // so a request in flight when the user switches profiles can't paint profile - // A's models/providers into profile B (or fire onMainModelChanged for A). + // A's models/providers into profile B. Confirmed saves still report origin. const profileEpoch = useRef(0) const refresh = useCallback( async ({ replaceSelection = false }: { replaceSelection?: boolean } = {}) => { + if (!isOwnerCurrent()) { + return + } + const epoch = profileEpoch.current setLoading(true) setError('') @@ -249,7 +290,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting getMoaModels(scopeProfile).catch(() => null) ]) - if (profileEpoch.current !== epoch) { + if (profileEpoch.current !== epoch || !isOwnerCurrent()) { return } @@ -273,18 +314,18 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting // The config record loads via its own shared query; a model switch can // change it server-side (aux slots), so nudge that cache to refetch. - void invalidateHermesConfig(scopeProfile) + void queryClient.invalidateQueries({ queryKey: configKey }) } catch (err) { - if (profileEpoch.current === epoch) { + if (profileEpoch.current === epoch && isOwnerCurrent()) { setError(err instanceof Error ? err.message : String(err)) } } finally { - if (profileEpoch.current === epoch) { + if (profileEpoch.current === epoch && isOwnerCurrent()) { setLoading(false) } } }, - [scopeProfile] + [configKey, isOwnerCurrent, queryClient, scopeProfile] ) useEffect(() => { @@ -406,20 +447,24 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting } moaSaveTimer.current = window.setTimeout(() => { + if (!isOwnerCurrent()) { + return + } + void saveMoaModels(next, scopeProfile) .then(saved => { - if (moaSaveGeneration.current === generation) { + if (moaSaveGeneration.current === generation && isOwnerCurrent()) { setMoa(saved) } }) .catch(err => { - if (moaSaveGeneration.current === generation) { + if (moaSaveGeneration.current === generation && isOwnerCurrent()) { setError(err instanceof Error ? err.message : String(err)) } }) }, 600) }, - [scopeProfile] + [isOwnerCurrent, scopeProfile] ) const updateMoaPreset = useCallback( @@ -475,9 +520,10 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setError('') try { + requireCurrentModelOwner(isOwnerCurrent) const saved = await saveMoaModels(next, scopeProfile) - if (profileEpoch.current !== epoch) { + if (profileEpoch.current !== epoch || !isOwnerCurrent()) { return } @@ -488,7 +534,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setApplying(false) } }, - [scopeProfile] + [isOwnerCurrent, scopeProfile] ) const auxiliaryTaskLabel = useCallback((key: string) => m.tasks[key]?.label ?? key, [m.tasks]) @@ -544,16 +590,18 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting const prev = config const next = setNested(config, key, value) - setConfig(next) try { + requireCurrentModelOwner(isOwnerCurrent) + setConfig(next) await saveHermesConfig(next, scopeProfile) + void queryClient.invalidateQueries({ queryKey: HERMES_CONFIG_KEY }) } catch (err) { setConfig(prev) notifyError(err, m.defaultsFailed) } }, - [config, m.defaultsFailed, scopeProfile, setConfig] + [config, isOwnerCurrent, m.defaultsFailed, queryClient, scopeProfile, setConfig] ) // Paste an API key for the selected `api_key` provider, persist it, then @@ -572,7 +620,13 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setError('') try { + requireCurrentModelOwner(isOwnerCurrent) await setEnvVar(keyEnv, apiKeyDraft.trim(), scopeProfile) + + if (!isOwnerCurrent()) { + return + } + setApiKeyDraft('') // Pick a sensible default for the freshly-activated provider (mirrors @@ -587,9 +641,13 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting nextModel = '' } + if (!isOwnerCurrent()) { + return + } + const options = await getGlobalModelOptions(undefined, scopeProfile) - if (profileEpoch.current !== epoch) { + if (profileEpoch.current !== epoch || !isOwnerCurrent()) { return } @@ -602,7 +660,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting } finally { setActivating(false) } - }, [apiKeyDraft, scopeProfile, selectedProviderRow]) + }, [apiKeyDraft, isOwnerCurrent, scopeProfile, selectedProviderRow]) // OAuth / external providers can't be activated with a pasted key — hand off // to the shared onboarding flow scoped to this provider's real sign-in. The @@ -613,7 +671,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting const rowSlug = selectedProviderRow?.slug.trim() ?? '' const slug = rowSlug || selectedProvider.trim() - if (!slug) { + if (!slug || !isOwnerCurrent()) { return } @@ -628,19 +686,20 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting // provider picker instead of deep-linking an unknown or stale slug. startManualOnboarding() } - }, [selectedProvider, selectedProviderRow]) + }, [isOwnerCurrent, selectedProvider, selectedProviderRow]) const applyMainModel = useCallback(async () => { if (!selectedProvider || !selectedModel) { return } - const epoch = profileEpoch.current - const origin = beginMainModelSave(scopeProfile) setApplying(true) setError('') try { + requireCurrentModelOwner(isOwnerCurrent) + const origin = beginMainModelSave(scopeProfile) + const result = await setMainModelAssignment( { model: selectedModel, @@ -648,29 +707,32 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting ...(selectedProviderRow?.api_url ? { base_url: selectedProviderRow.api_url } : {}) }, scopeProfile, - { ownsOrigin: () => isMainModelSaveOriginCurrent(origin, scopeProfile) } + { ownsOrigin: () => isOwnerCurrent() && isMainModelSaveOriginCurrent(origin, scopeProfile) } ) - if (profileEpoch.current !== epoch || !ownsMainModelSave(origin)) { + if (!ownsMainModelSave(origin)) { return } const provider = result.provider || selectedProvider const model = result.model || selectedModel - setMainModel({ provider, model }) - setSwitchStaleAux(result.stale_aux ?? []) - // The callback carries origin scope; controls decide whether this // owner may paint the foreground draft or only its own default cache. onMainModelChanged?.({ ...origin, provider, model }) + if (!isOwnerCurrent()) { + return + } + + setMainModel({ provider, model }) + setSwitchStaleAux(result.stale_aux ?? []) await refresh() } catch (err) { setError(err instanceof Error ? err.message : String(err)) } finally { setApplying(false) } - }, [onMainModelChanged, refresh, scopeProfile, selectedModel, selectedProvider, selectedProviderRow]) + }, [isOwnerCurrent, onMainModelChanged, refresh, scopeProfile, selectedModel, selectedProvider, selectedProviderRow]) // Sibling of the applyMainModel endpoint passthrough (#65254): auxiliary // assignments targeting a user-defined provider must carry that provider's @@ -696,6 +758,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setError('') try { + requireCurrentModelOwner(isOwnerCurrent) await setModelAssignment( { model: mainModel.model, @@ -713,7 +776,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setApplying(false) } }, - [endpointForProvider, mainModel, refresh, scopeProfile] + [endpointForProvider, isOwnerCurrent, mainModel, refresh, scopeProfile] ) const applyAuxiliaryDraft = useCallback( @@ -726,6 +789,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setError('') try { + requireCurrentModelOwner(isOwnerCurrent) await setModelAssignment( { model: auxDraft.model, @@ -736,6 +800,11 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting }, scopeProfile ) + + if (!isOwnerCurrent()) { + return + } + setEditingAuxTask(null) await refresh() } catch (err) { @@ -744,7 +813,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setApplying(false) } }, - [auxDraft, endpointForProvider, refresh, scopeProfile] + [auxDraft, endpointForProvider, isOwnerCurrent, refresh, scopeProfile] ) const beginAuxiliaryEdit = useCallback( @@ -770,6 +839,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting setError('') try { + requireCurrentModelOwner(isOwnerCurrent) await setModelAssignment( { model: mainModel.model, @@ -779,6 +849,11 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting }, scopeProfile ) + + if (!isOwnerCurrent()) { + return + } + setSwitchStaleAux([]) await refresh() } catch (err) { @@ -786,7 +861,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSetting } finally { setApplying(false) } - }, [mainModel, refresh, scopeProfile]) + }, [isOwnerCurrent, mainModel, refresh, scopeProfile]) if (loading && !mainModel) { return diff --git a/apps/desktop/src/app/shell/model-menu-panel-owner.test.tsx b/apps/desktop/src/app/shell/model-menu-panel-owner.test.tsx new file mode 100644 index 0000000000000..66cfdec29e369 --- /dev/null +++ b/apps/desktop/src/app/shell/model-menu-panel-owner.test.tsx @@ -0,0 +1,129 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +import { setApiRequestConnection, setApiRequestProfile } from '@/api/client' +import type * as Gateway from '@/store/gateway' +import { $activeGatewayProfile, $newChatConnectionId, $newChatProfile, $newChatRoute } from '@/store/profile' +import { + $activeSessionId, $currentModel, $currentProvider, $sessions, _resetComposerModelSelectionsForTests, + _resetSessionOwnerHintsForTests, captureComposerModelSelection, recordComposerModelSelection, + setComposerModelSelectionOwner, setSessionOwnerHint +} from '@/store/session' +import { knownOwnerForSession } from '@/store/session-states' +import type { ModelOptionsResponse } from '@/types/hermes' + +import { deferred } from '../../test/deferred' + +import { ModelMenuPanel } from './model-menu-panel' + +const calls = vi.hoisted(() => ({ agent: vi.fn(), rest: vi.fn() })) +vi.mock('@/store/gateway', async original => ({ + ...await original(), + requestGatewayForAgent: (...args: unknown[]) => calls.agent(...args) +})) +vi.mock('@/hermes', () => ({ getGlobalModelOptions: (...args: unknown[]) => calls.rest(...args), setApiRequestProfile: vi.fn() })) +// Inspect the real producer's current pair without involving dropdown portals. +vi.mock('./model-catalog-menu', () => ({ + ModelMenuCloseContext: {}, + ModelCatalogMenu: ({ controller }: { controller: { current: { model: string; provider: string } } }) => + {controller.current.model}|{controller.current.provider} +})) + +const ownerB = { connectionId: 'source-b', profile: 'same-name', targetProfile: 'backend-b' } +const options = (model: string): ModelOptionsResponse => ({ model, provider: 'custom:b', providers: [{ name: 'B', slug: 'custom:b', models: [model] }] }) + +beforeEach(() => { + vi.clearAllMocks() + calls.agent.mockReset().mockResolvedValue(options('catalog-b')) + calls.rest.mockReset().mockResolvedValue(options('rest-b')) + _resetComposerModelSelectionsForTests() + _resetSessionOwnerHintsForTests() + $sessions.set([]) + $activeSessionId.set(null) + $currentModel.set('ambient-a') + $currentProvider.set('provider-a') + setApiRequestConnection('source-a') + setApiRequestProfile('default') + $activeGatewayProfile.set('default') + $newChatRoute.set(ownerB) + $newChatProfile.set(ownerB.profile) + $newChatConnectionId.set(ownerB.connectionId) + setComposerModelSelectionOwner(ownerB) +}) +afterEach(() => { cleanup(); setApiRequestConnection(null); setApiRequestProfile('default'); $newChatRoute.set(null); $newChatProfile.set(null); $newChatConnectionId.set(null); $sessions.set([]); _resetSessionOwnerHintsForTests() }) + +function mount(rpcUnavailable?: 'empty' | 'rejected') { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const ambient = vi.fn(async () => options('ambient-catalog-a')) + + if (rpcUnavailable === 'empty') { + ambient.mockResolvedValue({ providers: [] }) + } else if (rpcUnavailable === 'rejected') { + ambient.mockRejectedValue(new Error('Offline catalog RPC')) + } + + const view = render() + + return { client, ambient, view } +} + +it('routes a fresh pending/failed B draft catalog to B while the foreground gateway stays on A', async () => { + mount() + await waitFor(() => expect(calls.agent).toHaveBeenCalledWith('source-b', 'same-name', 'model.options', { profile: 'backend-b', explicit_only: true })) + await waitFor(() => expect(screen.getByTestId('current').textContent).toBe('catalog-b|custom:b')) +}) + +it('retains a valid scalar draft receipt beside a stale complete catalog instead of borrowing ambient A', async () => { + recordComposerModelSelection(captureComposerModelSelection(ownerB), { model: 'scalar-b', provider: '', source: 'default' }) + mount() + await waitFor(() => expect(screen.getByTestId('current').textContent).toBe('scalar-b|')) +}) + +it('keeps late B catalog replies in B cache after a B → A draft rehome', async () => { + const pending = deferred() + calls.agent.mockReturnValueOnce(pending.promise).mockResolvedValue(options('catalog-a')) + const { client } = mount() + await waitFor(() => expect(calls.agent).toHaveBeenCalledOnce()) + await act(async () => { + const ownerA = { connectionId: 'source-a', profile: 'same-name', targetProfile: 'backend-a' } + $newChatRoute.set(ownerA) + $newChatConnectionId.set(ownerA.connectionId) + setComposerModelSelectionOwner(ownerA) + }) + await waitFor(() => expect(screen.getByTestId('current').textContent).toBe('catalog-a|custom:b')) + await act(async () => pending.resolve(options('late-b'))) + expect(screen.getByTestId('current').textContent).toBe('catalog-a|custom:b') + expect(client.getQueryData(['model-options', 'source-b::backend-b', 'global'])).toEqual(options('late-b')) +}) + +it('recovers a failed B RPC through the same captured B REST target', async () => { + calls.agent.mockRejectedValueOnce(new Error('B route unavailable')) + mount() + await waitFor(() => expect(calls.rest).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: 'source-b', profile: 'backend-b' })) + await waitFor(() => expect(screen.getByTestId('current').textContent).toBe('rest-b|custom:b')) +}) + +it.each([ + ['legacy', 'rejected'], ['legacy', 'empty'], ['local', 'rejected'], ['local', 'empty'] +] as const)('preserves a live %s owner through %s RPC recovery', async (source, rpcUnavailable) => { + const runtimeId = 'owned-runtime' + const connectionId = source === 'legacy' ? null : 'local' + $sessions.set([{ id: runtimeId, profile: 'default' }] as never) + + if (connectionId) { + setSessionOwnerHint(runtimeId, { connectionId, profile: 'default', mode: 'local' }) + } + + expect(knownOwnerForSession(runtimeId)).toEqual(connectionId ? { connectionId, profile: 'default', mode: 'local' } : 'default') + $activeSessionId.set(runtimeId) + $currentModel.set('') + $currentProvider.set('') + calls.rest.mockImplementation(async (_opts, scope) => options(scope.connectionId === 'local' ? 'local-b' : 'legacy-a')) + const { client, ambient } = mount(rpcUnavailable) + await waitFor(() => expect(calls.rest).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId, profile: 'default' })) + const model = source === 'legacy' ? 'legacy-a' : 'local-b' + await waitFor(() => expect(screen.getByTestId('current').textContent).toBe(`${model}|custom:b`)) + expect(ambient).toHaveBeenCalledWith('model.options', { profile: 'default', session_id: runtimeId, explicit_only: true }) + expect(client.getQueryData(['model-options', source === 'legacy' ? 'default' : 'local::default', runtimeId])).toEqual(options(model)) +}) 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 ae9d1d4be77d9..d964d105816a6 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.test.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.test.tsx @@ -86,15 +86,17 @@ function renderPanel(onSelectModel = vi.fn()) { it.each(['legacy-local', 'remote-target'])('loads the known session owner catalog while ambient source differs (%s)', async kind => { setApiRequestConnection('ambient-remote') + if (kind === 'legacy-local') { $sessions.set([{ id: 'runtime-1', profile: 'local-specialist' }] as never) } else { setSessionOwnerHint('runtime-1', { connectionId: 'owner-remote', profile: 'logical', targetProfile: 'backend-target', mode: 'remote' }) } + try { renderPanel() await vi.waitFor(() => expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { - connectionId: kind === 'legacy-local' ? 'local' : 'owner-remote', + connectionId: kind === 'legacy-local' ? null : 'owner-remote', profile: kind === 'legacy-local' ? 'local-specialist' : 'backend-target' })) } finally { diff --git a/apps/desktop/src/app/shell/model-menu-panel.tsx b/apps/desktop/src/app/shell/model-menu-panel.tsx index 2a84da56097a1..00e1a8dbb8786 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.tsx @@ -12,7 +12,7 @@ import { modelOptionsQueryKey, requestModelOptions, selectionUnavailable } from import { currentPickerSelection } from '@/lib/model-status-label' import { DEFAULT_REASONING_EFFORT } from '@/lib/reasoning-effort' import { cn } from '@/lib/utils' -import { activeGatewayConnectionId } from '@/store/gateway' +import { activeGatewayConnectionId, requestGatewayForAgent } from '@/store/gateway' import { $modelPresets, applyModelPreset, modelPresetKey, setModelPreset } from '@/store/model-presets' import { $visibleModels } from '@/store/model-visibility' import { notifyError } from '@/store/notifications' @@ -23,6 +23,7 @@ import { $defaultReasoningEffort, $selectedStoredSessionId, beginRuntimeOptionIntent, + getComposerModelSelection, markComposerSelectionManual, ownsRuntimeOptionIntent, setCurrentFastMode, @@ -31,6 +32,8 @@ import { import { $sessionStates, knownOwnerForSession, sessionTileDelegate } from '@/store/session-states' import type { ModelOptionsResponse } from '@/types/hermes' +import { useDraftComposerOwner } from '../hooks/use-composer-model-owner' + import { ModelCatalogMenu, type ModelMenuController } from './model-catalog-menu' export { ModelMenuCloseContext } from './model-catalog-menu' @@ -66,8 +69,13 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re const view = useSessionView() const activeSessionId = useStore(view.$runtimeId) const owner = knownOwnerForSession(activeSessionId) - const connectionId = owner && typeof owner === 'object' ? owner.connectionId : owner ? 'local' : activeGatewayConnectionId() - const catalogProfile = (typeof owner === 'string' ? owner : (owner?.targetProfile || owner?.profile)) || profile + const draftOwner = useDraftComposerOwner() + const connectionId = owner && typeof owner === 'object' ? owner.connectionId : owner ? null : !activeSessionId ? draftOwner.connectionId : activeGatewayConnectionId() + const catalogProfile = (typeof owner === 'string' ? owner : (owner?.targetProfile || owner?.profile)) || (!activeSessionId ? draftOwner.targetProfile || draftOwner.profile : profile) + + const catalogRequest = !activeSessionId + ? (method: string, params?: Record) => requestGatewayForAgent(draftOwner.connectionId, draftOwner.profile, method, params) + : requestGateway const unconfirmedOptions = useStore( useMemo( @@ -82,6 +90,7 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re const currentFastMode = useStore(view.$fast) const currentModel = useStore(view.$model) const currentProvider = useStore(view.$provider) + const draftSelection = !activeSessionId ? getComposerModelSelection(draftOwner) : null const currentReasoningEffort = useStore(view.$reasoningEffort) const modelPresets = useStore($modelPresets) const defaultEffort = useStore($defaultReasoningEffort) || DEFAULT_REASONING_EFFORT @@ -96,11 +105,13 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re const modelOptions = useQuery({ queryKey: modelOptionsQueryKey(catalogProfile, activeSessionId, connectionId), queryFn: (): Promise => - requestModelOptions({ connectionId, gateway, profile: catalogProfile, request: requestGateway, sessionId: activeSessionId }) + requestModelOptions({ connectionId, gateway, profile: catalogProfile, request: catalogRequest, sessionId: activeSessionId }) }) const { model: optionsModel, provider: optionsProvider } = currentPickerSelection( - { model: currentModel, provider: currentProvider }, + activeSessionId + ? { model: currentModel, provider: currentProvider } + : { model: draftSelection?.model || '', provider: draftSelection?.provider || '', authoritative: Boolean(draftSelection) }, modelOptions.data ) @@ -123,7 +134,7 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re gateway, profile: catalogProfile, refresh: true, - request: requestGateway, + request: catalogRequest, sessionId: activeSessionId }) @@ -329,7 +340,7 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re gateway={gateway} includeMoa profile={catalogProfile} - request={requestGateway} + request={catalogRequest} sessionId={activeSessionId} /> ) diff --git a/apps/desktop/src/components/model-picker.test.tsx b/apps/desktop/src/components/model-picker.test.tsx index f6bf30e3696d6..5b646e4d06cbe 100644 --- a/apps/desktop/src/components/model-picker.test.tsx +++ b/apps/desktop/src/components/model-picker.test.tsx @@ -17,14 +17,28 @@ afterEach(() => { cleanup(); vi.clearAllMocks() }) const payload = (model: string) => ({ providers: [{ models: [model], name: 'Target provider', slug: 'target' }] }) +it('retains a confirmed scalar selection instead of marking a stale catalog model current', async () => { + vi.mocked(getGlobalModelOptions).mockResolvedValue({ ...payload('catalog-model'), model: 'catalog-model', provider: 'target' }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + + const view = render( + + ) + + const row = await view.findByText('catalog-model') + expect(row.closest('[cmdk-item]')?.className).not.toContain('bg-primary text-primary-foreground') +}) + it('sends the modal target profile and explains a missing session selection without changing it', async () => { vi.mocked(getGlobalModelOptions).mockResolvedValue(payload('model-b')) const select = vi.fn() const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const view = render( ) + await view.findByText(/selected provider or model is unavailable in this profile/i) await view.findByText(/session keeps its own model selection/i) expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: 'owner', profile: 'specialist' }) @@ -37,10 +51,12 @@ it('keeps a late catalog reply in its original source and profile cache', async .mockImplementationOnce(() => new Promise(resolve => { resolveA = resolve })) .mockResolvedValueOnce(payload('model-b')) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const picker = (connectionId: string, profile: string) => + const view = render(picker('source-a', 'target-a')) await vi.waitFor(() => expect(getGlobalModelOptions).toHaveBeenCalledTimes(1)) view.rerender(picker('source-b', 'target-b')) @@ -54,9 +70,11 @@ it('visibility and picker subscribers share the same target catalog instead of a const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) const request = vi.fn(async (_method: string, params?: Record) => payload(params?.profile === 'target' ? 'target-model' : 'source-model')) const gateway = { request } as never + const view = render( ) + await vi.waitFor(() => expect(client.getQueryData(modelOptionsQueryKey('target'))).toEqual(payload('target-model'))) view.rerender( void profile?: string /** @@ -48,6 +50,7 @@ export function ModelPickerDialog({ sessionId, currentModel, currentProvider, + selectionIsAuthoritative = false, onSelect, profile = 'default', contentClassName @@ -70,7 +73,7 @@ export function ModelPickerDialog({ const providers = modelOptions.data?.providers ?? [] const { model: optionsModel, provider: optionsProvider } = currentPickerSelection( - { model: currentModel, provider: currentProvider }, + { model: currentModel, provider: currentProvider, authoritative: selectionIsAuthoritative }, modelOptions.data ) diff --git a/apps/desktop/src/lib/model-options.test.ts b/apps/desktop/src/lib/model-options.test.ts index fbdc4e35a3d35..2efb140bb17be 100644 --- a/apps/desktop/src/lib/model-options.test.ts +++ b/apps/desktop/src/lib/model-options.test.ts @@ -1,7 +1,16 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { cleanup, render, screen, waitFor } from '@testing-library/react' +import { createElement } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { setApiRequestConnection, setApiRequestProfile } from '@/api/client' +import { getGlobalModelOptions as readGlobalModelOptions } from '@/api/models' +import { ModelPickerDialog } from '@/components/model-picker' +import type { HermesApiRequest } from '@/global' import { getGlobalModelOptions } from '@/hermes' +import type { ModelOptionsResponse } from '@/types/hermes' + +import { deferred } from '../test/deferred' import { modelOptionsQueryKey, @@ -15,6 +24,24 @@ vi.mock('@/hermes', () => ({ getGlobalModelOptions: vi.fn(() => Promise.resolve(globalOptions)) })) +// Electron helpers have a separate compile project. Load their real pure +// exports for this bridge regression without pulling that project into the +// renderer's strict typecheck or changing Electron source/compiler settings. +const { apiRequestRegistryConnectionId, resolveProfileApiRequest } = await vi.importActual<{ + apiRequestRegistryConnectionId: (request: HermesApiRequest) => null | string + resolveProfileApiRequest: (profile: unknown, path: string, opts: Record) => { backendProfile: null | string } +}>('../../electron/connection-config') + +const { normalizeRegistry, resolvedConnectionId, resolveRegistryLocalRoute } = await vi.importActual<{ + normalizeRegistry: (input: unknown) => unknown + resolvedConnectionId: (registry: unknown, descriptor: Record) => null | string + resolveRegistryLocalRoute: (profile: unknown, opts: { globalRemote: boolean }) => { delegate: boolean; poolKey: string } +}>('../../electron/connection-registry') + +const { resolveDesktopRemoteRoute } = await vi.importActual<{ + resolveDesktopRemoteRoute: (input: Record) => null | { connectionId?: string; kind: string; source: string; url?: string } +}>('../../electron/desktop-remote-route') + describe('requestModelOptions', () => { afterEach(() => { vi.clearAllMocks() @@ -60,7 +87,7 @@ describe('requestModelOptions', () => { provider: 'hermes-local' }) - expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: 'local', profile: 'default' }) + expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: null, profile: 'default' }) }) it('recovers through profile-scoped REST when the gateway catalog request fails', async () => { @@ -79,7 +106,7 @@ describe('requestModelOptions', () => { await expect(requestModelOptions({ gateway: gateway as never, sessionId: 'session-1' })).resolves.toEqual( restPayload ) - expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: 'local', profile: 'default' }) + expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: null, profile: 'default' }) }) it('preserves the gateway error when its REST recovery path also fails', async () => { @@ -117,13 +144,13 @@ describe('requestModelOptions', () => { refresh: true, session_id: 'session-1' }) - expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true, refresh: true }, { connectionId: 'local', profile: 'default' }) + expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true, refresh: true }, { connectionId: null, profile: 'default' }) }) it('falls back to REST when no gateway is connected', async () => { await requestModelOptions({ refresh: true }) - expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true, refresh: true }, { connectionId: 'local', profile: 'default' }) + expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true, refresh: true }, { connectionId: null, profile: 'default' }) }) it('prefers an owner-routed request over the ambient gateway socket', async () => { @@ -168,7 +195,7 @@ describe('requestModelOptions', () => { vi.mocked(getGlobalModelOptions).mockResolvedValueOnce(restPayload) await expect(requestModelOptions({ profile: 'berry', request, sessionId: 'tile-1' })).resolves.toEqual(restPayload) - expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: 'local', profile: 'berry' }) + expect(getGlobalModelOptions).toHaveBeenCalledWith({ explicitOnly: true }, { connectionId: null, profile: 'berry' }) }) it('freezes source and target before a late gateway failure', async () => { @@ -205,7 +232,7 @@ describe('modelOptionsQueryKey', () => { it('isolates identically named profiles and sessions on different sources', () => { expect(modelOptionsQueryKey('target', 'session', 'remote-a')).not.toEqual(modelOptionsQueryKey('target', 'session', 'remote-b')) - expect(modelOptionsQueryKey('target', 'session', 'local')).toEqual(['model-options', 'target', 'session']) + expect(modelOptionsQueryKey('target', 'session', 'local')).toEqual(['model-options', 'local::target', 'session']) }) }) @@ -226,3 +253,154 @@ describe('selectionUnavailable', () => { expect(selectionUnavailable(undefined, 'ollama-launch', 'model-a')).toBe(false) }) }) + +describe('legacy-null and explicit-local catalog authority', () => { + const env = { url: 'https://legacy-a.invalid', token: 'inert-routing-fixture' } + + const registry = normalizeRegistry({ + version: 2, + primary: 'local', + connections: [{ id: 'local', kind: 'local', label: 'This device' }] + }) + + const catalog = (model: string): ModelOptionsResponse => ({ + model, + provider: model + '-provider', + providers: [{ slug: model + '-provider', name: model + ' provider', models: [model] }] + }) + + const routed: { request: HermesApiRequest; target: string }[] = [] + + // main's API branch uses this real tag resolver. Its legacy branch uses + // resolveProfileApiRequest/resolveDesktopRemoteRoute; registry local uses + // resolveRegistryLocalRoute. Exercise those pure decisions without main, + // spawning, network, or a live connection/configuration. + async function bridge(request: HermesApiRequest): Promise { + expect(request.path).toMatch(/^\/api\/model\/options\?/) + const connectionId = apiRequestRegistryConnectionId(request) + + if (connectionId === null) { + const route = resolveProfileApiRequest(request.profile, request.path, { + primaryProfile: 'default', globalRemote: true + }) + + const remote = resolveDesktopRemoteRoute({ + config: { mode: 'local' }, env, profile: route.backendProfile, registry + }) + + expect(route.backendProfile).toBeNull() + expect(remote).toMatchObject({ kind: 'remote', source: 'env', url: env.url }) + expect(remote?.connectionId).toBeUndefined() + expect(resolvedConnectionId(registry, { + mode: 'remote', remoteKind: 'url', baseUrl: env.url, token: env.token, authMode: 'token' + })).toBeNull() + routed.push({ request, target: env.url }) + + return catalog('model-legacy-a') as T + } + + expect(connectionId).toBe('local') + const local = resolveRegistryLocalRoute(request.profile, { globalRemote: Boolean(env.url) }) + expect(local).toEqual({ delegate: false, poolKey: 'conn:local::default' }) + routed.push({ request, target: local.poolKey }) + + return catalog('model-local-b') as T + } + + beforeEach(() => { + routed.length = 0 + setApiRequestConnection(null) + setApiRequestProfile('default') + vi.mocked(getGlobalModelOptions).mockReset().mockImplementation(readGlobalModelOptions) + vi.stubGlobal('hermesDesktop', { api: vi.fn(bridge) }) + }) + + afterEach(() => { + cleanup() + vi.restoreAllMocks() + vi.unstubAllGlobals() + vi.mocked(getGlobalModelOptions).mockReset().mockResolvedValue(globalOptions) + setApiRequestConnection(null) + setApiRequestProfile(null) + }) + + it('distinguishes keys for physically separate legacy environment remote A and forced-local B', async () => { + const remote = resolveDesktopRemoteRoute({ config: { mode: 'local' }, env, profile: 'default', registry }) + expect(remote).toMatchObject({ kind: 'remote', source: 'env', url: env.url }) + expect(remote?.connectionId).toBeUndefined() + expect(resolveRegistryLocalRoute('default', { globalRemote: Boolean(env.url) })).toEqual({ + delegate: false, poolKey: 'conn:local::default' + }) + expect(modelOptionsQueryKey('default', null, null)).not.toEqual(modelOptionsQueryKey('default', null, 'local')) + }) + + it.each(['rejected', 'empty'])('recovers a %s legacy RPC through the real API and legacy bridge route', async failure => { + const request = vi.fn(async () => { + if (failure === 'rejected') { + throw new Error('inert legacy RPC failure') + } + + return { providers: [] } + }) + + await expect(requestModelOptions({ connectionId: null, profile: 'default', request: request as never })).resolves.toEqual(catalog('model-legacy-a')) + expect(request).toHaveBeenCalledWith('model.options', { explicit_only: true, profile: 'default' }) + expect(routed).toEqual([{ request: expect.objectContaining({ profile: 'default' }), target: env.url }]) + expect(routed[0].request).not.toHaveProperty('connectionId') + }) + + it('keeps forced-local B recovery registry-pinned while the legacy primary resolves to remote A', async () => { + const request = vi.fn(async () => { throw new Error('inert local RPC failure') }) + await expect(requestModelOptions({ connectionId: 'local', profile: 'default', request })).resolves.toEqual(catalog('model-local-b')) + expect(routed).toEqual([{ + request: expect.objectContaining({ connectionId: 'local', profile: 'default' }), target: 'conn:local::default' + }]) + }) + + it('retains captured legacy A when its RPC fails after the foreground moves to named source C', async () => { + const pending = deferred() + const request = vi.fn(() => pending.promise) + const result = requestModelOptions({ request: request as never }) + setApiRequestConnection('source-c') + setApiRequestProfile('other-profile') + pending.reject(new Error('inert late legacy RPC failure')) + await expect(result).resolves.toEqual(catalog('model-legacy-a')) + expect(routed[0]).toMatchObject({ request: { profile: 'default' }, target: env.url }) + expect(routed[0].request).not.toHaveProperty('connectionId') + }) + + it('keeps a late legacy A catalog in its own cache without painting the foreground local B picker', async () => { + Element.prototype.scrollIntoView = vi.fn() + vi.stubGlobal('ResizeObserver', class { observe() {} unobserve() {} disconnect() {} }) + const pending = deferred() + const requestA = vi.fn(() => pending.promise) + const requestB = vi.fn(async () => { throw new Error('inert local RPC failure') }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + + const picker = (connectionId: null | string) => createElement(QueryClientProvider, { client }, + createElement(ModelPickerDialog, { + connectionId, profile: 'default', request: (connectionId === null ? requestA : requestB) as never, + open: true, onOpenChange: vi.fn(), onSelect: vi.fn(), currentModel: '', currentProvider: '' + })) + + const view = render(picker(null)) + + try { + await waitFor(() => expect(requestA).toHaveBeenCalledOnce()) + setApiRequestConnection('local') + view.rerender(picker('local')) + await screen.findByText('model-local-b') + setApiRequestConnection('source-c') + setApiRequestProfile('other-profile') + pending.reject(new Error('inert late legacy RPC failure')) + await waitFor(() => expect(client.getQueryData(modelOptionsQueryKey('default', null, null))).toMatchObject({ model: 'model-legacy-a' })) + expect(client.getQueryData(modelOptionsQueryKey('default', null, 'local'))).toMatchObject({ model: 'model-local-b' }) + expect(screen.queryByText('model-legacy-a')).toBeNull() + expect(screen.getByText('model-local-b')).toBeTruthy() + } finally { + pending.resolve({ providers: [] }) + view.unmount() + client.clear() + } + }) +}) diff --git a/apps/desktop/src/lib/model-options.ts b/apps/desktop/src/lib/model-options.ts index 7b76cf9860c6a..7eb0c9b0e5b24 100644 --- a/apps/desktop/src/lib/model-options.ts +++ b/apps/desktop/src/lib/model-options.ts @@ -42,7 +42,7 @@ export function modelOptionsQueryKey( ) { const profileKey = (profile ?? '').trim() || 'default' - const sourceKey = connectionId && connectionId !== 'local' ? `${connectionId}::${profileKey}` : profileKey + const sourceKey = connectionId ? `${connectionId}::${profileKey}` : profileKey return ['model-options', sourceKey, sessionId || 'global'] as const } @@ -57,6 +57,7 @@ function restModelOptions( profile: ProfileScope ): Promise { const opts = { explicitOnly, ...(refresh ? { refresh: true } : {}) } + return getGlobalModelOptions(opts, profile) } @@ -71,7 +72,7 @@ export async function requestModelOptions({ }: ModelOptionsRequest): Promise { // Capture the owner before either async leg; foreground source changes must // not redirect a late REST recovery into another profile or connection. - const scope = { connectionId: connectionId || 'local', profile: profile ?? getApiRequestProfile() ?? 'default' } + const scope = { connectionId, profile: profile ?? getApiRequestProfile() ?? 'default' } const dispatch = request ?? (gateway ? gateway.request.bind(gateway) : null) if (dispatch) { diff --git a/apps/desktop/src/lib/model-status-label.test.ts b/apps/desktop/src/lib/model-status-label.test.ts index d1ad06fb73ee2..22743a5edb1dd 100644 --- a/apps/desktop/src/lib/model-status-label.test.ts +++ b/apps/desktop/src/lib/model-status-label.test.ts @@ -66,6 +66,14 @@ describe('model-status-label', () => { expect(currentPickerSelection({ model: 'opus', provider: '' }, options)).toEqual(options) }) + it('keeps an authoritative scalar receipt without inventing its provider from a stale catalog', () => { + expect(currentPickerSelection({ model: 'scalar', provider: '', authoritative: true }, options)).toEqual({ model: 'scalar', provider: '' }) + }) + + it('still hydrates an empty pair even when a caller marks it authoritative', () => { + expect(currentPickerSelection({ model: '', provider: '', authoritative: true }, options)).toEqual(options) + }) + it('falls back to the store while options are still loading', () => { expect(currentPickerSelection(store, undefined)).toEqual(store) }) diff --git a/apps/desktop/src/lib/model-status-label.ts b/apps/desktop/src/lib/model-status-label.ts index 5dde20746f8e7..0cf97b9a50e13 100644 --- a/apps/desktop/src/lib/model-status-label.ts +++ b/apps/desktop/src/lib/model-status-label.ts @@ -3,10 +3,11 @@ import { DEFAULT_REASONING_EFFORT, reasoningEffortLabel } from '@/lib/reasoning- /** Which model/provider pair a picker should mark "current". SessionView state * also drives the composer label, so a complete pair there wins over an older * `model.options` response. During initial hydration (or pre-session startup), - * options remain the fallback. Pick one complete pair before mixing fields so + * options remain the fallback. A valid owned scalar receipt is authoritative + * even before backend provider resolution. Pick one pair before mixing fields so * a model is never shown under a different provider. */ export function currentPickerSelection( - store: { model: string; provider: string }, + store: { model: string; provider: string; authoritative?: boolean }, options?: { model?: string; provider?: string } ): { model: string; provider: string } { const storeSelection = { @@ -19,7 +20,7 @@ export function currentPickerSelection( provider: String(options?.provider || '') } - if (storeSelection.model && storeSelection.provider) { + if (storeSelection.model && (storeSelection.provider || store.authoritative)) { return storeSelection }