Skip to content

Commit c75f184

Browse files
authored
feat: Hide static providers with no models from provider list (RooCodeInc#7392)
1 parent 2c82f4c commit c75f184

File tree

2 files changed

+317
-2
lines changed

2 files changed

+317
-2
lines changed

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,11 +379,37 @@ const ApiOptions = ({
379379

380380
// Convert providers to SearchableSelect options
381381
const providerOptions = useMemo(() => {
382-
return filterProviders(PROVIDERS, organizationAllowList).map(({ value, label }) => ({
382+
// First filter by organization allow list
383+
const allowedProviders = filterProviders(PROVIDERS, organizationAllowList)
384+
385+
// Then filter out static providers that have no models (unless currently selected)
386+
const providersWithModels = allowedProviders.filter(({ value }) => {
387+
// Always show the currently selected provider to avoid breaking existing configurations
388+
// Use apiConfiguration.apiProvider directly since that's what's actually selected
389+
if (value === apiConfiguration.apiProvider) {
390+
return true
391+
}
392+
393+
// Check if this is a static provider (has models in MODELS_BY_PROVIDER)
394+
const staticModels = MODELS_BY_PROVIDER[value as ProviderName]
395+
396+
// If it's a static provider, check if it has any models after filtering
397+
if (staticModels) {
398+
const filteredModels = filterModels(staticModels, value as ProviderName, organizationAllowList)
399+
// Hide the provider if it has no models after filtering
400+
return filteredModels && Object.keys(filteredModels).length > 0
401+
}
402+
403+
// If it's a dynamic provider (not in MODELS_BY_PROVIDER), always show it
404+
// to avoid race conditions with async model fetching
405+
return true
406+
})
407+
408+
return providersWithModels.map(({ value, label }) => ({
383409
value,
384410
label,
385411
}))
386-
}, [organizationAllowList])
412+
}, [organizationAllowList, apiConfiguration.apiProvider])
387413

388414
return (
389415
<div className="flex flex-col gap-3">
Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,289 @@
1+
import { describe, it, expect, vi } from "vitest"
2+
import { render, screen } from "@testing-library/react"
3+
import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
4+
import ApiOptions from "../ApiOptions"
5+
import { MODELS_BY_PROVIDER, PROVIDERS } from "../constants"
6+
import type { ProviderSettings } from "@roo-code/types"
7+
import type { OrganizationAllowList } from "@roo/cloud"
8+
import { useExtensionState } from "@src/context/ExtensionStateContext"
9+
import { useSelectedModel } from "@src/components/ui/hooks/useSelectedModel"
10+
11+
// Mock the extension state context
12+
vi.mock("@src/context/ExtensionStateContext", () => ({
13+
useExtensionState: vi.fn(() => ({
14+
organizationAllowList: undefined,
15+
cloudIsAuthenticated: false,
16+
})),
17+
}))
18+
19+
// Mock the translation hook
20+
vi.mock("@src/i18n/TranslationContext", () => ({
21+
useAppTranslation: () => ({
22+
t: (key: string) => key,
23+
}),
24+
}))
25+
26+
// Mock vscode
27+
vi.mock("@src/utils/vscode", () => ({
28+
vscode: {
29+
postMessage: vi.fn(),
30+
},
31+
}))
32+
33+
// Mock the router models hook
34+
vi.mock("@src/components/ui/hooks/useRouterModels", () => ({
35+
useRouterModels: () => ({
36+
data: null,
37+
refetch: vi.fn(),
38+
}),
39+
}))
40+
41+
// Mock the selected model hook
42+
vi.mock("@src/components/ui/hooks/useSelectedModel", () => ({
43+
useSelectedModel: vi.fn(() => ({
44+
provider: "anthropic",
45+
id: "claude-3-5-sonnet-20241022",
46+
info: null,
47+
})),
48+
}))
49+
50+
// Mock the OpenRouter model providers hook
51+
vi.mock("@src/components/ui/hooks/useOpenRouterModelProviders", () => ({
52+
useOpenRouterModelProviders: () => ({
53+
data: null,
54+
}),
55+
OPENROUTER_DEFAULT_PROVIDER_NAME: "Auto",
56+
}))
57+
58+
// Mock the SearchableSelect component to capture the options passed to it
59+
vi.mock("@src/components/ui", () => ({
60+
SearchableSelect: ({ options, ...props }: any) => {
61+
// Store the options in a data attribute for testing
62+
return (
63+
<div data-testid="searchable-select" data-options={JSON.stringify(options)} {...props}>
64+
{options.map((opt: any) => (
65+
<div key={opt.value} data-testid={`option-${opt.value}`}>
66+
{opt.label}
67+
</div>
68+
))}
69+
</div>
70+
)
71+
},
72+
Select: ({ children }: any) => <div>{children}</div>,
73+
SelectTrigger: ({ children }: any) => <div>{children}</div>,
74+
SelectValue: ({ placeholder }: any) => <div>{placeholder}</div>,
75+
SelectContent: ({ children }: any) => <div>{children}</div>,
76+
SelectItem: ({ children, value }: any) => <div data-value={value}>{children}</div>,
77+
Collapsible: ({ children }: any) => <div>{children}</div>,
78+
CollapsibleTrigger: ({ children }: any) => <div>{children}</div>,
79+
CollapsibleContent: ({ children }: any) => <div>{children}</div>,
80+
Slider: ({ children, ...props }: any) => <div {...props}>{children}</div>,
81+
Button: ({ children, ...props }: any) => <button {...props}>{children}</button>,
82+
}))
83+
84+
describe("ApiOptions Provider Filtering", () => {
85+
const queryClient = new QueryClient({
86+
defaultOptions: {
87+
queries: { retry: false },
88+
},
89+
})
90+
91+
const defaultProps = {
92+
uriScheme: "vscode",
93+
apiConfiguration: {
94+
apiProvider: "anthropic",
95+
apiKey: "test-key",
96+
} as ProviderSettings,
97+
setApiConfigurationField: vi.fn(),
98+
fromWelcomeView: false,
99+
errorMessage: undefined,
100+
setErrorMessage: vi.fn(),
101+
}
102+
103+
const renderWithProviders = (props = defaultProps) => {
104+
return render(
105+
<QueryClientProvider client={queryClient}>
106+
<ApiOptions {...props} />
107+
</QueryClientProvider>,
108+
)
109+
}
110+
111+
it("should show all providers when no organization allow list is provided", () => {
112+
renderWithProviders()
113+
114+
const selectElement = screen.getByTestId("provider-select")
115+
const options = JSON.parse(selectElement.getAttribute("data-options") || "[]")
116+
117+
// Should include both static and dynamic providers
118+
const providerValues = options.map((opt: any) => opt.value)
119+
expect(providerValues).toContain("anthropic") // static provider
120+
expect(providerValues).toContain("openrouter") // dynamic provider
121+
expect(providerValues).toContain("ollama") // dynamic provider
122+
})
123+
124+
it("should hide static providers with empty models", () => {
125+
// Mock MODELS_BY_PROVIDER to have an empty provider
126+
const _originalModels = { ...MODELS_BY_PROVIDER }
127+
;(MODELS_BY_PROVIDER as any).emptyProvider = {}
128+
129+
// Add the empty provider to PROVIDERS
130+
PROVIDERS.push({ value: "emptyProvider", label: "Empty Provider" })
131+
132+
renderWithProviders()
133+
134+
const selectElement = screen.getByTestId("provider-select")
135+
const options = JSON.parse(selectElement.getAttribute("data-options") || "[]")
136+
const providerValues = options.map((opt: any) => opt.value)
137+
138+
// Should NOT include the empty static provider
139+
expect(providerValues).not.toContain("emptyProvider")
140+
141+
// Cleanup
142+
delete (MODELS_BY_PROVIDER as any).emptyProvider
143+
PROVIDERS.pop()
144+
})
145+
146+
it("should always show dynamic providers even if they have no models yet", () => {
147+
renderWithProviders()
148+
149+
const selectElement = screen.getByTestId("provider-select")
150+
const options = JSON.parse(selectElement.getAttribute("data-options") || "[]")
151+
const providerValues = options.map((opt: any) => opt.value)
152+
153+
// Dynamic providers (not in MODELS_BY_PROVIDER) should always be shown
154+
expect(providerValues).toContain("openrouter")
155+
expect(providerValues).toContain("ollama")
156+
expect(providerValues).toContain("lmstudio")
157+
expect(providerValues).toContain("litellm")
158+
expect(providerValues).toContain("glama")
159+
expect(providerValues).toContain("unbound")
160+
expect(providerValues).toContain("requesty")
161+
expect(providerValues).toContain("io-intelligence")
162+
})
163+
164+
it("should filter static providers based on organization allow list", () => {
165+
// Create a mock organization allow list that only allows certain models
166+
const allowList: OrganizationAllowList = {
167+
allowAll: false,
168+
providers: {
169+
anthropic: {
170+
allowAll: false,
171+
models: ["claude-3-5-sonnet-20241022"], // Only allow one model
172+
},
173+
gemini: {
174+
allowAll: false,
175+
models: [], // No models allowed
176+
},
177+
openrouter: {
178+
allowAll: true, // Dynamic provider with all models allowed
179+
},
180+
},
181+
}
182+
183+
// Mock the extension state with the allow list
184+
vi.mocked(useExtensionState).mockReturnValue({
185+
organizationAllowList: allowList,
186+
cloudIsAuthenticated: false,
187+
} as any)
188+
189+
renderWithProviders()
190+
191+
const selectElement = screen.getByTestId("provider-select")
192+
const options = JSON.parse(selectElement.getAttribute("data-options") || "[]")
193+
const providerValues = options.map((opt: any) => opt.value)
194+
195+
// Should include anthropic (has allowed models)
196+
expect(providerValues).toContain("anthropic")
197+
198+
// Should NOT include gemini (no allowed models)
199+
expect(providerValues).not.toContain("gemini")
200+
201+
// Should include openrouter (dynamic provider)
202+
expect(providerValues).toContain("openrouter")
203+
204+
// Should NOT include providers not in the allow list
205+
expect(providerValues).not.toContain("openai-native")
206+
expect(providerValues).not.toContain("mistral")
207+
})
208+
209+
it("should show static provider when allowAll is true for that provider", () => {
210+
const allowList: OrganizationAllowList = {
211+
allowAll: false,
212+
providers: {
213+
anthropic: {
214+
allowAll: true, // Allow all models for this provider
215+
},
216+
},
217+
}
218+
219+
vi.mocked(useExtensionState).mockReturnValue({
220+
organizationAllowList: allowList,
221+
cloudIsAuthenticated: false,
222+
} as any)
223+
224+
renderWithProviders()
225+
226+
const selectElement = screen.getByTestId("provider-select")
227+
const options = JSON.parse(selectElement.getAttribute("data-options") || "[]")
228+
const providerValues = options.map((opt: any) => opt.value)
229+
230+
// Should include anthropic since allowAll is true
231+
expect(providerValues).toContain("anthropic")
232+
})
233+
234+
it("should always show currently selected provider even if it has no models", () => {
235+
// Add an empty static provider to test
236+
;(MODELS_BY_PROVIDER as any).testEmptyProvider = {}
237+
// Add the provider to the PROVIDERS list
238+
PROVIDERS.push({ value: "testEmptyProvider", label: "Test Empty Provider" })
239+
240+
// Create a mock organization allow list that allows the provider but no models
241+
const allowList: OrganizationAllowList = {
242+
allowAll: false,
243+
providers: {
244+
testEmptyProvider: {
245+
allowAll: true, // Allow the provider itself, but it has no models in MODELS_BY_PROVIDER
246+
},
247+
anthropic: {
248+
allowAll: true, // Allow anthropic for comparison
249+
},
250+
},
251+
}
252+
253+
vi.mocked(useExtensionState).mockReturnValue({
254+
organizationAllowList: allowList,
255+
cloudIsAuthenticated: false,
256+
} as any)
257+
258+
// Mock the selected model hook to return testEmptyProvider as the selected provider
259+
;(useSelectedModel as any).mockReturnValue({
260+
provider: "testEmptyProvider",
261+
id: undefined,
262+
info: null,
263+
})
264+
265+
// Render with testEmptyProvider as the selected provider
266+
const props = {
267+
...defaultProps,
268+
apiConfiguration: {
269+
...defaultProps.apiConfiguration,
270+
apiProvider: "testEmptyProvider" as any,
271+
} as ProviderSettings,
272+
}
273+
274+
renderWithProviders(props)
275+
276+
const selectElement = screen.getByTestId("provider-select")
277+
const options = JSON.parse(selectElement.getAttribute("data-options") || "[]")
278+
const providerValues = options.map((opt: any) => opt.value)
279+
280+
// Should include testEmptyProvider even though it has no models (empty object in MODELS_BY_PROVIDER), because it's currently selected
281+
expect(providerValues).toContain("testEmptyProvider")
282+
// Should also include anthropic since it has allowAll: true
283+
expect(providerValues).toContain("anthropic")
284+
285+
// Cleanup
286+
delete (MODELS_BY_PROVIDER as any).testEmptyProvider
287+
PROVIDERS.pop()
288+
})
289+
})

0 commit comments

Comments
 (0)