Skip to content

Commit 2d2a238

Browse files
fix(ollama): always respond to model refresh, surface errors, and use form-edited base URL (#878)
* ollama fetch fix * address review comments
1 parent 933ee98 commit 2d2a238

5 files changed

Lines changed: 385 additions & 31 deletions

File tree

src/core/webview/__tests__/webviewMessageHandler.spec.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,9 @@ describe("webviewMessageHandler - image mentions", () => {
294294
describe("webviewMessageHandler - requestOllamaModels", () => {
295295
beforeEach(() => {
296296
vi.clearAllMocks()
297+
mockFlushModels.mockReset()
298+
mockFlushModels.mockResolvedValue(undefined)
299+
mockGetModels.mockReset()
297300
mockClineProvider.getState = vi.fn().mockResolvedValue({
298301
apiConfiguration: {
299302
ollamaModelId: "model-1",
@@ -331,6 +334,97 @@ describe("webviewMessageHandler - requestOllamaModels", () => {
331334
ollamaModels: mockModels,
332335
})
333336
})
337+
338+
it("posts empty models response when no models are found", async () => {
339+
mockGetModels.mockResolvedValue({})
340+
341+
await webviewMessageHandler(mockClineProvider, {
342+
type: "requestOllamaModels",
343+
})
344+
345+
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
346+
type: "ollamaModels",
347+
ollamaModels: {},
348+
})
349+
})
350+
351+
it("posts empty models response with error message and logs to output on fetch failure", async () => {
352+
mockGetModels.mockRejectedValue(new Error("Connection refused"))
353+
354+
await webviewMessageHandler(mockClineProvider, {
355+
type: "requestOllamaModels",
356+
})
357+
358+
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
359+
type: "ollamaModels",
360+
ollamaModels: {},
361+
error: "Connection refused",
362+
})
363+
364+
expect(mockClineProvider.log).toHaveBeenCalledWith(
365+
"[requestOllamaModels] Failed to read models for http://localhost:1234: Connection refused",
366+
)
367+
})
368+
369+
it("distinguishes a model cache refresh failure from a model read failure", async () => {
370+
mockFlushModels.mockRejectedValue(new Error("Cache write failed"))
371+
372+
await webviewMessageHandler(mockClineProvider, {
373+
type: "requestOllamaModels",
374+
values: { baseUrl: "https://ollama.example.com" },
375+
})
376+
377+
expect(mockGetModels).not.toHaveBeenCalled()
378+
expect(mockClineProvider.log).toHaveBeenCalledWith(
379+
"[requestOllamaModels] Failed to refresh model cache for https://ollama.example.com: Cache write failed",
380+
)
381+
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
382+
type: "ollamaModels",
383+
ollamaModels: {},
384+
error: "Cache write failed",
385+
})
386+
})
387+
388+
it("uses baseUrl from message values over saved state", async () => {
389+
const mockModels: ModelRecord = {
390+
"remote-model": {
391+
maxTokens: 4096,
392+
contextWindow: 8192,
393+
supportsPromptCache: false,
394+
description: "Remote model",
395+
},
396+
}
397+
398+
mockGetModels.mockResolvedValue(mockModels)
399+
400+
await webviewMessageHandler(mockClineProvider, {
401+
type: "requestOllamaModels",
402+
values: {
403+
baseUrl: "https://ollama.example.com",
404+
apiKey: "secret-key",
405+
},
406+
})
407+
408+
// Should use the URL from message values, not the saved state
409+
expect(mockFlushModels).toHaveBeenCalledWith(
410+
{
411+
provider: "ollama",
412+
baseUrl: "https://ollama.example.com",
413+
apiKey: "secret-key",
414+
},
415+
true,
416+
)
417+
expect(mockGetModels).toHaveBeenCalledWith({
418+
provider: "ollama",
419+
baseUrl: "https://ollama.example.com",
420+
apiKey: "secret-key",
421+
})
422+
423+
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
424+
type: "ollamaModels",
425+
ollamaModels: mockModels,
426+
})
427+
})
334428
})
335429

336430
describe("webviewMessageHandler - requestRouterModels", () => {

src/core/webview/webviewMessageHandler.ts

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,23 +1263,48 @@ export const webviewMessageHandler = async (
12631263
case "requestOllamaModels": {
12641264
// Specific handler for Ollama models only.
12651265
const { apiConfiguration: ollamaApiConfig } = await provider.getState()
1266+
// Prefer the baseUrl/apiKey from the message values (which reflect
1267+
// the user's unsaved edits in the settings form) over the saved
1268+
// state, so the refresh uses the URL the user is actually looking
1269+
// at — not the stale one from before they started editing.
1270+
const baseUrl = message.values?.baseUrl ?? ollamaApiConfig.ollamaBaseUrl
1271+
const apiKey = message.values?.apiKey ?? ollamaApiConfig.ollamaApiKey
1272+
const logBaseUrl = baseUrl || "http://localhost:11434"
1273+
const ollamaOptions = {
1274+
provider: "ollama" as const,
1275+
baseUrl,
1276+
apiKey,
1277+
}
12661278
try {
1267-
const ollamaOptions = {
1268-
provider: "ollama" as const,
1269-
baseUrl: ollamaApiConfig.ollamaBaseUrl,
1270-
apiKey: ollamaApiConfig.ollamaApiKey,
1271-
}
1272-
// Flush cache and refresh to ensure fresh models.
1279+
// Refresh the cache before reading the models. Keep this error
1280+
// separate from the read below so diagnostics identify which
1281+
// cache operation failed.
12731282
await flushModels(ollamaOptions, true)
1283+
} catch (error) {
1284+
const errorMsg = error instanceof Error ? error.message : String(error)
1285+
provider.log(`[requestOllamaModels] Failed to refresh model cache for ${logBaseUrl}: ${errorMsg}`)
1286+
provider.postMessageToWebview({
1287+
type: "ollamaModels",
1288+
ollamaModels: {},
1289+
error: errorMsg,
1290+
})
1291+
break
1292+
}
12741293

1294+
try {
12751295
const ollamaModels = await getModels(ollamaOptions)
12761296

1277-
if (Object.keys(ollamaModels).length > 0) {
1278-
provider.postMessageToWebview({ type: "ollamaModels", ollamaModels: ollamaModels })
1279-
}
1297+
// Always post a response so the webview refresh status can
1298+
// transition out of "loading" — even when no models are found.
1299+
provider.postMessageToWebview({ type: "ollamaModels", ollamaModels })
12801300
} catch (error) {
1281-
// Silently fail - user hasn't configured Ollama yet
1282-
console.debug("Ollama models fetch failed:", error)
1301+
const errorMsg = error instanceof Error ? error.message : String(error)
1302+
provider.log(`[requestOllamaModels] Failed to read models for ${logBaseUrl}: ${errorMsg}`)
1303+
provider.postMessageToWebview({
1304+
type: "ollamaModels",
1305+
ollamaModels: {},
1306+
error: errorMsg,
1307+
})
12831308
}
12841309
break
12851310
}

webview-ui/src/components/settings/ApiOptions.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,13 @@ const ApiOptions = ({
221221
},
222222
})
223223
} else if (selectedProvider === "ollama") {
224-
vscode.postMessage({ type: "requestOllamaModels" })
224+
vscode.postMessage({
225+
type: "requestOllamaModels",
226+
values: {
227+
baseUrl: apiConfiguration?.ollamaBaseUrl,
228+
apiKey: apiConfiguration?.ollamaApiKey,
229+
},
230+
})
225231
} else if (selectedProvider === "lmstudio") {
226232
requestLmStudioModels(apiConfiguration?.lmStudioBaseUrl)
227233
} else if (selectedProvider === "vscode-lm") {
@@ -245,6 +251,7 @@ const ApiOptions = ({
245251
apiConfiguration?.openAiBaseUrl,
246252
apiConfiguration?.openAiApiKey,
247253
apiConfiguration?.ollamaBaseUrl,
254+
apiConfiguration?.ollamaApiKey,
248255
apiConfiguration?.lmStudioBaseUrl,
249256
apiConfiguration?.litellmBaseUrl,
250257
apiConfiguration?.litellmApiKey,

webview-ui/src/components/settings/providers/Ollama.tsx

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { useState, useCallback, useMemo, useEffect } from "react"
2-
import { useEvent } from "react-use"
1+
import { useState, useCallback, useMemo, useEffect, useRef } from "react"
32
import { VSCodeTextField } from "@vscode/webview-ui-toolkit/react"
43
import { Checkbox } from "vscrui"
54

65
import { type ProviderSettings, type ExtensionMessage, type ModelRecord, ollamaDefaultModelInfo } from "@roo-code/types"
76

87
import { useAppTranslation } from "@src/i18n/TranslationContext"
98
import { useRouterModels } from "@src/components/ui/hooks/useRouterModels"
9+
import { Button } from "@src/components/ui"
1010
import { vscode } from "@src/utils/vscode"
1111

1212
import { inputEventTransform } from "../transforms"
@@ -22,6 +22,9 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro
2222
const { t } = useAppTranslation()
2323

2424
const [ollamaModels, setOllamaModels] = useState<ModelRecord>({})
25+
const [refreshStatus, setRefreshStatus] = useState<"idle" | "loading" | "success" | "error">("idle")
26+
const [refreshError, setRefreshError] = useState<string | undefined>()
27+
const refreshStatusRef = useRef(refreshStatus)
2528
const routerModels = useRouterModels()
2629

2730
const handleInputChange = useCallback(
@@ -35,20 +38,42 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro
3538
[setApiConfigurationField],
3639
)
3740

38-
const onMessage = useCallback((event: MessageEvent) => {
39-
const message: ExtensionMessage = event.data
41+
useEffect(() => {
42+
const handleMessage = (event: MessageEvent) => {
43+
const message: ExtensionMessage = event.data
44+
45+
if (message.type === "ollamaModels") {
46+
if (!message.error) {
47+
setOllamaModels(message.ollamaModels ?? {})
48+
}
4049

41-
switch (message.type) {
42-
case "ollamaModels":
43-
{
44-
const newModels = message.ollamaModels ?? {}
45-
setOllamaModels(newModels)
50+
if (refreshStatusRef.current === "loading") {
51+
const nextStatus = message.error ? "error" : "success"
52+
refreshStatusRef.current = nextStatus
53+
setRefreshStatus(nextStatus)
54+
setRefreshError(message.error)
4655
}
47-
break
56+
}
57+
}
58+
59+
window.addEventListener("message", handleMessage)
60+
return () => {
61+
window.removeEventListener("message", handleMessage)
4862
}
4963
}, [])
5064

51-
useEvent("message", onMessage)
65+
const handleRefreshModels = useCallback(() => {
66+
refreshStatusRef.current = "loading"
67+
setRefreshStatus("loading")
68+
setRefreshError(undefined)
69+
vscode.postMessage({
70+
type: "requestOllamaModels",
71+
values: {
72+
baseUrl: apiConfiguration?.ollamaBaseUrl,
73+
apiKey: apiConfiguration?.ollamaApiKey,
74+
},
75+
})
76+
}, [apiConfiguration?.ollamaBaseUrl, apiConfiguration?.ollamaApiKey])
5277

5378
// Refresh models on mount
5479
useEffect(() => {
@@ -102,6 +127,33 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro
102127
</div>
103128
</VSCodeTextField>
104129
)}
130+
<Button
131+
variant="outline"
132+
onClick={handleRefreshModels}
133+
disabled={refreshStatus === "loading"}
134+
className="w-full">
135+
<div className="flex items-center gap-2">
136+
{refreshStatus === "loading" ? (
137+
<span className="codicon codicon-loading codicon-modifier-spin" />
138+
) : (
139+
<span className="codicon codicon-refresh" />
140+
)}
141+
{t("settings:providers.refreshModels.label")}
142+
</div>
143+
</Button>
144+
{refreshStatus === "loading" && (
145+
<div className="text-sm text-vscode-descriptionForeground">
146+
{t("settings:providers.refreshModels.loading")}
147+
</div>
148+
)}
149+
{refreshStatus === "success" && (
150+
<div className="text-sm text-vscode-foreground">{t("settings:providers.refreshModels.success")}</div>
151+
)}
152+
{refreshStatus === "error" && (
153+
<div className="text-sm text-vscode-errorForeground">
154+
{refreshError || t("settings:providers.refreshModels.error")}
155+
</div>
156+
)}
105157
<ModelPicker
106158
apiConfiguration={apiConfiguration}
107159
setApiConfigurationField={setApiConfigurationField}

0 commit comments

Comments
 (0)