From eff47f7977e3be68c886b2f75520d0583bd13efc Mon Sep 17 00:00:00 2001 From: Naved Date: Fri, 10 Jul 2026 12:24:24 -0700 Subject: [PATCH 1/2] ollama fetch fix --- .../__tests__/webviewMessageHandler.spec.ts | 64 ++++++++ src/core/webview/webviewMessageHandler.ts | 29 +++- .../src/components/settings/ApiOptions.tsx | 8 +- .../components/settings/providers/Ollama.tsx | 73 +++++++-- .../providers/__tests__/Ollama.spec.tsx | 146 +++++++++++++++++- 5 files changed, 294 insertions(+), 26 deletions(-) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 335911a7c7..d04f30b43a 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -331,6 +331,70 @@ describe("webviewMessageHandler - requestOllamaModels", () => { ollamaModels: mockModels, }) }) + + it("posts empty models response when no models are found", async () => { + mockGetModels.mockResolvedValue({}) + + await webviewMessageHandler(mockClineProvider, { + type: "requestOllamaModels", + }) + + expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "ollamaModels", + ollamaModels: {}, + }) + }) + + it("posts empty models response with error message and logs to output on fetch failure", async () => { + mockGetModels.mockRejectedValue(new Error("Connection refused")) + + await webviewMessageHandler(mockClineProvider, { + type: "requestOllamaModels", + }) + + expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "ollamaModels", + ollamaModels: {}, + error: "Connection refused", + }) + + expect(mockClineProvider.log).toHaveBeenCalledWith( + expect.stringContaining("[requestOllamaModels] Failed to fetch models: Connection refused"), + ) + }) + + it("uses baseUrl from message values over saved state", async () => { + const mockModels: ModelRecord = { + "remote-model": { + maxTokens: 4096, + contextWindow: 8192, + supportsPromptCache: false, + description: "Remote model", + }, + } + + mockGetModels.mockResolvedValue(mockModels) + + await webviewMessageHandler(mockClineProvider, { + type: "requestOllamaModels", + values: { + baseUrl: "https://ollama.example.com", + apiKey: "secret-key", + }, + }) + + // Should use the URL from message values, not the saved state + expect(mockGetModels).toHaveBeenCalledWith({ + provider: "ollama", + baseUrl: "https://ollama.example.com", + apiKey: "secret-key", + }) + + expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "ollamaModels", + ollamaModels: mockModels, + }) + }) }) describe("webviewMessageHandler - requestRouterModels", () => { diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index fdcc53ddc1..d76c96022c 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1203,23 +1203,38 @@ export const webviewMessageHandler = async ( case "requestOllamaModels": { // Specific handler for Ollama models only. const { apiConfiguration: ollamaApiConfig } = await provider.getState() + // Prefer the baseUrl/apiKey from the message values (which reflect + // the user's unsaved edits in the settings form) over the saved + // state, so the refresh uses the URL the user is actually looking + // at — not the stale one from before they started editing. + const baseUrl = message.values?.baseUrl ?? ollamaApiConfig.ollamaBaseUrl + const apiKey = message.values?.apiKey ?? ollamaApiConfig.ollamaApiKey try { const ollamaOptions = { provider: "ollama" as const, - baseUrl: ollamaApiConfig.ollamaBaseUrl, - apiKey: ollamaApiConfig.ollamaApiKey, + baseUrl, + apiKey, } // Flush cache and refresh to ensure fresh models. await flushModels(ollamaOptions, true) const ollamaModels = await getModels(ollamaOptions) - if (Object.keys(ollamaModels).length > 0) { - provider.postMessageToWebview({ type: "ollamaModels", ollamaModels: ollamaModels }) - } + // Always post a response so the webview refresh status can + // transition out of "loading" — even when no models are found. + provider.postMessageToWebview({ type: "ollamaModels", ollamaModels: ollamaModels }) } catch (error) { - // Silently fail - user hasn't configured Ollama yet - console.debug("Ollama models fetch failed:", error) + // Log the error to the output channel for debugging, but still + // post an empty response so the webview doesn't stay stuck in + // the loading state. Include the error message so the webview + // can display it directly in the settings panel. + const errorMsg = error instanceof Error ? error.message : String(error) + provider.log(`[requestOllamaModels] Failed to fetch models: ${errorMsg}`) + provider.postMessageToWebview({ + type: "ollamaModels", + ollamaModels: {}, + error: errorMsg, + }) } break } diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index 6bfe20c718..76cb2e3cf9 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -218,7 +218,13 @@ const ApiOptions = ({ }, }) } else if (selectedProvider === "ollama") { - vscode.postMessage({ type: "requestOllamaModels" }) + vscode.postMessage({ + type: "requestOllamaModels", + values: { + baseUrl: apiConfiguration?.ollamaBaseUrl, + apiKey: apiConfiguration?.ollamaApiKey, + }, + }) } else if (selectedProvider === "lmstudio") { requestLmStudioModels(apiConfiguration?.lmStudioBaseUrl) } else if (selectedProvider === "vscode-lm") { diff --git a/webview-ui/src/components/settings/providers/Ollama.tsx b/webview-ui/src/components/settings/providers/Ollama.tsx index 9d92ee972b..61e65da5d7 100644 --- a/webview-ui/src/components/settings/providers/Ollama.tsx +++ b/webview-ui/src/components/settings/providers/Ollama.tsx @@ -1,5 +1,4 @@ import { useState, useCallback, useMemo, useEffect } from "react" -import { useEvent } from "react-use" import { VSCodeTextField } from "@vscode/webview-ui-toolkit/react" import { Checkbox } from "vscrui" @@ -7,6 +6,7 @@ import { type ProviderSettings, type ExtensionMessage, type ModelRecord, ollamaD import { useAppTranslation } from "@src/i18n/TranslationContext" import { useRouterModels } from "@src/components/ui/hooks/useRouterModels" +import { Button } from "@src/components/ui" import { vscode } from "@src/utils/vscode" import { inputEventTransform } from "../transforms" @@ -22,6 +22,8 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro const { t } = useAppTranslation() const [ollamaModels, setOllamaModels] = useState({}) + const [refreshStatus, setRefreshStatus] = useState<"idle" | "loading" | "success" | "error">("idle") + const [refreshError, setRefreshError] = useState() const routerModels = useRouterModels() const handleInputChange = useCallback( @@ -35,20 +37,42 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro [setApiConfigurationField], ) - const onMessage = useCallback((event: MessageEvent) => { - const message: ExtensionMessage = event.data + useEffect(() => { + const handleMessage = (event: MessageEvent) => { + const message: ExtensionMessage = event.data + + if (message.type === "ollamaModels") { + const newModels = message.ollamaModels ?? {} + setOllamaModels(newModels) - switch (message.type) { - case "ollamaModels": - { - const newModels = message.ollamaModels ?? {} - setOllamaModels(newModels) + if (refreshStatus === "loading") { + if (Object.keys(newModels).length > 0) { + setRefreshStatus("success") + } else { + setRefreshStatus("error") + setRefreshError(message.error) + } } - break + } } - }, []) - useEvent("message", onMessage) + window.addEventListener("message", handleMessage) + return () => { + window.removeEventListener("message", handleMessage) + } + }, [refreshStatus]) + + const handleRefreshModels = useCallback(() => { + setRefreshStatus("loading") + setRefreshError(undefined) + vscode.postMessage({ + type: "requestOllamaModels", + values: { + baseUrl: apiConfiguration?.ollamaBaseUrl, + apiKey: apiConfiguration?.ollamaApiKey, + }, + }) + }, [apiConfiguration?.ollamaBaseUrl, apiConfiguration?.ollamaApiKey]) // Refresh models on mount useEffect(() => { @@ -102,6 +126,33 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro )} + + {refreshStatus === "loading" && ( +
+ {t("settings:providers.refreshModels.loading")} +
+ )} + {refreshStatus === "success" && ( +
{t("settings:providers.refreshModels.success")}
+ )} + {refreshStatus === "error" && ( +
+ {refreshError || t("settings:providers.refreshModels.error")} +
+ )} ({ ), })) -// Mock react-use -vi.mock("react-use", () => ({ - useEvent: vi.fn(), -})) - // Mock useRouterModels vi.mock("@src/components/ui/hooks/useRouterModels", () => ({ useRouterModels: () => ({ data: {}, isLoading: false, error: null }), })) +const { postMessageMock } = vi.hoisted(() => ({ + postMessageMock: vi.fn(), +})) + // Mock vscode vi.mock("@src/utils/vscode", () => ({ - vscode: { postMessage: vi.fn() }, + vscode: { postMessage: postMessageMock }, +})) + +// Stub the shared Button so we can assert onClick/disabled without its styling deps. +vi.mock("@src/components/ui", () => ({ + Button: ({ children, onClick, disabled, className }: any) => ( + + ), })) describe("Ollama Component - thinking setting", () => { @@ -221,3 +229,127 @@ describe("Ollama Component - thinking setting", () => { expect(screen.queryByTestId("thinking-budget")).toBeNull() }) }) + +describe("Ollama Component - refresh models", () => { + const mockSetApiConfigurationField = vi.fn() + + const dispatchMessage = (data: any) => + act(() => { + window.dispatchEvent(new MessageEvent("message", { data })) + }) + + beforeEach(() => { + vi.clearAllMocks() + }) + + it("renders the refresh button in idle state", () => { + render( + , + ) + + const button = screen.getByTestId("refresh-button") + expect(button).not.toBeDisabled() + expect(button.querySelector(".codicon-refresh")).not.toBeNull() + expect(screen.getByText("settings:providers.refreshModels.label")).toBeInTheDocument() + }) + + it("sends requestOllamaModels with baseUrl and apiKey when the refresh button is clicked", () => { + render( + , + ) + + fireEvent.click(screen.getByTestId("refresh-button")) + + expect(postMessageMock).toHaveBeenCalledWith({ + type: "requestOllamaModels", + values: { + baseUrl: "https://ollama.example.com", + apiKey: "secret-key", + }, + }) + }) + + it("enters loading state and disables the button while refreshing", () => { + render( + , + ) + + fireEvent.click(screen.getByTestId("refresh-button")) + + const button = screen.getByTestId("refresh-button") + expect(button).toBeDisabled() + expect(button.querySelector(".codicon-loading")).not.toBeNull() + expect(screen.getByText("settings:providers.refreshModels.loading")).toBeInTheDocument() + }) + + it("shows success state when ollamaModels arrives with models while loading", () => { + render( + , + ) + + fireEvent.click(screen.getByTestId("refresh-button")) + dispatchMessage({ type: "ollamaModels", ollamaModels: { "llama3:latest": {} } }) + + expect(screen.getByText("settings:providers.refreshModels.success")).toBeInTheDocument() + }) + + it("shows error state when ollamaModels arrives with empty models while loading", () => { + render( + , + ) + + fireEvent.click(screen.getByTestId("refresh-button")) + dispatchMessage({ type: "ollamaModels", ollamaModels: {} }) + + expect(screen.getByText("settings:providers.refreshModels.error")).toBeInTheDocument() + }) + + it("displays the backend error message when ollamaModels arrives with an error", () => { + render( + , + ) + + fireEvent.click(screen.getByTestId("refresh-button")) + dispatchMessage({ type: "ollamaModels", ollamaModels: {}, error: "Connection refused" }) + + expect(screen.getByText("Connection refused")).toBeInTheDocument() + }) + + it("ignores ollamaModels messages when not in loading state", () => { + render( + , + ) + + // No refresh initiated; an unsolicited ollamaModels message should be a no-op. + dispatchMessage({ type: "ollamaModels", ollamaModels: { "llama3:latest": {} } }) + + expect(screen.queryByText("settings:providers.refreshModels.success")).not.toBeInTheDocument() + expect(screen.queryByText("settings:providers.refreshModels.loading")).not.toBeInTheDocument() + }) +}) From 3d6980576a341e36ff5ebcbeab2686880692b6fe Mon Sep 17 00:00:00 2001 From: Naved Merchant Date: Thu, 16 Jul 2026 18:19:19 -0700 Subject: [PATCH 2/2] address review comments --- .../__tests__/webviewMessageHandler.spec.ts | 32 +++++++++- src/core/webview/webviewMessageHandler.ts | 34 ++++++---- .../src/components/settings/ApiOptions.tsx | 1 + .../components/settings/providers/Ollama.tsx | 23 +++---- .../providers/__tests__/Ollama.spec.tsx | 64 ++++++++++++++++--- 5 files changed, 120 insertions(+), 34 deletions(-) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index c4c2c6e489..1db29c90fc 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -294,6 +294,9 @@ describe("webviewMessageHandler - image mentions", () => { describe("webviewMessageHandler - requestOllamaModels", () => { beforeEach(() => { vi.clearAllMocks() + mockFlushModels.mockReset() + mockFlushModels.mockResolvedValue(undefined) + mockGetModels.mockReset() mockClineProvider.getState = vi.fn().mockResolvedValue({ apiConfiguration: { ollamaModelId: "model-1", @@ -359,10 +362,29 @@ describe("webviewMessageHandler - requestOllamaModels", () => { }) expect(mockClineProvider.log).toHaveBeenCalledWith( - expect.stringContaining("[requestOllamaModels] Failed to fetch models: Connection refused"), + "[requestOllamaModels] Failed to read models for http://localhost:1234: Connection refused", ) }) + it("distinguishes a model cache refresh failure from a model read failure", async () => { + mockFlushModels.mockRejectedValue(new Error("Cache write failed")) + + await webviewMessageHandler(mockClineProvider, { + type: "requestOllamaModels", + values: { baseUrl: "https://ollama.example.com" }, + }) + + expect(mockGetModels).not.toHaveBeenCalled() + expect(mockClineProvider.log).toHaveBeenCalledWith( + "[requestOllamaModels] Failed to refresh model cache for https://ollama.example.com: Cache write failed", + ) + expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "ollamaModels", + ollamaModels: {}, + error: "Cache write failed", + }) + }) + it("uses baseUrl from message values over saved state", async () => { const mockModels: ModelRecord = { "remote-model": { @@ -384,6 +406,14 @@ describe("webviewMessageHandler - requestOllamaModels", () => { }) // Should use the URL from message values, not the saved state + expect(mockFlushModels).toHaveBeenCalledWith( + { + provider: "ollama", + baseUrl: "https://ollama.example.com", + apiKey: "secret-key", + }, + true, + ) expect(mockGetModels).toHaveBeenCalledWith({ provider: "ollama", baseUrl: "https://ollama.example.com", diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 4057f65151..9a7396571b 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1227,27 +1227,37 @@ export const webviewMessageHandler = async ( // at — not the stale one from before they started editing. const baseUrl = message.values?.baseUrl ?? ollamaApiConfig.ollamaBaseUrl const apiKey = message.values?.apiKey ?? ollamaApiConfig.ollamaApiKey + const logBaseUrl = baseUrl || "http://localhost:11434" + const ollamaOptions = { + provider: "ollama" as const, + baseUrl, + apiKey, + } try { - const ollamaOptions = { - provider: "ollama" as const, - baseUrl, - apiKey, - } - // Flush cache and refresh to ensure fresh models. + // Refresh the cache before reading the models. Keep this error + // separate from the read below so diagnostics identify which + // cache operation failed. await flushModels(ollamaOptions, true) + } catch (error) { + const errorMsg = error instanceof Error ? error.message : String(error) + provider.log(`[requestOllamaModels] Failed to refresh model cache for ${logBaseUrl}: ${errorMsg}`) + provider.postMessageToWebview({ + type: "ollamaModels", + ollamaModels: {}, + error: errorMsg, + }) + break + } + try { const ollamaModels = await getModels(ollamaOptions) // Always post a response so the webview refresh status can // transition out of "loading" — even when no models are found. - provider.postMessageToWebview({ type: "ollamaModels", ollamaModels: ollamaModels }) + provider.postMessageToWebview({ type: "ollamaModels", ollamaModels }) } catch (error) { - // Log the error to the output channel for debugging, but still - // post an empty response so the webview doesn't stay stuck in - // the loading state. Include the error message so the webview - // can display it directly in the settings panel. const errorMsg = error instanceof Error ? error.message : String(error) - provider.log(`[requestOllamaModels] Failed to fetch models: ${errorMsg}`) + provider.log(`[requestOllamaModels] Failed to read models for ${logBaseUrl}: ${errorMsg}`) provider.postMessageToWebview({ type: "ollamaModels", ollamaModels: {}, diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index 2686047271..2e3129fc34 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -249,6 +249,7 @@ const ApiOptions = ({ apiConfiguration?.openAiBaseUrl, apiConfiguration?.openAiApiKey, apiConfiguration?.ollamaBaseUrl, + apiConfiguration?.ollamaApiKey, apiConfiguration?.lmStudioBaseUrl, apiConfiguration?.litellmBaseUrl, apiConfiguration?.litellmApiKey, diff --git a/webview-ui/src/components/settings/providers/Ollama.tsx b/webview-ui/src/components/settings/providers/Ollama.tsx index 61e65da5d7..9b11c85369 100644 --- a/webview-ui/src/components/settings/providers/Ollama.tsx +++ b/webview-ui/src/components/settings/providers/Ollama.tsx @@ -1,4 +1,4 @@ -import { useState, useCallback, useMemo, useEffect } from "react" +import { useState, useCallback, useMemo, useEffect, useRef } from "react" import { VSCodeTextField } from "@vscode/webview-ui-toolkit/react" import { Checkbox } from "vscrui" @@ -24,6 +24,7 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro const [ollamaModels, setOllamaModels] = useState({}) const [refreshStatus, setRefreshStatus] = useState<"idle" | "loading" | "success" | "error">("idle") const [refreshError, setRefreshError] = useState() + const refreshStatusRef = useRef(refreshStatus) const routerModels = useRouterModels() const handleInputChange = useCallback( @@ -42,16 +43,15 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro const message: ExtensionMessage = event.data if (message.type === "ollamaModels") { - const newModels = message.ollamaModels ?? {} - setOllamaModels(newModels) + if (!message.error) { + setOllamaModels(message.ollamaModels ?? {}) + } - if (refreshStatus === "loading") { - if (Object.keys(newModels).length > 0) { - setRefreshStatus("success") - } else { - setRefreshStatus("error") - setRefreshError(message.error) - } + if (refreshStatusRef.current === "loading") { + const nextStatus = message.error ? "error" : "success" + refreshStatusRef.current = nextStatus + setRefreshStatus(nextStatus) + setRefreshError(message.error) } } } @@ -60,9 +60,10 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro return () => { window.removeEventListener("message", handleMessage) } - }, [refreshStatus]) + }, []) const handleRefreshModels = useCallback(() => { + refreshStatusRef.current = "loading" setRefreshStatus("loading") setRefreshError(undefined) vscode.postMessage({ diff --git a/webview-ui/src/components/settings/providers/__tests__/Ollama.spec.tsx b/webview-ui/src/components/settings/providers/__tests__/Ollama.spec.tsx index e912f25c1f..a4e14b4d81 100644 --- a/webview-ui/src/components/settings/providers/__tests__/Ollama.spec.tsx +++ b/webview-ui/src/components/settings/providers/__tests__/Ollama.spec.tsx @@ -45,7 +45,7 @@ vi.mock("@src/i18n/TranslationContext", () => ({ // Mock the ModelPicker vi.mock("../../ModelPicker", () => ({ - ModelPicker: () =>
Model Picker
, + ModelPicker: ({ models }: any) =>
{Object.keys(models ?? {}).join(",")}
, })) // Mock the ThinkingBudget @@ -72,13 +72,17 @@ vi.mock("@src/utils/vscode", () => ({ })) // Stub the shared Button so we can assert onClick/disabled without its styling deps. -vi.mock("@src/components/ui", () => ({ - Button: ({ children, onClick, disabled, className }: any) => ( - - ), -})) +vi.mock("@src/components/ui", async () => { + const actual = await vi.importActual("@src/components/ui") + return { + ...actual, + Button: ({ children, onClick, disabled, className }: any) => ( + + ), + } +}) describe("Ollama Component - thinking setting", () => { const mockSetApiConfigurationField = vi.fn() @@ -296,6 +300,27 @@ describe("Ollama Component - refresh models", () => { expect(screen.getByText("settings:providers.refreshModels.loading")).toBeInTheDocument() }) + it("handles a response dispatched synchronously while posting the refresh request", () => { + render( + , + ) + + // Ignore the mount request and respond synchronously to the explicit refresh. + postMessageMock.mockClear() + postMessageMock.mockImplementationOnce(() => { + window.dispatchEvent( + new MessageEvent("message", { data: { type: "ollamaModels", ollamaModels: { "llama3:latest": {} } } }), + ) + }) + fireEvent.click(screen.getByTestId("refresh-button")) + + expect(screen.getByText("settings:providers.refreshModels.success")).toBeInTheDocument() + expect(screen.getByTestId("model-picker")).toHaveTextContent("llama3:latest") + }) + it("shows success state when ollamaModels arrives with models while loading", () => { render( { expect(screen.getByText("settings:providers.refreshModels.success")).toBeInTheDocument() }) - it("shows error state when ollamaModels arrives with empty models while loading", () => { + it("shows success state when Ollama is reachable but has no installed models", () => { render( { fireEvent.click(screen.getByTestId("refresh-button")) dispatchMessage({ type: "ollamaModels", ollamaModels: {} }) - expect(screen.getByText("settings:providers.refreshModels.error")).toBeInTheDocument() + expect(screen.getByText("settings:providers.refreshModels.success")).toBeInTheDocument() }) it("displays the backend error message when ollamaModels arrives with an error", () => { @@ -338,6 +363,25 @@ describe("Ollama Component - refresh models", () => { expect(screen.getByText("Connection refused")).toBeInTheDocument() }) + it("preserves previously loaded models when a later refresh fails", () => { + render( + , + ) + + fireEvent.click(screen.getByTestId("refresh-button")) + dispatchMessage({ type: "ollamaModels", ollamaModels: { "llama3:latest": {} } }) + expect(screen.getByTestId("model-picker")).toHaveTextContent("llama3:latest") + + fireEvent.click(screen.getByTestId("refresh-button")) + dispatchMessage({ type: "ollamaModels", ollamaModels: {}, error: "Connection refused" }) + + expect(screen.getByText("Connection refused")).toBeInTheDocument() + expect(screen.getByTestId("model-picker")).toHaveTextContent("llama3:latest") + }) + it("ignores ollamaModels messages when not in loading state", () => { render(