From 6b2121da12189fa7ce1f33394d18abd24cf8a01b Mon Sep 17 00:00:00 2001 From: Jeevan Mohan Pawar <35501212+jeevan6996@users.noreply.github.com> Date: Wed, 15 Apr 2026 12:47:02 +0100 Subject: [PATCH] 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 --- src/utils/model/model.github.test.ts | 57 ++++++++++++++++++++++++++++ src/utils/model/model.ts | 20 ++++++++-- 2 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 src/utils/model/model.github.test.ts diff --git a/src/utils/model/model.github.test.ts b/src/utils/model/model.github.test.ts new file mode 100644 index 00000000..a9280317 --- /dev/null +++ b/src/utils/model/model.github.test.ts @@ -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]') + } +}) diff --git a/src/utils/model/model.ts b/src/utils/model/model.ts index 236896ee..ee7516a2 100644 --- a/src/utils/model/model.ts +++ b/src/utils/model/model.ts @@ -33,6 +33,12 @@ export type ModelShortName = string export type ModelName = string 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 { if (process.env.ANTHROPIC_SMALL_FAST_MODEL) return process.env.ANTHROPIC_SMALL_FAST_MODEL // For Gemini provider, use a fast model @@ -82,6 +88,7 @@ export function getUserSpecifiedModelSetting(): ModelSetting | undefined { specifiedModel = modelOverride } else { const settings = getSettings_DEPRECATED() || {} + const setting = normalizeModelSetting(settings.model) // Read the model env var that matches the active provider to prevent // cross-provider leaks (e.g. ANTHROPIC_MODEL sent to the OpenAI API). const provider = getAPIProvider() @@ -90,7 +97,7 @@ export function getUserSpecifiedModelSetting(): ModelSetting | undefined { (provider === 'mistral' ? process.env.MISTRAL_MODEL : undefined) || (provider === 'openai' || provider === 'gemini' || provider === 'mistral' || provider === 'github' ? process.env.OPENAI_MODEL : undefined) || (provider === 'firstParty' ? process.env.ANTHROPIC_MODEL : undefined) || - settings.model || + setting || undefined } @@ -264,7 +271,11 @@ export function getDefaultMainLoopModelSetting(): ModelName | ModelAlias { // GitHub Copilot provider: check settings.model first, then env, then default if (getAPIProvider() === 'github') { 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 if (getAPIProvider() === 'gemini') { @@ -595,7 +606,10 @@ export function getPublicModelName(model: ModelName): string { export function parseUserSpecifiedModel( modelInput: ModelName | ModelAlias, ): ModelName { - const modelInputTrimmed = modelInput.trim() + const modelInputTrimmed = normalizeModelSetting(modelInput) + if (!modelInputTrimmed) { + return getDefaultSonnetModel() + } const normalizedModel = modelInputTrimmed.toLowerCase() const has1mTag = has1mContext(normalizedModel)