Skip to content

Commit 4203a2d

Browse files
committed
fix: LiteLLM provider model desync after Sync Models or settings navigation
- Invalidate React Query router-models cache on successful sync in LiteLLM.tsx so useSelectedModel picks up the refreshed list (mirrors Poe.tsx fix) - Preserve configured litellm model ID when model list is empty/loading in useSelectedModel.ts instead of silently falling back to hardcoded default - Pass litellm credentials in the debounced requestRouterModels message in ApiOptions.tsx to prevent fetches with stale config from clearing the model list
1 parent e8acc6a commit 4203a2d

8 files changed

Lines changed: 186 additions & 42 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"zoo-code": patch
3+
---
4+
5+
Fix LiteLLM provider cache key collision, credential priority, and model-selection fallback to non-existent default.
6+
7+
Two bugs are addressed:
8+
9+
1. **Cache key collision**: All URL-scoped providers (LiteLLM, Ollama, LM Studio, Poe, DeepSeek,
10+
Requesty) previously shared one cache entry keyed only on the provider name. Switching between
11+
profiles backed by different servers silently served the wrong model list and the stale list
12+
persisted across VS Code restarts via the disk cache. Fixed with a compound cache key:
13+
URL-scoped providers use `provider:baseUrl`; key-scoped providers (LiteLLM, Poe, Requesty)
14+
additionally include a short sha256 hash of the API key (`provider:baseUrl:sha256(apiKey)`) so that
15+
two different API keys on the same server never share a cache entry (relevant when the server
16+
enforces per-key model allowlists). The `RouterProvider.getModel()` cold-start fallback is also
17+
corrected to pass the full options so it resolves the same compound key.
18+
19+
2. **Silent fallback to hardcoded default**: When the LiteLLM model list was empty (due to the
20+
collision above, a failed sync, or a transient error), `useSelectedModel` reset the configured
21+
model ID to `claude-3-7-sonnet-20250219` -- a model that typically does not exist on user
22+
LiteLLM servers. Four sub-fixes: preserve the configured model ID when the list is empty;
23+
invalidate the React Query router-models cache after a successful "Sync Models" click; pass the
24+
current LiteLLM credentials in the debounced `requestRouterModels` message; and correct the
25+
credential priority in `webviewMessageHandler.ts` so that message values (current unsaved field
26+
state) take precedence over stale saved config, matching the pattern already used for DeepSeek.

src/api/providers/fetchers/modelCache.ts

Lines changed: 96 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as path from "path"
22
import fs from "fs/promises"
33
import * as fsSync from "fs"
4+
import { createHash } from "crypto"
45

56
import NodeCache from "node-cache"
67
import { z } from "zod"
@@ -34,28 +35,86 @@ const memoryCache = new NodeCache({ stdTTL: 5 * 60, checkperiod: 5 * 60 })
3435
// Zod schema for validating ModelRecord structure from disk cache
3536
const modelRecordSchema = z.record(z.string(), modelInfoSchema)
3637

37-
// Track in-flight refresh requests to prevent concurrent API calls for the same provider
38-
// This prevents race conditions where multiple calls might overwrite each other's results
39-
const inFlightRefresh = new Map<RouterName, Promise<ModelRecord>>()
38+
// Track in-flight refresh requests to prevent concurrent API calls for the same provider+url.
39+
// Keyed on the compound cache key (see getCacheKey) so that two different LiteLLM servers never
40+
// deduplicate each other's in-flight refreshes.
41+
const inFlightRefresh = new Map<string, Promise<ModelRecord>>()
4042

4143
// Providers whose model lists are scoped to the signed-in user (e.g. per-account
4244
// allowlists or org policies). For these we MUST NOT cache results on disk or
4345
// in memory: a sign-in/out cycle could otherwise serve a previous user's model
4446
// list to the next user, and stale data could mask backend allowlist updates.
4547
const AUTH_SCOPED_PROVIDERS: ReadonlySet<RouterName> = new Set(["zoo-gateway"])
4648

49+
// Providers whose model list is determined by the server URL, not just by the provider name.
50+
// Each unique baseUrl must be cached independently so that switching endpoints never serves
51+
// stale results from a previously-cached server.
52+
const URL_SCOPED_PROVIDERS: ReadonlySet<RouterName> = new Set([
53+
"litellm",
54+
"poe",
55+
"deepseek",
56+
"ollama",
57+
"lmstudio",
58+
"requesty",
59+
])
60+
61+
// Providers where the API key itself determines which models are visible (e.g. per-key
62+
// allowlists on a shared proxy). For these the cache key also includes a short hash of
63+
// the API key so that two different keys on the same server never share a cache entry.
64+
const KEY_SCOPED_PROVIDERS: ReadonlySet<RouterName> = new Set([
65+
"litellm", // Per-key model allowlists are a first-class LiteLLM proxy feature
66+
"poe", // Per-account model availability
67+
"requesty", // Per-account custom model policies
68+
])
69+
4770
function isAuthScopedProvider(provider: RouterName): boolean {
4871
return AUTH_SCOPED_PROVIDERS.has(provider)
4972
}
5073

51-
async function writeModels(router: RouterName, data: ModelRecord) {
52-
const filename = `${router}_models.json`
74+
/**
75+
* Build a cache key that is unique per provider+server+key combination.
76+
*
77+
* - URL-scoped providers include the normalized baseUrl so that two different servers
78+
* of the same provider type never share a cache entry.
79+
* - Key-scoped providers additionally fold in a short sha256 hash of the API key so that
80+
* two different API keys on the same server never share a cache entry (relevant when
81+
* the server enforces per-key model allowlists, e.g. LiteLLM, Poe, Requesty).
82+
*/
83+
function getCacheKey(options: GetModelsOptions): string {
84+
const { provider } = options
85+
const isUrlScoped = URL_SCOPED_PROVIDERS.has(provider as RouterName)
86+
const isKeyScoped = KEY_SCOPED_PROVIDERS.has(provider as RouterName)
87+
88+
if (isUrlScoped && options.baseUrl) {
89+
// Strip trailing slashes so "http://host:4000/" and "http://host:4000" map to the same key.
90+
const normalizedUrl = options.baseUrl.replace(/\/+$/, "")
91+
if (isKeyScoped && options.apiKey) {
92+
// Short (16-char) sha256 prefix -- enough to make collisions effectively impossible
93+
// while keeping filenames readable. We do not need the full digest here.
94+
const keyHash = createHash("sha256").update(options.apiKey).digest("hex").slice(0, 16)
95+
return `${provider}:${normalizedUrl}:${keyHash}`
96+
}
97+
return `${provider}:${normalizedUrl}`
98+
}
99+
return provider
100+
}
101+
102+
/**
103+
* Convert a cache key to a filesystem-safe filename component.
104+
* Replaces characters that are illegal or awkward in filenames with underscores.
105+
*/
106+
function cacheKeyToFilename(cacheKey: string): string {
107+
return cacheKey.replace(/[:/\\?#*<>|"\s]+/g, "_")
108+
}
109+
110+
async function writeModels(cacheKey: string, data: ModelRecord) {
111+
const filename = `${cacheKeyToFilename(cacheKey)}_models.json`
53112
const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
54113
await safeWriteJson(path.join(cacheDir, filename), data)
55114
}
56115

57-
async function readModels(router: RouterName): Promise<ModelRecord | undefined> {
58-
const filename = `${router}_models.json`
116+
async function readModels(cacheKey: string): Promise<ModelRecord | undefined> {
117+
const filename = `${cacheKeyToFilename(cacheKey)}_models.json`
59118
const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
60119
const filePath = path.join(cacheDir, filename)
61120
const exists = await fileExistsAtPath(filePath)
@@ -133,10 +192,11 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise<Model
133192
*/
134193
export const getModels = async (options: GetModelsOptions): Promise<ModelRecord> => {
135194
const { provider } = options
195+
const cacheKey = getCacheKey(options)
136196

137197
const shouldSkipCache = isAuthScopedProvider(provider)
138198

139-
let models = shouldSkipCache ? undefined : getModelsFromCache(provider)
199+
let models = shouldSkipCache ? undefined : getModelsFromCache(options)
140200

141201
if (models) {
142202
return models
@@ -149,10 +209,10 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
149209
// Only cache non-empty results so a failed API response doesn't get persisted
150210
// as if the provider had no models. Auth-scoped providers skip caching entirely.
151211
if (modelCount > 0 && !shouldSkipCache) {
152-
memoryCache.set(provider, models)
212+
memoryCache.set(cacheKey, models)
153213

154-
await writeModels(provider, models).catch((err) =>
155-
console.error(`[MODEL_CACHE] Error writing ${provider} models to file cache:`, err),
214+
await writeModels(cacheKey, models).catch((err) =>
215+
console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err),
156216
)
157217
} else if (modelCount === 0) {
158218
TelemetryService.instance.captureEvent(TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, {
@@ -182,17 +242,18 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
182242
*/
183243
export const refreshModels = async (options: GetModelsOptions): Promise<ModelRecord> => {
184244
const { provider } = options
245+
const cacheKey = getCacheKey(options)
185246

186247
const shouldSkipCache = isAuthScopedProvider(provider)
187248

188-
// Check if there's already an in-flight refresh for this provider.
249+
// Check if there's already an in-flight refresh for this provider+url combination.
189250
// This prevents race conditions where multiple concurrent refreshes might
190251
// overwrite each other's results. Skip de-duplication for auth-scoped
191252
// providers because two concurrent calls may carry different tokens
192253
// (e.g., after a sign-out/sign-in within the same session) and we must
193254
// not return the first caller's results to the second caller.
194255
if (!shouldSkipCache) {
195-
const existingRequest = inFlightRefresh.get(provider)
256+
const existingRequest = inFlightRefresh.get(cacheKey)
196257
if (existingRequest) {
197258
return existingRequest
198259
}
@@ -206,7 +267,7 @@ export const refreshModels = async (options: GetModelsOptions): Promise<ModelRec
206267
const modelCount = Object.keys(models).length
207268

208269
// Get existing cached data for comparison
209-
const existingCache = shouldSkipCache ? undefined : getModelsFromCache(provider)
270+
const existingCache = shouldSkipCache ? undefined : getModelsFromCache(options)
210271
const existingCount = existingCache ? Object.keys(existingCache).length : 0
211272

212273
if (modelCount === 0) {
@@ -224,34 +285,34 @@ export const refreshModels = async (options: GetModelsOptions): Promise<ModelRec
224285
}
225286

226287
if (!shouldSkipCache) {
227-
memoryCache.set(provider, models)
288+
memoryCache.set(cacheKey, models)
228289

229-
await writeModels(provider, models).catch((err) =>
230-
console.error(`[refreshModels] Error writing ${provider} models to disk:`, err),
290+
await writeModels(cacheKey, models).catch((err) =>
291+
console.error(`[refreshModels] Error writing ${cacheKey} models to disk:`, err),
231292
)
232293
}
233294

234295
return models
235296
} catch (error) {
236297
// Log the error for debugging, then return existing cache if available (graceful degradation).
237298
// For auth-scoped providers (zoo-gateway) we MUST NOT return cached models from a prior
238-
// session, since they could belong to a different user return empty instead.
239-
console.error(`[refreshModels] Failed to refresh ${provider} models:`, error)
299+
// session, since they could belong to a different user -- return empty instead.
300+
console.error(`[refreshModels] Failed to refresh ${cacheKey} models:`, error)
240301
if (shouldSkipCache) {
241302
return {}
242303
}
243-
return getModelsFromCache(provider) || {}
304+
return getModelsFromCache(options) || {}
244305
} finally {
245306
// Always clean up the in-flight tracking
246307
if (!shouldSkipCache) {
247-
inFlightRefresh.delete(provider)
308+
inFlightRefresh.delete(cacheKey)
248309
}
249310
}
250311
})()
251312

252313
// Track the in-flight request (auth-scoped providers are excluded; see above).
253314
if (!shouldSkipCache) {
254-
inFlightRefresh.set(provider, refreshPromise)
315+
inFlightRefresh.set(cacheKey, refreshPromise)
255316
}
256317

257318
return refreshPromise
@@ -290,16 +351,17 @@ export async function initializeModelCacheRefresh(): Promise<void> {
290351
* @param refresh - If true, immediately fetch fresh data from API
291352
*/
292353
export const flushModels = async (options: GetModelsOptions, refresh: boolean = false): Promise<void> => {
293-
const { provider } = options
294354
if (refresh) {
295355
// Don't delete memory cache - let refreshModels atomically replace it
296356
// This prevents a race condition where getModels() might be called
297357
// before refresh completes, avoiding a gap in cache availability
298358
// Await the refresh to ensure the cache is updated before returning
299359
await refreshModels(options)
300360
} else {
301-
// Only delete memory cache when not refreshing
302-
memoryCache.del(provider)
361+
// Only delete memory cache when not refreshing. Use the compound cache key so that
362+
// URL-scoped providers (litellm, poe, etc.) actually evict the per-server entry rather
363+
// than a bare provider-name entry that was never written.
364+
memoryCache.del(getCacheKey(options))
303365
}
304366
}
305367

@@ -311,17 +373,20 @@ export const flushModels = async (options: GetModelsOptions, refresh: boolean =
311373
* @param provider - The provider to get models for.
312374
* @returns Models from memory cache, disk cache, or undefined if not cached.
313375
*/
314-
export function getModelsFromCache(provider: ProviderName): ModelRecord | undefined {
376+
export function getModelsFromCache(
377+
options: GetModelsOptions | ProviderName,
378+
): ModelRecord | undefined {
379+
const cacheKey = typeof options === "string" ? options : getCacheKey(options)
315380
// Check memory cache first (fast)
316-
const memoryModels = memoryCache.get<ModelRecord>(provider)
381+
const memoryModels = memoryCache.get<ModelRecord>(cacheKey)
317382
if (memoryModels) {
318383
return memoryModels
319384
}
320385

321386
// Memory cache miss - try to load from disk synchronously
322387
// This is acceptable because it only happens on cold start or after cache expiry
323388
try {
324-
const filename = `${provider}_models.json`
389+
const filename = `${cacheKeyToFilename(cacheKey)}_models.json`
325390
const cacheDir = getCacheDirectoryPathSync()
326391
if (!cacheDir) {
327392
return undefined
@@ -339,19 +404,19 @@ export function getModelsFromCache(provider: ProviderName): ModelRecord | undefi
339404
const validation = modelRecordSchema.safeParse(models)
340405
if (!validation.success) {
341406
console.error(
342-
`[MODEL_CACHE] Invalid disk cache data structure for ${provider}:`,
407+
`[MODEL_CACHE] Invalid disk cache data structure for ${cacheKey}:`,
343408
validation.error.format(),
344409
)
345410
return undefined
346411
}
347412

348413
// Populate memory cache for future fast access
349-
memoryCache.set(provider, validation.data)
414+
memoryCache.set(cacheKey, validation.data)
350415

351416
return validation.data
352417
}
353418
} catch (error) {
354-
console.error(`[MODEL_CACHE] Error loading ${provider} models from disk:`, error)
419+
console.error(`[MODEL_CACHE] Error loading ${cacheKey} models from disk:`, error)
355420
}
356421

357422
return undefined

src/api/providers/router-provider.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,14 @@ export abstract class RouterProvider extends BaseProvider {
6969
return { id, info: this.models[id] }
7070
}
7171

72-
// Fall back to global cache (synchronous disk/memory cache)
73-
// This ensures models are available before fetchModel() is called
74-
const cachedModels = getModelsFromCache(this.name)
72+
// Fall back to global cache (synchronous disk/memory cache).
73+
// Pass the full options so URL-scoped providers (litellm, ollama, etc.)
74+
// resolve the same compound cache key that fetchModel() wrote under.
75+
const cachedModels = getModelsFromCache({
76+
provider: this.name,
77+
baseUrl: this.client.baseURL,
78+
apiKey: this.client.apiKey,
79+
})
7580
if (cachedModels?.[id]) {
7681
// Also populate instance models for future calls
7782
this.models = cachedModels

src/core/webview/webviewMessageHandler.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,9 +1074,11 @@ export const webviewMessageHandler = async (
10741074
},
10751075
]
10761076

1077-
// LiteLLM is conditional on baseUrl+apiKey
1078-
const litellmApiKey = apiConfiguration.litellmApiKey || message?.values?.litellmApiKey
1079-
const litellmBaseUrl = apiConfiguration.litellmBaseUrl || message?.values?.litellmBaseUrl
1077+
// LiteLLM is conditional on baseUrl+apiKey.
1078+
// Prefer explicit values from message (current unsaved field state) over saved config,
1079+
// matching the pattern used for DeepSeek and other credential-carrying providers.
1080+
const litellmApiKey = message?.values?.litellmApiKey ?? apiConfiguration.litellmApiKey
1081+
const litellmBaseUrl = message?.values?.litellmBaseUrl ?? apiConfiguration.litellmBaseUrl
10801082

10811083
if (litellmApiKey && litellmBaseUrl) {
10821084
// If explicit credentials are provided in message.values (from Refresh Models button),

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,15 @@ const ApiOptions = ({
222222
requestLmStudioModels(apiConfiguration?.lmStudioBaseUrl)
223223
} else if (selectedProvider === "vscode-lm") {
224224
vscode.postMessage({ type: "requestVsCodeLmModels" })
225-
} else if (selectedProvider === "litellm" || selectedProvider === "poe") {
225+
} else if (selectedProvider === "litellm") {
226+
vscode.postMessage({
227+
type: "requestRouterModels",
228+
values: {
229+
litellmApiKey: apiConfiguration?.litellmApiKey,
230+
litellmBaseUrl: apiConfiguration?.litellmBaseUrl,
231+
},
232+
})
233+
} else if (selectedProvider === "poe") {
226234
vscode.postMessage({ type: "requestRouterModels" })
227235
}
228236
},

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useCallback, useState, useEffect, useRef } from "react"
22
import { VSCodeTextField, VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react"
3+
import { useQueryClient } from "@tanstack/react-query"
34

45
import {
56
type ProviderSettings,
@@ -34,6 +35,7 @@ export const LiteLLM = ({
3435
simplifySettings,
3536
}: LiteLLMProps) => {
3637
const { t } = useAppTranslation()
38+
const queryClient = useQueryClient()
3739
const { routerModels } = useExtensionState()
3840
const [refreshStatus, setRefreshStatus] = useState<"idle" | "loading" | "success" | "error">("idle")
3941
const [refreshError, setRefreshError] = useState<string | undefined>()
@@ -55,6 +57,9 @@ export const LiteLLM = ({
5557
if (refreshStatus === "loading") {
5658
if (!litellmErrorJustReceived.current) {
5759
setRefreshStatus("success")
60+
// Invalidate the react-query router models cache so
61+
// useSelectedModel picks up the refreshed list.
62+
queryClient.invalidateQueries({ queryKey: ["routerModels"] })
5863
}
5964
// If litellmErrorJustReceived.current is true, status is already (or will be) "error".
6065
}
@@ -65,7 +70,7 @@ export const LiteLLM = ({
6570
return () => {
6671
window.removeEventListener("message", handleMessage)
6772
}
68-
}, [refreshStatus, refreshError, setRefreshStatus, setRefreshError])
73+
}, [refreshStatus, queryClient])
6974

7075
const handleInputChange = useCallback(
7176
<K extends keyof ProviderSettings, E>(

0 commit comments

Comments
 (0)