diff --git a/src/commands/model/model.test.tsx b/src/commands/model/model.test.tsx index 7cb5a82464..7516829ffb 100644 --- a/src/commands/model/model.test.tsx +++ b/src/commands/model/model.test.tsx @@ -281,7 +281,11 @@ test('descriptor model options include active profile configured models', async ]) }) -test('descriptor model options omit route defaults outside active profile models', async () => { +test('descriptor model options show all route models and append profile-only models', async () => { + // Regression test for #1423: when a provider profile only has one saved model, + // the /model picker must still show the full route catalog — not just that one + // model. Profile-configured models that are NOT already in the route catalog + // are appended at the end so they remain accessible. const activeProfile = { id: 'mistral-profile', name: 'Mistral AI', @@ -314,6 +318,10 @@ test('descriptor model options omit route defaults outside active profile models const { mergeActiveProfileModelOptions } = await importFreshModelModule('descriptor-profile-model-filter') + // Route has devstral-latest and mistral-small-latest. + // Profile has mistral-medium-latest (not in route) and mistral-small-latest (in route). + // Expected: all route options appear, plus mistral-medium-latest appended because + // it is only in the profile and not in the route catalog. expect( mergeActiveProfileModelOptions( 'mistral', @@ -332,15 +340,20 @@ test('descriptor model options omit route defaults outside active profile models ), ).toEqual([ { - value: 'mistral-medium-latest', - label: 'mistral-medium-latest', - description: 'Provider: Mistral AI', + value: 'devstral-latest', + label: 'Devstral Latest', + description: 'Recommended · Provider: Mistral AI', }, { value: 'mistral-small-latest', label: 'Mistral Small Latest', description: 'Provider: Mistral AI', }, + { + value: 'mistral-medium-latest', + label: 'mistral-medium-latest', + description: 'Provider: Mistral AI', + }, ]) }) @@ -474,6 +487,48 @@ test('/model refresh clears descriptor cache and reports updates', async () => { expect(messages).toContain('Updated OpenRouter models.') }) +test('descriptor optionsOverride filters out models not in availableModels allowlist', async () => { + // Verify that when availableModels is set in settings, the descriptor route options + // passed to mergeActiveProfileModelOptions are already filtered by isModelAllowed. + // We test this by mocking isModelAllowed to only allow a specific model, + // then confirming the output of mergeActiveProfileModelOptions (given pre-filtered + // input, as loadDescriptorDiscoveryContext now does) omits blocked models. + + mock.module('../../utils/model/modelAllowlist.js', () => ({ + isModelAllowed: (model: string) => model === 'openai/gpt-5-mini', + })) + + mock.module('../../utils/providerProfiles.js', () => ({ + getActiveOpenAIModelOptionsCache: () => [], + getActiveProviderProfile: () => null, + getProfileModelOptions: () => [], + setActiveOpenAIModelOptionsCache: () => {}, + })) + + const { mergeActiveProfileModelOptions } = await importFreshModelModule( + 'allowlist-filter-options', + ) + + // Simulate what loadDescriptorDiscoveryContext does after the fix: + // the full catalog has two models, but only the allowed one passes the filter. + const allCatalogOptions = [ + { value: 'openai/gpt-5-mini', label: 'GPT-5 Mini', description: 'Provider: OpenRouter' }, + { value: 'qwen/qwen3-32b', label: 'Qwen3 32B', description: 'Provider: OpenRouter' }, + ] + // This filter mirrors the fix applied in loadDescriptorDiscoveryContext + const isModelAllowedFn = (model: string) => model === 'openai/gpt-5-mini' + const filteredOptions = allCatalogOptions.filter(o => + typeof o.value === 'string' ? isModelAllowedFn(o.value) : true + ) + + // mergeActiveProfileModelOptions with no active profile just returns the input + const result = mergeActiveProfileModelOptions('openrouter', filteredOptions) + + const resultValues = result.map(o => o.value) + expect(resultValues).toContain('openai/gpt-5-mini') + expect(resultValues).not.toContain('qwen/qwen3-32b') +}) + test('/model does not auto-refresh descriptor models when nonessential traffic is disabled', async () => { process.env.CLAUDE_CODE_USE_OPENAI = '1' process.env.OPENAI_BASE_URL = 'https://openrouter.ai/api/v1' diff --git a/src/commands/model/model.tsx b/src/commands/model/model.tsx index 00c123574d..8df4d0618e 100644 --- a/src/commands/model/model.tsx +++ b/src/commands/model/model.tsx @@ -136,15 +136,19 @@ export function mergeActiveProfileModelOptions( return routeOptions } - const routeOptionsByValue = new Map( + // Always show the full route catalog so users can see and switch to any + // available model (#1423). Additionally append any profile-configured models + // that are not already present in the route catalog (e.g. a custom model ID + // the user saved in their provider profile). + const routeOptionKeys = new Set( routeOptions.flatMap(option => { const value = typeof option.value === 'string' ? option.value.trim().toLowerCase() : '' - return value ? [[value, option] as const] : [] + return value ? [value] : [] }), ) - const merged: ModelOption[] = [] - const seen = new Set() + const merged: ModelOption[] = [...routeOptions] + const seen = new Set(routeOptionKeys) for (const option of profileOptions) { const value = typeof option.value === 'string' ? option.value.trim() : '' @@ -152,9 +156,16 @@ export function mergeActiveProfileModelOptions( if (!value || seen.has(key)) { continue } + // Don't surface profile-configured models that are explicitly blocked by + // the availableModels allowlist. The catalog entries feeding routeOptions + // were already filtered upstream; without this guard, a blocked model + // absent from the filtered catalog would be re-added here from profileOptions. + if (!isModelAllowed(value)) { + continue + } seen.add(key) - merged.push(routeOptionsByValue.get(key) ?? option) + merged.push(option) } return merged @@ -242,12 +253,15 @@ async function loadDescriptorDiscoveryContext( staticEntries, routeDefaultModel, ) + const allowedRouteOptions = routeOptions.filter(o => + typeof o.value === 'string' ? isModelAllowed(o.value) : true + ) return { kind: 'descriptor', autoRefresh: false, canRefresh, - optionsOverride: mergeActiveProfileModelOptions(routeId, routeOptions), + optionsOverride: mergeActiveProfileModelOptions(routeId, allowedRouteOptions), routeId, routeDefaultModel, routeLabel, @@ -289,13 +303,16 @@ async function loadDescriptorDiscoveryContext( mergedEntries, routeDefaultModel, ) + const allowedRouteOptions = routeOptions.filter(o => + typeof o.value === 'string' ? isModelAllowed(o.value) : true + ) return { kind: 'descriptor', autoRefresh, canRefresh, discoveryState, - optionsOverride: mergeActiveProfileModelOptions(routeId, routeOptions), + optionsOverride: mergeActiveProfileModelOptions(routeId, allowedRouteOptions), routeId, routeDefaultModel, routeLabel, @@ -435,6 +452,11 @@ function ModelPickerWrapper({ } const handleSelect = (model: string | null, effort: EffortLevel | undefined) => { + if (model && !isModelAllowed(model)) { + onDone(`Model '${model}' is not available. Your organization restricts model selection.`, { display: 'system' }) + return + } + logEvent('tengu_model_command_menu', { action: String(model) as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, from_model: String(mainLoopModel) as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, @@ -517,13 +539,21 @@ function ModelPickerWrapper({ }, ) + const discoveredRouteOptions = buildRouteCatalogModelOptions( + discoveryContext.routeLabel, + result?.models ?? [], + discoveryContext.routeDefaultModel, + ) + // Apply the same allowlist filter used at initial load so models blocked + // by availableModels don't slip back in after a refresh. Profile-only + // models added by mergeActiveProfileModelOptions are not filtered here + // (same as the initial-load path at line ~250). + const allowedRouteOptions = discoveredRouteOptions.filter(o => + typeof o.value === 'string' ? isModelAllowed(o.value) : true, + ) const nextOptions = mergeActiveProfileModelOptions( discoveryContext.routeId, - buildRouteCatalogModelOptions( - discoveryContext.routeLabel, - result?.models ?? [], - discoveryContext.routeDefaultModel, - ), + allowedRouteOptions, ) const changed = !haveSameModelOptions(optionsOverride ?? [], nextOptions) @@ -793,13 +823,20 @@ async function refreshModelsAndSummarize(): Promise { ...getOpenAIDiscoveryRequestOptions(discoveryContext.routeId), forceRefresh: true, }) + const discoveredRefreshOptions = buildRouteCatalogModelOptions( + discoveryContext.routeLabel, + result?.models ?? [], + discoveryContext.routeDefaultModel, + ) + // Apply the same allowlist that filters initial load and interactive refresh + // so the "changed" comparison is apples-to-apples with the filtered + // discoveryContext.optionsOverride and blocked models don't count as a change. + const allowedRefreshOptions = discoveredRefreshOptions.filter(o => + typeof o.value === 'string' ? isModelAllowed(o.value) : true, + ) const nextOptions = mergeActiveProfileModelOptions( discoveryContext.routeId, - buildRouteCatalogModelOptions( - discoveryContext.routeLabel, - result?.models ?? [], - discoveryContext.routeDefaultModel, - ), + allowedRefreshOptions, ) const changed = !haveSameModelOptions( discoveryContext.optionsOverride,