From 12908e2ccdf54db8a1e8cd3e3e283804e53a4fdf Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Tue, 2 Jun 2026 06:10:47 +0530 Subject: [PATCH] fix(ai-settings): reset orphaned local-runtime routing when the runtime is disabled (#3167) Co-authored-by: Claude Opus 4.8 (1M context) --- .../components/settings/panels/AIPanel.tsx | 76 ++++++++----- .../panels/__tests__/AIPanel.test.tsx | 52 +++++++++ .../panels/__tests__/aiRouting.test.ts | 102 ++++++++++++++++++ .../components/settings/panels/aiRouting.ts | 51 +++++++++ 4 files changed, 253 insertions(+), 28 deletions(-) create mode 100644 app/src/components/settings/panels/__tests__/aiRouting.test.ts create mode 100644 app/src/components/settings/panels/aiRouting.ts diff --git a/app/src/components/settings/panels/AIPanel.tsx b/app/src/components/settings/panels/AIPanel.tsx index e92b653d4..43a244e88 100644 --- a/app/src/components/settings/panels/AIPanel.tsx +++ b/app/src/components/settings/panels/AIPanel.tsx @@ -53,6 +53,7 @@ import { import { ConfirmationModal } from '../../intelligence/ConfirmationModal'; import SettingsHeader from '../components/SettingsHeader'; import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; +import { routingWithProviderRemoved } from './aiRouting'; import { authStyleForBuiltinCloudProvider, BUILTIN_CLOUD_PROVIDER_META, @@ -67,7 +68,7 @@ import { useReembedBackfillModal } from './useReembedBackfillModal'; // Types // ───────────────────────────────────────────────────────────────────────────── -type CloudProvider = { +export type CloudProvider = { id: string; slug: string; label: string; @@ -92,7 +93,7 @@ type WorkloadId = type WorkloadGroup = 'chat' | 'background'; -type ProviderRef = +export type ProviderRef = | { kind: 'openhuman' } | { kind: 'default' } | { kind: 'cloud'; providerSlug: string; model: string; temperature?: number | null } @@ -100,7 +101,7 @@ type ProviderRef = type Workload = { id: WorkloadId; group: WorkloadGroup; label: string; description: string }; -type RoutingMap = Record; +export type RoutingMap = Record; type RoutingMode = 'managed' | 'own' | 'custom'; const ROUTING_WORKLOAD_IDS: WorkloadId[] = [ 'chat', @@ -626,6 +627,15 @@ const ProviderKeyDialog = ({ } setError(null); + // A provider credential is being saved. This adds/updates a `cloudProviders` + // entry only — it does NOT change the workload routing map, so routing is + // unchanged afterwards (see inferRoutingMode). Logged for routing diagnostics. + console.debug('[ai-settings][routing] saving provider credential', { + slug, + local_runtime: isLocalRuntime, + kind: isLocalRuntime ? 'endpoint' : 'apiKey', + }); + setPhase('saving'); try { await onSubmit(trimmed); @@ -2724,7 +2734,26 @@ const AIPanel = ({ embedded = false }: AIPanelProps = {}) => { const chatRows = WORKLOADS.filter(w => w.group === 'chat'); const bgRows = WORKLOADS.filter(w => w.group === 'background'); - const inferredRoutingMode = useMemo(() => inferRoutingMode(draft.routing), [draft.routing]); + const inferredRoutingMode = useMemo(() => { + const mode = inferRoutingMode(draft.routing); + // Routing mode is derived purely from the workload routing map, not from the + // set of configured providers: saving a provider key only adds a + // `cloudProviders` entry, it does not rewrite `routing`. So "managed while a + // provider key is configured" is an expected state — the user must pick a + // route to actually use their provider. Surfaced for support diagnostics + // (the recurring "my key is added but not used" question). + const configuredWithKey = draft.cloudProviders.filter(p => p.maskedKey.startsWith('••••')); + console.debug('[ai-settings][routing] inferred mode', { + mode, + routing: ROUTING_WORKLOAD_IDS.map(id => `${id}:${draft.routing[id]?.kind}`), + configured_providers: draft.cloudProviders.map(p => p.slug), + configured_with_key: configuredWithKey.map(p => p.slug), + // A provider key is configured but routing is still managed → the provider + // is not used until the user selects a custom route. + configured_but_managed: mode === 'managed' && configuredWithKey.length > 0, + }); + return mode; + }, [draft.routing, draft.cloudProviders]); const effectiveRoutingMode: RoutingMode = routingEditorMode === 'own' ? 'own' @@ -2800,14 +2829,11 @@ const AIPanel = ({ embedded = false }: AIPanelProps = {}) => { // Toggle OFF: remove the provider + scrub any // routing entries that pin to it. const remaining = draft.cloudProviders.filter(cp => cp.id !== existing.id); - const nextRouting = Object.fromEntries( - Object.entries(draft.routing).map(([wid, ref]) => [ - wid, - ref.kind === 'cloud' && ref.providerSlug === existing.slug - ? ({ kind: 'default' } as const) - : ref, - ]) - ) as typeof draft.routing; + const nextRouting = routingWithProviderRemoved( + draft.routing, + { slug: existing.slug, isLocalRuntime: false }, + remaining + ); await persist({ ...draft, cloudProviders: remaining, @@ -2834,14 +2860,11 @@ const AIPanel = ({ embedded = false }: AIPanelProps = {}) => { busy={busyAction === `toggle-${existing.slug}`} onToggle={async () => { const remaining = draft.cloudProviders.filter(cp => cp.id !== existing.id); - const nextRouting = Object.fromEntries( - Object.entries(draft.routing).map(([wid, ref]) => [ - wid, - ref.kind === 'cloud' && ref.providerSlug === existing.slug - ? ({ kind: 'default' } as const) - : ref, - ]) - ) as typeof draft.routing; + const nextRouting = routingWithProviderRemoved( + draft.routing, + { slug: existing.slug, isLocalRuntime: false }, + remaining + ); await persist({ ...draft, cloudProviders: remaining, routing: nextRouting }); }} /> @@ -2872,14 +2895,11 @@ const AIPanel = ({ embedded = false }: AIPanelProps = {}) => { const remaining = draft.cloudProviders.filter( cp => cp.id !== existing.id ); - const nextRouting = Object.fromEntries( - Object.entries(draft.routing).map(([wid, ref]) => [ - wid, - ref.kind === 'cloud' && ref.providerSlug === localKind - ? ({ kind: 'default' } as const) - : ref, - ]) - ) as typeof draft.routing; + const nextRouting = routingWithProviderRemoved( + draft.routing, + { slug: localKind, isLocalRuntime: true }, + remaining + ); await persist({ ...draft, cloudProviders: remaining, diff --git a/app/src/components/settings/panels/__tests__/AIPanel.test.tsx b/app/src/components/settings/panels/__tests__/AIPanel.test.tsx index 9c37cc741..72cef58e5 100644 --- a/app/src/components/settings/panels/__tests__/AIPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/AIPanel.test.tsx @@ -583,6 +583,58 @@ describe('AIPanel', () => { expect(nextSettings.routing.coding).toEqual({ kind: 'openhuman' }); }); + // ─── chip toggle: local runtime toggle OFF scrubs orphaned local routing ───── + + it('toggling OFF a local runtime resets workloads routed to it back to default', async () => { + const settingsWithOllama = { + cloudProviders: [ + { + id: 'p_ollama_1', + slug: 'ollama', + label: 'Ollama', + endpoint: 'http://localhost:11434/v1', + auth_style: 'bearer' as const, + has_api_key: false, + }, + ], + routing: { + chat: { kind: 'local' as const, model: 'llama3' }, + reasoning: { kind: 'local' as const, model: 'llama3' }, + agentic: { kind: 'openhuman' as const }, + coding: { kind: 'openhuman' as const }, + memory: { kind: 'openhuman' as const }, + embeddings: { kind: 'openhuman' as const }, + heartbeat: { kind: 'openhuman' as const }, + learning: { kind: 'openhuman' as const }, + subconscious: { kind: 'openhuman' as const }, + }, + }; + vi.mocked(loadAISettings).mockResolvedValue(settingsWithOllama); + vi.mocked(saveAISettings).mockResolvedValue(undefined); + + renderWithProviders(); + + await waitFor(() => + expect(screen.getByRole('switch', { name: /Disconnect Ollama/i })).toBeInTheDocument() + ); + + // Toggle Ollama OFF — no other local runtime remains, so its routed + // workloads are orphaned and must reset to the user default route. + fireEvent.click(screen.getByRole('switch', { name: /Disconnect Ollama/i })); + + await waitFor(() => expect(vi.mocked(saveAISettings)).toHaveBeenCalled()); + const [, nextSettings] = vi.mocked(saveAISettings).mock.calls[0]; + + expect( + nextSettings.cloudProviders.find((p: { slug: string }) => p.slug === 'ollama') + ).toBeUndefined(); + // Local-routed workloads reset to default (the fix — previously left orphaned). + expect(nextSettings.routing.chat).toEqual({ kind: 'default' }); + expect(nextSettings.routing.reasoning).toEqual({ kind: 'default' }); + // Already-managed entries unchanged. + expect(nextSettings.routing.agentic).toEqual({ kind: 'openhuman' }); + }); + // ─── API-key dialog: failed setCloudProviderKey does not add provider ──────── it('when setCloudProviderKey throws, the provider is NOT added to the draft', async () => { diff --git a/app/src/components/settings/panels/__tests__/aiRouting.test.ts b/app/src/components/settings/panels/__tests__/aiRouting.test.ts new file mode 100644 index 000000000..3eae75f1a --- /dev/null +++ b/app/src/components/settings/panels/__tests__/aiRouting.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from 'vitest'; + +import type { CloudProvider, ProviderRef, RoutingMap } from '../AIPanel'; +import { routingWithProviderRemoved } from '../aiRouting'; + +const WORKLOADS = [ + 'chat', + 'reasoning', + 'agentic', + 'coding', + 'memory', + 'heartbeat', + 'learning', + 'subconscious', +] as const; + +/** Build a full 8-workload routing map defaulting every slot to managed. */ +function routingOf( + overrides: Partial> +): RoutingMap { + const base = Object.fromEntries( + WORKLOADS.map(w => [w, { kind: 'default' } as ProviderRef]) + ) as RoutingMap; + return { ...base, ...overrides }; +} + +const cloudRef = (slug: string, model = 'm'): ProviderRef => ({ + kind: 'cloud', + providerSlug: slug, + model, +}); +const localRef = (model = 'llama3'): ProviderRef => ({ kind: 'local', model }); +// The helper only reads `.slug`; the rest of CloudProvider is irrelevant here. +const provider = (slug: string): CloudProvider => + ({ + id: `id-${slug}`, + slug, + label: slug, + endpoint: '', + maskedKey: '', + }) as unknown as CloudProvider; + +describe('routingWithProviderRemoved', () => { + it('resets workloads pinned to a removed cloud provider back to default', () => { + const routing = routingOf({ chat: cloudRef('openrouter'), coding: cloudRef('openrouter') }); + const next = routingWithProviderRemoved( + routing, + { slug: 'openrouter', isLocalRuntime: false }, + [] + ); + expect(next.chat).toEqual({ kind: 'default' }); + expect(next.coding).toEqual({ kind: 'default' }); + }); + + it('leaves a different cloud provider untouched when one is removed', () => { + const routing = routingOf({ chat: cloudRef('openrouter'), reasoning: cloudRef('openai') }); + const next = routingWithProviderRemoved( + routing, + { slug: 'openrouter', isLocalRuntime: false }, + [provider('openai')] + ); + expect(next.chat).toEqual({ kind: 'default' }); + expect(next.reasoning).toEqual(cloudRef('openai')); + }); + + it('resets local-runtime refs when the last local runtime is disabled', () => { + const routing = routingOf({ chat: localRef('llama3'), agentic: localRef('llama3') }); + // Disabling ollama with no local runtime remaining → local refs are orphaned. + const next = routingWithProviderRemoved(routing, { slug: 'ollama', isLocalRuntime: true }, []); + expect(next.chat).toEqual({ kind: 'default' }); + expect(next.agentic).toEqual({ kind: 'default' }); + }); + + it('keeps local refs when another local runtime is still enabled', () => { + const routing = routingOf({ chat: localRef('llama3') }); + // Disabling lmstudio while ollama remains → the local ref may resolve to ollama. + const next = routingWithProviderRemoved(routing, { slug: 'lmstudio', isLocalRuntime: true }, [ + provider('ollama'), + ]); + expect(next.chat).toEqual(localRef('llama3')); + }); + + it('does not scrub local refs when a cloud provider is removed (regression guard)', () => { + const routing = routingOf({ chat: localRef('llama3'), reasoning: cloudRef('openrouter') }); + const next = routingWithProviderRemoved( + routing, + { slug: 'openrouter', isLocalRuntime: false }, + [] + ); + expect(next.chat).toEqual(localRef('llama3')); + expect(next.reasoning).toEqual({ kind: 'default' }); + }); + + it('preserves all 8 workload slots', () => { + const next = routingWithProviderRemoved( + routingOf({}), + { slug: 'x', isLocalRuntime: false }, + [] + ); + expect(Object.keys(next).sort()).toEqual([...WORKLOADS].sort()); + }); +}); diff --git a/app/src/components/settings/panels/aiRouting.ts b/app/src/components/settings/panels/aiRouting.ts new file mode 100644 index 000000000..6f86c3fa0 --- /dev/null +++ b/app/src/components/settings/panels/aiRouting.ts @@ -0,0 +1,51 @@ +/* + * Pure routing-map helpers for the AI settings panel. + * + * Kept out of AIPanel.tsx so the logic is unit-testable without rendering the + * whole panel (the types are imported type-only, so there is no runtime import + * cycle — AIPanel imports these functions, this module imports only erased + * types back from it). + */ +import type { CloudProvider, ProviderRef, RoutingMap } from './AIPanel'; + +const LOCAL_RUNTIME_SLUGS = ['ollama', 'lmstudio'] as const; + +/** + * Reset any workload routing ref pinned to a now-removed provider back to + * `{ kind: 'default' }` (managed), so disabling a provider can never leave + * orphaned routing that still points at it. + * + * Matching differs by provider kind because routing refs carry different + * identity: + * - **Cloud / custom** providers are matched precisely by `providerSlug` + * (`{ kind: 'cloud', providerSlug, model }`). + * - **Local runtimes** (Ollama / LM Studio) have NO slug on their routing refs + * (`{ kind: 'local', model }`), so an individual local ref can't be tied back + * to a specific runtime. A `local` ref is therefore only definitively + * orphaned once NO local runtime remains enabled; while another local runtime + * is still enabled we leave `local` refs alone since they may resolve to the + * survivor. + * + * Before this helper the local case was silently a no-op: the toggle-off + * handlers only matched `kind === 'cloud' && providerSlug === `, which + * a `kind: 'local'` ref can never satisfy — so disabling Ollama / LM Studio left + * its routed workloads pinned to a now-removed runtime. + */ +export function routingWithProviderRemoved( + routing: RoutingMap, + removed: { slug: string; isLocalRuntime: boolean }, + remainingProviders: readonly CloudProvider[] +): RoutingMap { + const anyLocalRuntimeLeft = remainingProviders.some(p => + (LOCAL_RUNTIME_SLUGS as readonly string[]).includes(p.slug) + ); + + const scrubbed = Object.entries(routing).map(([workloadId, ref]) => { + const pinnedToRemovedCloud = ref.kind === 'cloud' && ref.providerSlug === removed.slug; + const orphanedLocal = ref.kind === 'local' && removed.isLocalRuntime && !anyLocalRuntimeLeft; + const nextRef: ProviderRef = pinnedToRemovedCloud || orphanedLocal ? { kind: 'default' } : ref; + return [workloadId, nextRef] as const; + }); + + return Object.fromEntries(scrubbed) as RoutingMap; +}