Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Nov 28, 2025

This PR fixes the LiteLLM cache flush issue where the "Refresh Models" button was causing an "Invalid URL" error.

Problem

When refreshing LiteLLM models, the cache flush was not receiving the required apiKey and baseUrl parameters, resulting in an "Invalid URL" error in the getLiteLLMModels function.

Solution

  • Updated flushModels() in modelCache.ts to accept an optional GetModelsOptions parameter
  • Modified the two locations where LiteLLM calls flushModels() to pass the required credentials:
    1. In the flushRouterModels message handler
    2. When requesting models with new credentials

Changes

  • src/api/providers/fetchers/modelCache.ts: Added optional options parameter to flushModels()
  • src/core/webview/webviewMessageHandler.ts: Pass LiteLLM configuration when flushing cache

Fixes #9682


Important

Fixes LiteLLM cache flush by passing credentials in flushModels() to prevent "Invalid URL" error.

  • Behavior:
    • Fixes "Invalid URL" error in getLiteLLMModels by passing apiKey and baseUrl when flushing cache.
    • Updates flushModels() in modelCache.ts to accept GetModelsOptions parameter.
    • Modifies flushRouterModels and model requests with new credentials in webviewMessageHandler.ts to pass LiteLLM credentials.
  • Functions:
    • flushModels() in modelCache.ts now accepts an optional options parameter for credentials.
    • webviewMessageHandler.ts updates flushModels() calls to include LiteLLM credentials when necessary.

This description was created by Ellipsis for f3df489. You can customize this summary. It will automatically update as commits are pushed.

When refreshing LiteLLM models, the cache flush was not receiving the
required apiKey and baseUrl parameters, causing an 'Invalid URL' error.

This fix:
- Updates flushModels() to accept optional GetModelsOptions parameter
- Passes LiteLLM credentials when flushing cache in both places:
  1. When explicitly refreshing via flushRouterModels message
  2. When requesting models with new credentials

Fixes #9682
@roomote roomote bot requested review from cte, jr and mrubens as code owners November 28, 2025 22:54
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Nov 28, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Nov 28, 2025

Rooviewer Clock   See task on Roo Cloud

Review completed. No issues found.

The PR correctly fixes the LiteLLM cache flush issue by:

  • Adding an optional options parameter to flushModels() to accept credentials
  • Passing LiteLLM credentials (apiKey and baseUrl) when flushing the cache in the requestRouterModels handler
  • Maintaining backward compatibility with other providers

The implementation follows existing code patterns and properly addresses the root cause described in issue #9682.

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 28, 2025
@tl-hbk
Copy link

tl-hbk commented Dec 3, 2025

I tested this fix locally with 2 different litellm instances. While this fixes the flushing the cache, when switching between different configuration profiles, if the litellm url returns different models for the profile, it takes 2 clicks of "Refresh Models" for the new models to actually render in the drop down.

@pdecat
Copy link
Contributor

pdecat commented Dec 5, 2025

I tested this fix locally with 2 different litellm instances. While this fixes the flushing the cache, when switching between different configuration profiles, if the litellm url returns different models for the profile, it takes 2 clicks of "Refresh Models" for the new models to actually render in the drop down.

Hi, I've submitted an alternative PR to tackle that issue, PTAL: #9870

@tl-hbk
Copy link

tl-hbk commented Dec 5, 2025

I tested this fix locally with 2 different litellm instances. While this fixes the flushing the cache, when switching between different configuration profiles, if the litellm url returns different models for the profile, it takes 2 clicks of "Refresh Models" for the new models to actually render in the drop down.

Hi, I've submitted an alternative PR to tackle that issue, PTAL: #9870

Awesome thanks! I just tested out your PR as well to see if that rendering issue I mentioned still happens and it does. Rather than clicking refresh models twice though I noticed that if I clicked refresh models then open the drop down and click some random model, the next time I opened the drop down again it would be rendered with the models from the previous refresh.

@pdecat
Copy link
Contributor

pdecat commented Dec 5, 2025

Awesome thanks! I just tested out your PR as well to see if that rendering issue I mentioned still happens and it does. Rather than clicking refresh models twice though I noticed that if I clicked refresh models then open the drop down and click some random model, the next time I opened the drop down again it would be rendered with the models from the previous refresh.

I believe the fix for that is to await the call to refreshModels():

diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts
index e07b124a0..f98817f4c 100644
--- a/src/api/providers/fetchers/modelCache.ts
+++ b/src/api/providers/fetchers/modelCache.ts
@@ -281,9 +281,8 @@ export const flushModels = async (options: GetModelsOptions, refresh: boolean =
                // Don't delete memory cache - let refreshModels atomically replace it
                // This prevents a race condition where getModels() might be called
                // before refresh completes, avoiding a gap in cache availability
-               refreshModels(options).catch((error) => {
-                       console.error(`[flushModels] Refresh failed for ${provider}:`, error)
-               })
+               // Await the refresh to ensure the cache is updated before returning
+               await refreshModels(options)
        } else {
                // Only delete memory cache when not refreshing
                memoryCache.del(provider)

However, I cannot test that right now.

@tl-hbk
Copy link

tl-hbk commented Dec 5, 2025

Awesome thanks! I just tested out your PR as well to see if that rendering issue I mentioned still happens and it does. Rather than clicking refresh models twice though I noticed that if I clicked refresh models then open the drop down and click some random model, the next time I opened the drop down again it would be rendered with the models from the previous refresh.

I believe the fix for that is to await the call to refreshModels():

diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts
index e07b124a0..f98817f4c 100644
--- a/src/api/providers/fetchers/modelCache.ts
+++ b/src/api/providers/fetchers/modelCache.ts
@@ -281,9 +281,8 @@ export const flushModels = async (options: GetModelsOptions, refresh: boolean =
                // Don't delete memory cache - let refreshModels atomically replace it
                // This prevents a race condition where getModels() might be called
                // before refresh completes, avoiding a gap in cache availability
-               refreshModels(options).catch((error) => {
-                       console.error(`[flushModels] Refresh failed for ${provider}:`, error)
-               })
+               // Await the refresh to ensure the cache is updated before returning
+               await refreshModels(options)
        } else {
                // Only delete memory cache when not refreshing
                memoryCache.del(provider)

However, I cannot test that right now.

I just tested that and it did fix it thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

[BUG] LiteLLM cache flush doesn't work

5 participants