fix(models): prevent /models crash from non-string saved model values (#691)
* fix(models): guard GitHub default model setting against non-string values * test(models): avoid brittle GitHub default assertion in model guard test
This commit is contained in:
committed by
GitHub
parent
c207cdbdcc
commit
6b2121da12
57
src/utils/model/model.github.test.ts
Normal file
57
src/utils/model/model.github.test.ts
Normal file
@@ -0,0 +1,57 @@
|
|||||||
|
import { afterEach, beforeEach, expect, test } from 'bun:test'
|
||||||
|
|
||||||
|
import { saveGlobalConfig } from '../config.js'
|
||||||
|
import { getDefaultMainLoopModelSetting, getUserSpecifiedModelSetting } from './model.js'
|
||||||
|
|
||||||
|
const env = {
|
||||||
|
CLAUDE_CODE_USE_GITHUB: process.env.CLAUDE_CODE_USE_GITHUB,
|
||||||
|
CLAUDE_CODE_USE_OPENAI: process.env.CLAUDE_CODE_USE_OPENAI,
|
||||||
|
CLAUDE_CODE_USE_GEMINI: process.env.CLAUDE_CODE_USE_GEMINI,
|
||||||
|
CLAUDE_CODE_USE_BEDROCK: process.env.CLAUDE_CODE_USE_BEDROCK,
|
||||||
|
CLAUDE_CODE_USE_VERTEX: process.env.CLAUDE_CODE_USE_VERTEX,
|
||||||
|
CLAUDE_CODE_USE_FOUNDRY: process.env.CLAUDE_CODE_USE_FOUNDRY,
|
||||||
|
OPENAI_MODEL: process.env.OPENAI_MODEL,
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
process.env.CLAUDE_CODE_USE_GITHUB = '1'
|
||||||
|
delete process.env.CLAUDE_CODE_USE_OPENAI
|
||||||
|
delete process.env.CLAUDE_CODE_USE_GEMINI
|
||||||
|
delete process.env.CLAUDE_CODE_USE_BEDROCK
|
||||||
|
delete process.env.CLAUDE_CODE_USE_VERTEX
|
||||||
|
delete process.env.CLAUDE_CODE_USE_FOUNDRY
|
||||||
|
delete process.env.OPENAI_MODEL
|
||||||
|
saveGlobalConfig(current => ({
|
||||||
|
...current,
|
||||||
|
model: ({ bad: true } as unknown) as string,
|
||||||
|
}))
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
process.env.CLAUDE_CODE_USE_GITHUB = env.CLAUDE_CODE_USE_GITHUB
|
||||||
|
process.env.CLAUDE_CODE_USE_OPENAI = env.CLAUDE_CODE_USE_OPENAI
|
||||||
|
process.env.CLAUDE_CODE_USE_GEMINI = env.CLAUDE_CODE_USE_GEMINI
|
||||||
|
process.env.CLAUDE_CODE_USE_BEDROCK = env.CLAUDE_CODE_USE_BEDROCK
|
||||||
|
process.env.CLAUDE_CODE_USE_VERTEX = env.CLAUDE_CODE_USE_VERTEX
|
||||||
|
process.env.CLAUDE_CODE_USE_FOUNDRY = env.CLAUDE_CODE_USE_FOUNDRY
|
||||||
|
process.env.OPENAI_MODEL = env.OPENAI_MODEL
|
||||||
|
saveGlobalConfig(current => ({
|
||||||
|
...current,
|
||||||
|
model: undefined,
|
||||||
|
}))
|
||||||
|
})
|
||||||
|
|
||||||
|
test('github default model setting ignores non-string saved model', () => {
|
||||||
|
const model = getDefaultMainLoopModelSetting()
|
||||||
|
expect(typeof model).toBe('string')
|
||||||
|
expect(model).not.toBe('[object Object]')
|
||||||
|
expect(model.length).toBeGreaterThan(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('user specified model ignores non-string saved model', () => {
|
||||||
|
const model = getUserSpecifiedModelSetting()
|
||||||
|
if (model !== undefined && model !== null) {
|
||||||
|
expect(typeof model).toBe('string')
|
||||||
|
expect(model).not.toBe('[object Object]')
|
||||||
|
}
|
||||||
|
})
|
||||||
@@ -33,6 +33,12 @@ export type ModelShortName = string
|
|||||||
export type ModelName = string
|
export type ModelName = string
|
||||||
export type ModelSetting = ModelName | ModelAlias | null
|
export type ModelSetting = ModelName | ModelAlias | null
|
||||||
|
|
||||||
|
function normalizeModelSetting(value: unknown): ModelName | ModelAlias | undefined {
|
||||||
|
if (typeof value !== 'string') return undefined
|
||||||
|
const trimmed = value.trim()
|
||||||
|
return trimmed.length > 0 ? trimmed : undefined
|
||||||
|
}
|
||||||
|
|
||||||
export function getSmallFastModel(): ModelName {
|
export function getSmallFastModel(): ModelName {
|
||||||
if (process.env.ANTHROPIC_SMALL_FAST_MODEL) return process.env.ANTHROPIC_SMALL_FAST_MODEL
|
if (process.env.ANTHROPIC_SMALL_FAST_MODEL) return process.env.ANTHROPIC_SMALL_FAST_MODEL
|
||||||
// For Gemini provider, use a fast model
|
// For Gemini provider, use a fast model
|
||||||
@@ -82,6 +88,7 @@ export function getUserSpecifiedModelSetting(): ModelSetting | undefined {
|
|||||||
specifiedModel = modelOverride
|
specifiedModel = modelOverride
|
||||||
} else {
|
} else {
|
||||||
const settings = getSettings_DEPRECATED() || {}
|
const settings = getSettings_DEPRECATED() || {}
|
||||||
|
const setting = normalizeModelSetting(settings.model)
|
||||||
// Read the model env var that matches the active provider to prevent
|
// Read the model env var that matches the active provider to prevent
|
||||||
// cross-provider leaks (e.g. ANTHROPIC_MODEL sent to the OpenAI API).
|
// cross-provider leaks (e.g. ANTHROPIC_MODEL sent to the OpenAI API).
|
||||||
const provider = getAPIProvider()
|
const provider = getAPIProvider()
|
||||||
@@ -90,7 +97,7 @@ export function getUserSpecifiedModelSetting(): ModelSetting | undefined {
|
|||||||
(provider === 'mistral' ? process.env.MISTRAL_MODEL : undefined) ||
|
(provider === 'mistral' ? process.env.MISTRAL_MODEL : undefined) ||
|
||||||
(provider === 'openai' || provider === 'gemini' || provider === 'mistral' || provider === 'github' ? process.env.OPENAI_MODEL : undefined) ||
|
(provider === 'openai' || provider === 'gemini' || provider === 'mistral' || provider === 'github' ? process.env.OPENAI_MODEL : undefined) ||
|
||||||
(provider === 'firstParty' ? process.env.ANTHROPIC_MODEL : undefined) ||
|
(provider === 'firstParty' ? process.env.ANTHROPIC_MODEL : undefined) ||
|
||||||
settings.model ||
|
setting ||
|
||||||
undefined
|
undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -264,7 +271,11 @@ export function getDefaultMainLoopModelSetting(): ModelName | ModelAlias {
|
|||||||
// GitHub Copilot provider: check settings.model first, then env, then default
|
// GitHub Copilot provider: check settings.model first, then env, then default
|
||||||
if (getAPIProvider() === 'github') {
|
if (getAPIProvider() === 'github') {
|
||||||
const settings = getSettings_DEPRECATED() || {}
|
const settings = getSettings_DEPRECATED() || {}
|
||||||
return settings.model || process.env.OPENAI_MODEL || 'github:copilot'
|
return (
|
||||||
|
normalizeModelSetting(settings.model) ||
|
||||||
|
normalizeModelSetting(process.env.OPENAI_MODEL) ||
|
||||||
|
'github:copilot'
|
||||||
|
)
|
||||||
}
|
}
|
||||||
// Gemini provider: always use the configured Gemini model
|
// Gemini provider: always use the configured Gemini model
|
||||||
if (getAPIProvider() === 'gemini') {
|
if (getAPIProvider() === 'gemini') {
|
||||||
@@ -595,7 +606,10 @@ export function getPublicModelName(model: ModelName): string {
|
|||||||
export function parseUserSpecifiedModel(
|
export function parseUserSpecifiedModel(
|
||||||
modelInput: ModelName | ModelAlias,
|
modelInput: ModelName | ModelAlias,
|
||||||
): ModelName {
|
): ModelName {
|
||||||
const modelInputTrimmed = modelInput.trim()
|
const modelInputTrimmed = normalizeModelSetting(modelInput)
|
||||||
|
if (!modelInputTrimmed) {
|
||||||
|
return getDefaultSonnetModel()
|
||||||
|
}
|
||||||
const normalizedModel = modelInputTrimmed.toLowerCase()
|
const normalizedModel = modelInputTrimmed.toLowerCase()
|
||||||
|
|
||||||
const has1mTag = has1mContext(normalizedModel)
|
const has1mTag = has1mContext(normalizedModel)
|
||||||
|
|||||||
Reference in New Issue
Block a user