Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 59 additions & 4 deletions src/commands/model/model.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
},
])
})

Expand Down Expand Up @@ -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'
Expand Down
71 changes: 54 additions & 17 deletions src/commands/model/model.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,25 +136,36 @@ 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<string>()
const merged: ModelOption[] = [...routeOptions]
const seen = new Set<string>(routeOptionKeys)

for (const option of profileOptions) {
const value = typeof option.value === 'string' ? option.value.trim() : ''
const key = value.toLowerCase()
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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -793,13 +823,20 @@ async function refreshModelsAndSummarize(): Promise<string> {
...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,
Expand Down