From 81de79c8995fc5c851a88d70cbc22189dd339278 Mon Sep 17 00:00:00 2001 From: "mingholy.lmh" Date: Thu, 8 Jan 2026 12:11:23 +0800 Subject: [PATCH] fix: best effort to use resolved authType/model across the repo --- packages/cli/src/acp-integration/acpAgent.ts | 2 +- packages/cli/src/config/auth.test.ts | 60 ++++++ packages/cli/src/config/auth.ts | 45 ++-- packages/cli/src/config/config.ts | 9 +- packages/cli/src/core/initializer.ts | 5 - packages/cli/src/gemini.test.tsx | 1 - packages/cli/src/gemini.tsx | 18 +- packages/cli/src/test-utils/render.tsx | 24 ++- packages/cli/src/ui/AppContainer.tsx | 16 +- packages/cli/src/ui/auth/AuthDialog.test.tsx | 13 +- packages/cli/src/ui/auth/AuthDialog.tsx | 15 +- packages/cli/src/ui/auth/useAuth.ts | 3 +- packages/cli/src/utils/modelConfigUtils.ts | 21 ++ packages/cli/src/utils/systemInfo.test.ts | 4 + packages/cli/src/utils/systemInfo.ts | 3 +- .../src/validateNonInterActiveAuth.test.ts | 199 ++++++++++-------- .../cli/src/validateNonInterActiveAuth.ts | 47 +---- packages/core/src/config/config.ts | 1 - packages/core/src/models/modelsConfig.test.ts | 123 ++++++++++- packages/core/src/models/modelsConfig.ts | 65 +----- 20 files changed, 414 insertions(+), 260 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index d56d196db..1850ba43f 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -311,7 +311,7 @@ class GeminiAgent { } private async ensureAuthenticated(config: Config): Promise { - const selectedType = this.settings.merged.security?.auth?.selectedType; + const selectedType = config.getAuthType(); if (!selectedType) { throw acp.RequestError.authRequired('No Selected Type'); } diff --git a/packages/cli/src/config/auth.test.ts b/packages/cli/src/config/auth.test.ts index 39652c5c5..ce3173c62 100644 --- a/packages/cli/src/config/auth.test.ts +++ b/packages/cli/src/config/auth.test.ts @@ -167,4 +167,64 @@ describe('validateAuthMethod', () => { expect(validateAuthMethod(AuthType.USE_VERTEX_AI)).toBeNull(); }); + + it('should use config.modelsConfig.getModel() when Config is provided', () => { + // Settings has a different model + vi.mocked(settings.loadSettings).mockReturnValue({ + merged: { + model: { name: 'settings-model' }, + modelProviders: { + openai: [ + { id: 'settings-model', envKey: 'SETTINGS_API_KEY' }, + { id: 'cli-model', envKey: 'CLI_API_KEY' }, + ], + }, + }, + } as unknown as ReturnType); + + // Mock Config object that returns a different model (e.g., from CLI args) + const mockConfig = { + modelsConfig: { + getModel: vi.fn().mockReturnValue('cli-model'), + }, + } as unknown as import('@qwen-code/qwen-code-core').Config; + + // Set the env key for the CLI model, not the settings model + process.env['CLI_API_KEY'] = 'cli-key'; + + // Should use 'cli-model' from config.modelsConfig.getModel(), not 'settings-model' + const result = validateAuthMethod(AuthType.USE_OPENAI, mockConfig); + expect(result).toBeNull(); + expect(mockConfig.modelsConfig.getModel).toHaveBeenCalled(); + }); + + it('should fail validation when Config provides different model without matching env key', () => { + // Clean up any existing env keys first + delete process.env['CLI_API_KEY']; + delete process.env['SETTINGS_API_KEY']; + delete process.env['OPENAI_API_KEY']; + + vi.mocked(settings.loadSettings).mockReturnValue({ + merged: { + model: { name: 'settings-model' }, + modelProviders: { + openai: [ + { id: 'settings-model', envKey: 'SETTINGS_API_KEY' }, + { id: 'cli-model', envKey: 'CLI_API_KEY' }, + ], + }, + }, + } as unknown as ReturnType); + + const mockConfig = { + modelsConfig: { + getModel: vi.fn().mockReturnValue('cli-model'), + }, + } as unknown as import('@qwen-code/qwen-code-core').Config; + + // Don't set CLI_API_KEY - validation should fail + const result = validateAuthMethod(AuthType.USE_OPENAI, mockConfig); + expect(result).not.toBeNull(); + expect(result).toContain('CLI_API_KEY'); + }); }); diff --git a/packages/cli/src/config/auth.ts b/packages/cli/src/config/auth.ts index e3656a277..5fbe07dce 100644 --- a/packages/cli/src/config/auth.ts +++ b/packages/cli/src/config/auth.ts @@ -4,10 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { AuthType } from '@qwen-code/qwen-code-core'; -import type { - ModelProvidersConfig, - ProviderModelConfig, +import { + AuthType, + type Config, + type ModelProvidersConfig, + type ProviderModelConfig, } from '@qwen-code/qwen-code-core'; import { loadEnvironment, loadSettings, type Settings } from './settings.js'; import { t } from '../i18n/index.js'; @@ -45,14 +46,11 @@ function findModelConfig( /** * Check if API key is available for the given auth type and model configuration. * Prioritizes custom envKey from modelProviders over default environment variables. - * - * @returns hasKey - whether an API key is available - * @returns checkedEnvKey - the environment variable name that was checked - * @returns isExplicitEnvKey - true if model has explicit envKey configured (no apiKey fallback allowed) */ function hasApiKeyForAuth( authType: string, settings: Settings, + config?: Config, ): { hasKey: boolean; checkedEnvKey: string | undefined; @@ -61,7 +59,10 @@ function hasApiKeyForAuth( const modelProviders = settings.modelProviders as | ModelProvidersConfig | undefined; - const modelId = settings.model?.name; + + // Use config.modelsConfig.getModel() if available for accurate model ID resolution + // that accounts for CLI args, env vars, and settings. Fall back to settings.model.name. + const modelId = config?.modelsConfig.getModel() ?? settings.model?.name; // Try to find model-specific envKey from modelProviders const modelConfig = findModelConfig(modelProviders, authType, modelId); @@ -104,10 +105,15 @@ function hasApiKeyForAuth( * Generate API key error message based on auth check result. * Returns null if API key is present, otherwise returns the appropriate error message. */ -function getApiKeyError(authMethod: string, settings: Settings): string | null { +function getApiKeyError( + authMethod: string, + settings: Settings, + config?: Config, +): string | null { const { hasKey, checkedEnvKey, isExplicitEnvKey } = hasApiKeyForAuth( authMethod, settings, + config, ); if (hasKey) { return null; @@ -126,7 +132,13 @@ function getApiKeyError(authMethod: string, settings: Settings): string | null { ); } -export function validateAuthMethod(authMethod: string): string | null { +/** + * Validate that the required credentials and configuration exist for the given auth method. + */ +export function validateAuthMethod( + authMethod: string, + config?: Config, +): string | null { const settings = loadSettings(); loadEnvironment(settings.merged); @@ -134,6 +146,7 @@ export function validateAuthMethod(authMethod: string): string | null { const { hasKey, checkedEnvKey, isExplicitEnvKey } = hasApiKeyForAuth( authMethod, settings.merged, + config, ); if (!hasKey) { const envKeyHint = checkedEnvKey @@ -162,7 +175,7 @@ export function validateAuthMethod(authMethod: string): string | null { } if (authMethod === AuthType.USE_ANTHROPIC) { - const apiKeyError = getApiKeyError(authMethod, settings.merged); + const apiKeyError = getApiKeyError(authMethod, settings.merged, config); if (apiKeyError) { return apiKeyError; } @@ -171,7 +184,9 @@ export function validateAuthMethod(authMethod: string): string | null { const modelProviders = settings.merged.modelProviders as | ModelProvidersConfig | undefined; - const modelId = settings.merged.model?.name; + // Use config.modelsConfig.getModel() if available for accurate model ID + const modelId = + config?.modelsConfig.getModel() ?? settings.merged.model?.name; const modelConfig = findModelConfig(modelProviders, authMethod, modelId); if (modelConfig && !modelConfig.baseUrl) { @@ -187,7 +202,7 @@ export function validateAuthMethod(authMethod: string): string | null { } if (authMethod === AuthType.USE_GEMINI) { - const apiKeyError = getApiKeyError(authMethod, settings.merged); + const apiKeyError = getApiKeyError(authMethod, settings.merged, config); if (apiKeyError) { return apiKeyError; } @@ -195,7 +210,7 @@ export function validateAuthMethod(authMethod: string): string | null { } if (authMethod === AuthType.USE_VERTEX_AI) { - const apiKeyError = getApiKeyError(authMethod, settings.merged); + const apiKeyError = getApiKeyError(authMethod, settings.merged, config); if (apiKeyError) { return apiKeyError; } diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 9fffe8fae..a9f47f0b0 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -31,7 +31,10 @@ import { } from '@qwen-code/qwen-code-core'; import { extensionsCommand } from '../commands/extensions.js'; import type { Settings } from './settings.js'; -import { resolveCliGenerationConfig } from '../utils/modelConfigUtils.js'; +import { + resolveCliGenerationConfig, + getAuthTypeFromEnv, +} from '../utils/modelConfigUtils.js'; import yargs, { type Argv } from 'yargs'; import { hideBin } from 'yargs/helpers'; import * as fs from 'node:fs'; @@ -925,7 +928,9 @@ export async function loadCliConfig( const selectedAuthType = (argv.authType as AuthType | undefined) || - settings.security?.auth?.selectedType; + settings.security?.auth?.selectedType || + /* getAuthTypeFromEnv means no authType was explicitly provided, we infer the authType from env vars */ + getAuthTypeFromEnv(); // Unified resolution of generation config with source attribution const resolvedCliConfig = resolveCliGenerationConfig({ diff --git a/packages/cli/src/core/initializer.ts b/packages/cli/src/core/initializer.ts index 062c0b516..56f65b1c5 100644 --- a/packages/cli/src/core/initializer.ts +++ b/packages/cli/src/core/initializer.ts @@ -60,11 +60,6 @@ export async function initializeApp( } const themeError = validateTheme(settings); - // Open auth dialog if: - // 1. No authType was explicitly selected (neither from CLI --auth-type nor settings), OR - // 2. Authentication failed - // wasAuthTypeExplicitlyProvided() returns true if CLI or settings specified authType, - // false if using the default QWEN_OAUTH const shouldOpenAuthDialog = !config.modelsConfig.wasAuthTypeExplicitlyProvided() || !!authError; diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 09dcd013d..c2f971ec4 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -371,7 +371,6 @@ describe('gemini.tsx main function', () => { expect(inputArg).toBe('hello stream'); expect(validateAuthSpy).toHaveBeenCalledWith( - undefined, undefined, configStub, expect.any(Object), diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index da945546d..5dcc9a140 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Config, AuthType } from '@qwen-code/qwen-code-core'; +import type { Config } from '@qwen-code/qwen-code-core'; import { InputFormat, logUserPrompt } from '@qwen-code/qwen-code-core'; import { render } from 'ink'; import dns from 'node:dns'; @@ -252,22 +252,16 @@ export async function main() { argv, ); - if ( - settings.merged.security?.auth?.selectedType && - !settings.merged.security?.auth?.useExternal - ) { + if (!settings.merged.security?.auth?.useExternal) { // Validate authentication here because the sandbox will interfere with the Oauth2 web redirect. try { - const err = validateAuthMethod( - settings.merged.security.auth.selectedType, - ); + const authType = partialConfig.modelsConfig.getCurrentAuthType(); + const err = validateAuthMethod(authType, partialConfig); if (err) { throw new Error(err); } - await partialConfig.refreshAuth( - settings.merged.security.auth.selectedType, - ); + await partialConfig.refreshAuth(authType); } catch (err) { console.error('Error authenticating:', err); process.exit(1); @@ -440,8 +434,6 @@ export async function main() { } const nonInteractiveConfig = await validateNonInteractiveAuth( - (argv.authType as AuthType) || - settings.merged.security?.auth?.selectedType, settings.merged.security?.auth?.useExternal, config, settings, diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 690d765d8..9cb768b5e 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -6,10 +6,12 @@ import { render } from 'ink-testing-library'; import type React from 'react'; +import type { Config } from '@qwen-code/qwen-code-core'; import { LoadedSettings } from '../config/settings.js'; import { KeypressProvider } from '../ui/contexts/KeypressContext.js'; import { SettingsContext } from '../ui/contexts/SettingsContext.js'; import { ShellFocusContext } from '../ui/contexts/ShellFocusContext.js'; +import { ConfigContext } from '../ui/contexts/ConfigContext.js'; const mockSettings = new LoadedSettings( { path: '', settings: {}, originalSettings: {} }, @@ -22,14 +24,24 @@ const mockSettings = new LoadedSettings( export const renderWithProviders = ( component: React.ReactElement, - { shellFocus = true, settings = mockSettings } = {}, + { + shellFocus = true, + settings = mockSettings, + config = undefined, + }: { + shellFocus?: boolean; + settings?: LoadedSettings; + config?: Config; + } = {}, ): ReturnType => render( - - - {component} - - + + + + {component} + + + , ); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 1449a7f4b..f6b41b127 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -373,34 +373,32 @@ export const AppContainer = (props: AppContainerProps) => { if ( settings.merged.security?.auth?.enforcedType && - settings.merged.security?.auth.selectedType && + config.modelsConfig.getCurrentAuthType() && settings.merged.security?.auth.enforcedType !== - settings.merged.security?.auth.selectedType + config.modelsConfig.getCurrentAuthType() ) { onAuthError( t( 'Authentication is enforced to be {{enforcedType}}, but you are currently using {{currentType}}.', { enforcedType: settings.merged.security?.auth.enforcedType, - currentType: settings.merged.security?.auth.selectedType, + currentType: config.modelsConfig.getCurrentAuthType(), }, ), ); - } else if ( - settings.merged.security?.auth?.selectedType && - !settings.merged.security?.auth?.useExternal - ) { + } else if (!settings.merged.security?.auth?.useExternal) { const error = validateAuthMethod( - settings.merged.security.auth.selectedType, + config.modelsConfig.getCurrentAuthType(), + config, ); if (error) { onAuthError(error); } } }, [ - settings.merged.security?.auth?.selectedType, settings.merged.security?.auth?.enforcedType, settings.merged.security?.auth?.useExternal, + config, onAuthError, ]); diff --git a/packages/cli/src/ui/auth/AuthDialog.test.tsx b/packages/cli/src/ui/auth/AuthDialog.test.tsx index 28e13e889..610cb1152 100644 --- a/packages/cli/src/ui/auth/AuthDialog.test.tsx +++ b/packages/cli/src/ui/auth/AuthDialog.test.tsx @@ -7,6 +7,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { AuthDialog } from './AuthDialog.js'; import { LoadedSettings } from '../../config/settings.js'; +import type { Config } from '@qwen-code/qwen-code-core'; import { AuthType } from '@qwen-code/qwen-code-core'; import { renderWithProviders } from '../../test-utils/render.js'; import { UIStateContext } from '../contexts/UIStateContext.js'; @@ -43,17 +44,24 @@ const renderAuthDialog = ( settings: LoadedSettings, uiStateOverrides: Partial = {}, uiActionsOverrides: Partial = {}, + configAuthType: AuthType | undefined = undefined, + configApiKey: string | undefined = undefined, ) => { const uiState = createMockUIState(uiStateOverrides); const uiActions = createMockUIActions(uiActionsOverrides); + const mockConfig = { + getAuthType: vi.fn(() => configAuthType), + getContentGeneratorConfig: vi.fn(() => ({ apiKey: configApiKey })), + } as unknown as Config; + return renderWithProviders( , - { settings }, + { settings, config: mockConfig }, ); }; @@ -421,6 +429,7 @@ describe('AuthDialog', () => { settings, {}, { handleAuthSelect }, + undefined, // config.getAuthType() returns undefined ); await wait(); @@ -475,6 +484,7 @@ describe('AuthDialog', () => { settings, { authError: 'Initial error' }, { handleAuthSelect }, + undefined, // config.getAuthType() returns undefined ); await wait(); @@ -528,6 +538,7 @@ describe('AuthDialog', () => { settings, {}, { handleAuthSelect }, + AuthType.USE_OPENAI, // config.getAuthType() returns USE_OPENAI ); await wait(); diff --git a/packages/cli/src/ui/auth/AuthDialog.tsx b/packages/cli/src/ui/auth/AuthDialog.tsx index 44e2affaa..9ae1ea2a7 100644 --- a/packages/cli/src/ui/auth/AuthDialog.tsx +++ b/packages/cli/src/ui/auth/AuthDialog.tsx @@ -13,7 +13,7 @@ import { useKeypress } from '../hooks/useKeypress.js'; import { RadioButtonSelect } from '../components/shared/RadioButtonSelect.js'; import { useUIState } from '../contexts/UIStateContext.js'; import { useUIActions } from '../contexts/UIActionsContext.js'; -import { useSettings } from '../contexts/SettingsContext.js'; +import { useConfig } from '../contexts/ConfigContext.js'; import { t } from '../../i18n/index.js'; function parseDefaultAuthType( @@ -31,7 +31,7 @@ function parseDefaultAuthType( export function AuthDialog(): React.JSX.Element { const { pendingAuthType, authError } = useUIState(); const { handleAuthSelect: onAuthSelect } = useUIActions(); - const settings = useSettings(); + const config = useConfig(); const [errorMessage, setErrorMessage] = useState(null); const [selectedIndex, setSelectedIndex] = useState(null); @@ -57,9 +57,10 @@ export function AuthDialog(): React.JSX.Element { return item.value === pendingAuthType; } - // Priority 2: settings.merged.security?.auth?.selectedType - if (settings.merged.security?.auth?.selectedType) { - return item.value === settings.merged.security?.auth?.selectedType; + // Priority 2: config.getAuthType() - the source of truth + const currentAuthType = config.getAuthType(); + if (currentAuthType) { + return item.value === currentAuthType; } // Priority 3: QWEN_DEFAULT_AUTH_TYPE env var @@ -75,7 +76,7 @@ export function AuthDialog(): React.JSX.Element { }), ); - const hasApiKey = Boolean(settings.merged.security?.auth?.apiKey); + const hasApiKey = Boolean(config.getContentGeneratorConfig()?.apiKey); const currentSelectedAuthType = selectedIndex !== null ? items[selectedIndex]?.value @@ -99,7 +100,7 @@ export function AuthDialog(): React.JSX.Element { if (errorMessage) { return; } - if (settings.merged.security?.auth?.selectedType === undefined) { + if (config.getAuthType() === undefined) { // Prevent exiting if no auth method is set setErrorMessage( t( diff --git a/packages/cli/src/ui/auth/useAuth.ts b/packages/cli/src/ui/auth/useAuth.ts index bfc80ca70..7237ac33d 100644 --- a/packages/cli/src/ui/auth/useAuth.ts +++ b/packages/cli/src/ui/auth/useAuth.ts @@ -27,8 +27,7 @@ export const useAuthCommand = ( config: Config, addItem: (item: Omit, timestamp: number) => void, ) => { - const unAuthenticated = - settings.merged.security?.auth?.selectedType === undefined; + const unAuthenticated = config.getAuthType() === undefined; const [authState, setAuthState] = useState( unAuthenticated ? AuthState.Updating : AuthState.Unauthenticated, diff --git a/packages/cli/src/utils/modelConfigUtils.ts b/packages/cli/src/utils/modelConfigUtils.ts index d82252d58..b66039d75 100644 --- a/packages/cli/src/utils/modelConfigUtils.ts +++ b/packages/cli/src/utils/modelConfigUtils.ts @@ -42,6 +42,27 @@ export interface ResolvedCliGenerationConfig { sources: ContentGeneratorConfigSources; } +export function getAuthTypeFromEnv(): AuthType | undefined { + if (process.env['OPENAI_API_KEY']) { + return AuthType.USE_OPENAI; + } + if (process.env['QWEN_OAUTH']) { + return AuthType.QWEN_OAUTH; + } + + if (process.env['GEMINI_API_KEY']) { + return AuthType.USE_GEMINI; + } + if (process.env['GOOGLE_API_KEY']) { + return AuthType.USE_VERTEX_AI; + } + if (process.env['ANTHROPIC_API_KEY']) { + return AuthType.USE_ANTHROPIC; + } + + return undefined; +} + /** * Unified resolver for CLI generation config. * diff --git a/packages/cli/src/utils/systemInfo.test.ts b/packages/cli/src/utils/systemInfo.test.ts index 4849f1b1f..8d257605d 100644 --- a/packages/cli/src/utils/systemInfo.test.ts +++ b/packages/cli/src/utils/systemInfo.test.ts @@ -57,6 +57,7 @@ describe('systemInfo', () => { getModel: vi.fn().mockReturnValue('test-model'), getIdeMode: vi.fn().mockReturnValue(true), getSessionId: vi.fn().mockReturnValue('test-session-id'), + getAuthType: vi.fn().mockReturnValue('test-auth'), getContentGeneratorConfig: vi.fn().mockReturnValue({ baseUrl: 'https://api.openai.com', }), @@ -273,6 +274,9 @@ describe('systemInfo', () => { // Update the mock context to use OpenAI auth mockContext.services.settings.merged.security!.auth!.selectedType = AuthType.USE_OPENAI; + vi.mocked(mockContext.services.config!.getAuthType).mockReturnValue( + AuthType.USE_OPENAI, + ); const extendedInfo = await getExtendedSystemInfo(mockContext); diff --git a/packages/cli/src/utils/systemInfo.ts b/packages/cli/src/utils/systemInfo.ts index 5f067b3ae..1af3f41f5 100644 --- a/packages/cli/src/utils/systemInfo.ts +++ b/packages/cli/src/utils/systemInfo.ts @@ -115,8 +115,7 @@ export async function getSystemInfo( const sandboxEnv = getSandboxEnv(); const modelVersion = context.services.config?.getModel() || 'Unknown'; const cliVersion = await getCliVersion(); - const selectedAuthType = - context.services.settings.merged.security?.auth?.selectedType || ''; + const selectedAuthType = context.services.config?.getAuthType() || ''; const ideClient = await getIdeClientName(context); const sessionId = context.services.config?.getSessionId() || 'unknown'; diff --git a/packages/cli/src/validateNonInterActiveAuth.test.ts b/packages/cli/src/validateNonInterActiveAuth.test.ts index 2997847d3..dcaf6b118 100644 --- a/packages/cli/src/validateNonInterActiveAuth.test.ts +++ b/packages/cli/src/validateNonInterActiveAuth.test.ts @@ -14,6 +14,20 @@ import * as JsonOutputAdapterModule from './nonInteractive/io/JsonOutputAdapter. import * as StreamJsonOutputAdapterModule from './nonInteractive/io/StreamJsonOutputAdapter.js'; import * as cleanupModule from './utils/cleanup.js'; +// Helper to create a mock Config with modelsConfig +function createMockConfig(overrides?: Partial): Config { + return { + refreshAuth: vi.fn().mockResolvedValue('refreshed'), + getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT), + getContentGeneratorConfig: vi.fn().mockReturnValue({ authType: undefined }), + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH), + }, + ...overrides, + } as unknown as Config; +} + describe('validateNonInterActiveAuth', () => { let originalEnvGeminiApiKey: string | undefined; let originalEnvVertexAi: string | undefined; @@ -107,17 +121,20 @@ describe('validateNonInterActiveAuth', () => { vi.restoreAllMocks(); }); - it('exits if no auth type is configured or env vars set', async () => { - const nonInteractiveConfig = { + it('exits if validateAuthMethod fails for default auth type', async () => { + // Mock validateAuthMethod to return error (e.g., missing API key) + vi.spyOn(auth, 'validateAuthMethod').mockReturnValue( + 'Missing API key for authentication', + ); + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, - getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH), + }, + }); try { await validateNonInteractiveAuth( - undefined, undefined, nonInteractiveConfig, mockSettings, @@ -127,22 +144,21 @@ describe('validateNonInterActiveAuth', () => { expect((e as Error).message).toContain('process.exit(1) called'); } expect(consoleErrorSpy).toHaveBeenCalledWith( - expect.stringContaining('Please set an Auth method'), + expect.stringContaining('Missing API key'), ); expect(processExitSpy).toHaveBeenCalledWith(1); }); it('uses USE_OPENAI if OPENAI_API_KEY is set', async () => { process.env['OPENAI_API_KEY'] = 'fake-openai-key'; - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, - getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI), + }, + }); await validateNonInteractiveAuth( - undefined, undefined, nonInteractiveConfig, mockSettings, @@ -151,15 +167,14 @@ describe('validateNonInterActiveAuth', () => { }); it('uses configured QWEN_OAUTH if provided', async () => { - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, - getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH), + }, + }); await validateNonInteractiveAuth( - AuthType.QWEN_OAUTH, undefined, nonInteractiveConfig, mockSettings, @@ -170,16 +185,11 @@ describe('validateNonInterActiveAuth', () => { it('exits if validateAuthMethod returns error', async () => { // Mock validateAuthMethod to return error vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!'); - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, - getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + }); try { await validateNonInteractiveAuth( - AuthType.USE_GEMINI, undefined, nonInteractiveConfig, mockSettings, @@ -197,14 +207,13 @@ describe('validateNonInterActiveAuth', () => { const validateAuthMethodSpy = vi .spyOn(auth, 'validateAuthMethod') .mockReturnValue('Auth error!'); - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, - } as unknown as Config; + }); - // Even with an invalid auth type, it should not exit - // because validation is skipped. + // Even with validation errors, it should not exit + // because validation is skipped when useExternalAuth is true. await validateNonInteractiveAuth( - 'invalid-auth-type' as AuthType, true, // useExternalAuth = true nonInteractiveConfig, mockSettings, @@ -213,8 +222,8 @@ describe('validateNonInterActiveAuth', () => { expect(validateAuthMethodSpy).not.toHaveBeenCalled(); expect(consoleErrorSpy).not.toHaveBeenCalled(); expect(processExitSpy).not.toHaveBeenCalled(); - // We still expect refreshAuth to be called with the (invalid) type - expect(refreshAuthMock).toHaveBeenCalledWith('invalid-auth-type'); + // refreshAuth is called with the authType from config.modelsConfig.getCurrentAuthType() + expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.QWEN_OAUTH); }); it('uses enforcedAuthType if provided', async () => { @@ -222,11 +231,14 @@ describe('validateNonInterActiveAuth', () => { mockSettings.merged.security!.auth!.selectedType = AuthType.USE_OPENAI; // Set required env var for USE_OPENAI to ensure enforcedAuthType takes precedence process.env['OPENAI_API_KEY'] = 'fake-key'; - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI), + }, + }); await validateNonInteractiveAuth( - AuthType.USE_OPENAI, undefined, nonInteractiveConfig, mockSettings, @@ -237,16 +249,15 @@ describe('validateNonInterActiveAuth', () => { it('exits if currentAuthType does not match enforcedAuthType', async () => { mockSettings.merged.security!.auth!.enforcedType = AuthType.QWEN_OAUTH; process.env['OPENAI_API_KEY'] = 'fake-key'; - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, - getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI), + }, + }); try { await validateNonInteractiveAuth( - AuthType.USE_OPENAI, undefined, nonInteractiveConfig, mockSettings, @@ -279,18 +290,21 @@ describe('validateNonInterActiveAuth', () => { ); }); - it('emits error result and exits when no auth is configured', async () => { - const nonInteractiveConfig = { + it('emits error result and exits when validateAuthMethod fails', async () => { + vi.spyOn(auth, 'validateAuthMethod').mockReturnValue( + 'Missing API key for authentication', + ); + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH), + }, + }); try { await validateNonInteractiveAuth( - undefined, undefined, nonInteractiveConfig, mockSettings, @@ -302,9 +316,7 @@ describe('validateNonInterActiveAuth', () => { expect(emitResultMock).toHaveBeenCalledWith({ isError: true, - errorMessage: expect.stringContaining( - 'Please set an Auth method in your', - ), + errorMessage: expect.stringContaining('Missing API key'), durationMs: 0, apiDurationMs: 0, numTurns: 0, @@ -319,17 +331,17 @@ describe('validateNonInterActiveAuth', () => { mockSettings.merged.security!.auth!.enforcedType = AuthType.QWEN_OAUTH; process.env['OPENAI_API_KEY'] = 'fake-key'; - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI), + }, + }); try { await validateNonInteractiveAuth( - undefined, undefined, nonInteractiveConfig, mockSettings, @@ -354,21 +366,21 @@ describe('validateNonInterActiveAuth', () => { expect(consoleErrorSpy).not.toHaveBeenCalled(); }); - it('emits error result and exits when validateAuthMethod fails', async () => { + it('emits error result and exits when API key validation fails', async () => { vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!'); process.env['OPENAI_API_KEY'] = 'fake-key'; - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI), + }, + }); try { await validateNonInteractiveAuth( - AuthType.USE_OPENAI, undefined, nonInteractiveConfig, mockSettings, @@ -413,19 +425,22 @@ describe('validateNonInterActiveAuth', () => { ); }); - it('emits error result and exits when no auth is configured', async () => { - const nonInteractiveConfig = { + it('emits error result and exits when validateAuthMethod fails', async () => { + vi.spyOn(auth, 'validateAuthMethod').mockReturnValue( + 'Missing API key for authentication', + ); + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON), getIncludePartialMessages: vi.fn().mockReturnValue(false), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH), + }, + }); try { await validateNonInteractiveAuth( - undefined, undefined, nonInteractiveConfig, mockSettings, @@ -437,9 +452,7 @@ describe('validateNonInterActiveAuth', () => { expect(emitResultMock).toHaveBeenCalledWith({ isError: true, - errorMessage: expect.stringContaining( - 'Please set an Auth method in your', - ), + errorMessage: expect.stringContaining('Missing API key'), durationMs: 0, apiDurationMs: 0, numTurns: 0, @@ -454,18 +467,18 @@ describe('validateNonInterActiveAuth', () => { mockSettings.merged.security!.auth!.enforcedType = AuthType.QWEN_OAUTH; process.env['OPENAI_API_KEY'] = 'fake-key'; - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON), getIncludePartialMessages: vi.fn().mockReturnValue(false), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI), + }, + }); try { await validateNonInteractiveAuth( - undefined, undefined, nonInteractiveConfig, mockSettings, @@ -490,22 +503,22 @@ describe('validateNonInterActiveAuth', () => { expect(consoleErrorSpy).not.toHaveBeenCalled(); }); - it('emits error result and exits when validateAuthMethod fails', async () => { + it('emits error result and exits when API key validation fails', async () => { vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!'); process.env['OPENAI_API_KEY'] = 'fake-key'; - const nonInteractiveConfig = { + const nonInteractiveConfig = createMockConfig({ refreshAuth: refreshAuthMock, getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON), getIncludePartialMessages: vi.fn().mockReturnValue(false), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue({ authType: undefined }), - } as unknown as Config; + modelsConfig: { + getModel: vi.fn().mockReturnValue('default-model'), + getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI), + }, + }); try { await validateNonInteractiveAuth( - AuthType.USE_OPENAI, undefined, nonInteractiveConfig, mockSettings, diff --git a/packages/cli/src/validateNonInterActiveAuth.ts b/packages/cli/src/validateNonInterActiveAuth.ts index be5425a97..2a4df7ca6 100644 --- a/packages/cli/src/validateNonInterActiveAuth.ts +++ b/packages/cli/src/validateNonInterActiveAuth.ts @@ -5,63 +5,30 @@ */ import type { Config } from '@qwen-code/qwen-code-core'; -import { AuthType, OutputFormat } from '@qwen-code/qwen-code-core'; -import { USER_SETTINGS_PATH } from './config/settings.js'; +import { OutputFormat } from '@qwen-code/qwen-code-core'; import { validateAuthMethod } from './config/auth.js'; import { type LoadedSettings } from './config/settings.js'; import { JsonOutputAdapter } from './nonInteractive/io/JsonOutputAdapter.js'; import { StreamJsonOutputAdapter } from './nonInteractive/io/StreamJsonOutputAdapter.js'; import { runExitCleanup } from './utils/cleanup.js'; -function getAuthTypeFromEnv(): AuthType | undefined { - if (process.env['OPENAI_API_KEY']) { - return AuthType.USE_OPENAI; - } - if (process.env['QWEN_OAUTH']) { - return AuthType.QWEN_OAUTH; - } - - if (process.env['GEMINI_API_KEY']) { - return AuthType.USE_GEMINI; - } - if (process.env['GOOGLE_API_KEY']) { - return AuthType.USE_VERTEX_AI; - } - if (process.env['ANTHROPIC_API_KEY']) { - return AuthType.USE_ANTHROPIC; - } - - return undefined; -} - export async function validateNonInteractiveAuth( - configuredAuthType: AuthType | undefined, useExternalAuth: boolean | undefined, nonInteractiveConfig: Config, settings: LoadedSettings, ): Promise { try { + // Get the actual authType from config which has already resolved CLI args, env vars, and settings + const authType = nonInteractiveConfig.modelsConfig.getCurrentAuthType(); + const enforcedType = settings.merged.security?.auth?.enforcedType; - if (enforcedType) { - const currentAuthType = getAuthTypeFromEnv(); - if (currentAuthType !== enforcedType) { - const message = `The configured auth type is ${enforcedType}, but the current auth type is ${currentAuthType}. Please re-authenticate with the correct type.`; - throw new Error(message); - } - } - - const effectiveAuthType = - enforcedType || configuredAuthType || getAuthTypeFromEnv(); - - if (!effectiveAuthType) { - const message = `Please set an Auth method in your ${USER_SETTINGS_PATH} or specify one of the following environment variables before running: QWEN_OAUTH, OPENAI_API_KEY`; + if (enforcedType && enforcedType !== authType) { + const message = `The configured auth type is ${enforcedType}, but the current auth type is ${authType}. Please re-authenticate with the correct type.`; throw new Error(message); } - const authType: AuthType = effectiveAuthType as AuthType; - if (!useExternalAuth) { - const err = validateAuthMethod(String(authType)); + const err = validateAuthMethod(authType, nonInteractiveConfig); if (err != null) { throw new Error(err); } diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index eae1dd44b..e96c9c0ba 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -632,7 +632,6 @@ export class Config { // - generationConfig.authType may have a default value from resolvers this._modelsConfig = new ModelsConfig({ initialAuthType: params.authType ?? params.generationConfig?.authType, - initialModelId: params.model, modelProvidersConfig: this.modelProvidersConfig, generationConfig: { model: params.model, diff --git a/packages/core/src/models/modelsConfig.test.ts b/packages/core/src/models/modelsConfig.test.ts index 8f8441e00..ae2808d75 100644 --- a/packages/core/src/models/modelsConfig.test.ts +++ b/packages/core/src/models/modelsConfig.test.ts @@ -174,8 +174,10 @@ describe('ModelsConfig', () => { const modelsConfig = new ModelsConfig({ initialAuthType: AuthType.USE_OPENAI, - initialModelId: 'model-a', modelProvidersConfig, + generationConfig: { + model: 'model-a', + }, }); // Simulate key prompt flow / explicit key provided via CLI/settings. @@ -209,7 +211,6 @@ describe('ModelsConfig', () => { // Simulate settings.model.generationConfig being resolved into ModelsConfig.generationConfig const modelsConfig = new ModelsConfig({ initialAuthType: AuthType.USE_OPENAI, - initialModelId: 'model-a', modelProvidersConfig, generationConfig: { model: 'model-a', @@ -271,7 +272,6 @@ describe('ModelsConfig', () => { const modelsConfig = new ModelsConfig({ initialAuthType: AuthType.USE_OPENAI, - initialModelId: 'model-a', modelProvidersConfig, generationConfig: { model: 'model-a', @@ -463,4 +463,121 @@ describe('ModelsConfig', () => { expect(gc.apiKey).toBe('QWEN_OAUTH_DYNAMIC_TOKEN'); expect(gc.apiKeyEnvKey).toBeUndefined(); }); + + it('should maintain consistency between currentModelId and _generationConfig.model after initialization', () => { + const modelProvidersConfig: ModelProvidersConfig = { + openai: [ + { + id: 'test-model', + name: 'Test Model', + baseUrl: 'https://api.example.com/v1', + envKey: 'TEST_API_KEY', + }, + ], + }; + + // Test case 1: generationConfig.model provided with other config + const config1 = new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + modelProvidersConfig, + generationConfig: { + model: 'test-model', + samplingParams: { temperature: 0.5 }, + }, + }); + expect(config1.getModel()).toBe('test-model'); + expect(config1.getGenerationConfig().model).toBe('test-model'); + + // Test case 2: generationConfig.model provided + const config2 = new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + modelProvidersConfig, + generationConfig: { + model: 'test-model', + }, + }); + expect(config2.getModel()).toBe('test-model'); + expect(config2.getGenerationConfig().model).toBe('test-model'); + + // Test case 3: no model provided (empty string fallback) + const config3 = new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + modelProvidersConfig, + generationConfig: {}, + }); + expect(config3.getModel()).toBe('coder-model'); // Falls back to DEFAULT_QWEN_MODEL + expect(config3.getGenerationConfig().model).toBeUndefined(); + }); + + it('should maintain consistency between currentModelId and _generationConfig.model during syncAfterAuthRefresh', () => { + const modelProvidersConfig: ModelProvidersConfig = { + openai: [ + { + id: 'model-a', + name: 'Model A', + baseUrl: 'https://api.example.com/v1', + envKey: 'API_KEY_A', + }, + ], + }; + + const modelsConfig = new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + modelProvidersConfig, + generationConfig: { + model: 'model-a', + }, + }); + + // Manually set credentials to trigger preserveManualCredentials path + modelsConfig.updateCredentials({ apiKey: 'manual-key' }); + + // syncAfterAuthRefresh with a different modelId + modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'model-a'); + + // Both should be consistent + expect(modelsConfig.getModel()).toBe('model-a'); + expect(modelsConfig.getGenerationConfig().model).toBe('model-a'); + }); + + it('should maintain consistency between currentModelId and _generationConfig.model during setModel', async () => { + const modelProvidersConfig: ModelProvidersConfig = { + openai: [ + { + id: 'model-a', + name: 'Model A', + baseUrl: 'https://api.example.com/v1', + envKey: 'API_KEY_A', + }, + ], + }; + + const modelsConfig = new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + modelProvidersConfig, + }); + + // setModel with a raw model ID + await modelsConfig.setModel('custom-model'); + + // Both should be consistent + expect(modelsConfig.getModel()).toBe('custom-model'); + expect(modelsConfig.getGenerationConfig().model).toBe('custom-model'); + }); + + it('should maintain consistency between currentModelId and _generationConfig.model during updateCredentials', () => { + const modelsConfig = new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + }); + + // updateCredentials with model + modelsConfig.updateCredentials({ + apiKey: 'test-key', + model: 'updated-model', + }); + + // Both should be consistent + expect(modelsConfig.getModel()).toBe('updated-model'); + expect(modelsConfig.getGenerationConfig().model).toBe('updated-model'); + }); }); diff --git a/packages/core/src/models/modelsConfig.ts b/packages/core/src/models/modelsConfig.ts index 5e066b96f..e6908aeea 100644 --- a/packages/core/src/models/modelsConfig.ts +++ b/packages/core/src/models/modelsConfig.ts @@ -45,8 +45,6 @@ export type OnModelChangeCallback = ( export interface ModelsConfigOptions { /** Initial authType from settings */ initialAuthType?: AuthType; - /** Initial model ID from settings */ - initialModelId?: string; /** Model providers configuration */ modelProvidersConfig?: ModelProvidersConfig; /** Generation config from CLI/settings */ @@ -73,7 +71,6 @@ export class ModelsConfig { // Current selection state private currentAuthType: AuthType; - private currentModelId: string; // Generation config state private _generationConfig: Partial; @@ -119,7 +116,6 @@ export class ModelsConfig { private snapshotState(): { currentAuthType: AuthType; - currentModelId: string; generationConfig: Partial; generationConfigSources: ContentGeneratorConfigSources; strictModelProviderSelection: boolean; @@ -128,7 +124,6 @@ export class ModelsConfig { } { return { currentAuthType: this.currentAuthType, - currentModelId: this.currentModelId, generationConfig: ModelsConfig.deepClone(this._generationConfig), generationConfigSources: ModelsConfig.deepClone( this.generationConfigSources, @@ -143,7 +138,6 @@ export class ModelsConfig { snapshot: ReturnType, ): void { this.currentAuthType = snapshot.currentAuthType; - this.currentModelId = snapshot.currentModelId; this._generationConfig = snapshot.generationConfig; this.generationConfigSources = snapshot.generationConfigSources; this.strictModelProviderSelection = snapshot.strictModelProviderSelection; @@ -157,8 +151,9 @@ export class ModelsConfig { this.onModelChange = options.onModelChange; // Initialize generation config + // Note: generationConfig.model should already be fully resolved by ModelConfigResolver + // before ModelsConfig is instantiated, so we use it as the single source of truth this._generationConfig = { - model: options.initialModelId, ...(options.generationConfig || {}), }; this.generationConfigSources = options.generationConfigSources || {}; @@ -168,54 +163,13 @@ export class ModelsConfig { // Initialize selection state this.currentAuthType = options.initialAuthType || AuthType.QWEN_OAUTH; - this.currentModelId = options.initialModelId || ''; - - // Validate and initialize default selection - this.initializeDefaultSelection(); - } - - /** - * Initialize default selection based on settings/environment. - * - * Note: The generationConfig passed to ModelsConfig should already be fully - * resolved by ModelConfigResolver, which handles CLI args, env vars, and settings. - * This method primarily validates and sets up internal state. - */ - private initializeDefaultSelection(): void { - // If generationConfig already has a model (resolved by ModelConfigResolver), - // use that as the current selection - if (this._generationConfig.model) { - this.currentModelId = this._generationConfig.model; - return; - } - - // Check if persisted model selection is valid - if ( - this.currentModelId && - this.modelRegistry.hasModel(this.currentAuthType, this.currentModelId) - ) { - return; - } - - // Use registry default - const defaultModel = this.modelRegistry.getDefaultModelForAuthType( - this.currentAuthType, - ); - if (defaultModel) { - this.currentModelId = defaultModel.id; - if (!this._generationConfig.model) { - this._generationConfig.model = defaultModel.id; - } - } } /** * Get current model ID */ getModel(): string { - return ( - this._generationConfig.model || this.currentModelId || DEFAULT_QWEN_MODEL - ); + return this._generationConfig.model || DEFAULT_QWEN_MODEL; } /** @@ -269,7 +223,6 @@ export class ModelsConfig { ) { this.strictModelProviderSelection = false; this._generationConfig.model = newModel; - this.currentModelId = newModel; this.generationConfigSources['model'] = { kind: 'programmatic', detail: metadata?.reason || 'setModel', @@ -286,7 +239,6 @@ export class ModelsConfig { // Raw model override: update generation config in-place this.strictModelProviderSelection = false; this._generationConfig.model = newModel; - this.currentModelId = newModel; this.generationConfigSources['model'] = { kind: 'programmatic', detail: metadata?.reason || 'setModel', @@ -322,12 +274,9 @@ export class ModelsConfig { // Apply model defaults this.applyResolvedModelDefaults(model); - // Update selection state - this.currentModelId = modelId; - const requiresRefresh = isAuthTypeChange ? true - : this.checkRequiresRefresh(snapshot.currentModelId); + : this.checkRequiresRefresh(snapshot.generationConfig.model || ''); if (this.onModelChange) { await this.onModelChange(authType, requiresRefresh); @@ -395,7 +344,6 @@ export class ModelsConfig { } if (credentials.model) { this._generationConfig.model = credentials.model; - this.currentModelId = credentials.model; this.generationConfigSources['model'] = { kind: 'programmatic', detail: 'updateCredentials', @@ -603,7 +551,7 @@ export class ModelsConfig { ); const currentModel = this.modelRegistry.getModel( this.currentAuthType, - this.currentModelId, + this._generationConfig.model || '', ); // If either model is not in registry, require refresh to be safe @@ -644,7 +592,7 @@ export class ModelsConfig { this.strictModelProviderSelection = false; this.currentAuthType = authType; if (modelId) { - this.currentModelId = modelId; + this._generationConfig.model = modelId; } return; } @@ -656,7 +604,6 @@ export class ModelsConfig { if (resolved) { this.applyResolvedModelDefaults(resolved); this.currentAuthType = authType; - this.currentModelId = modelId; } } else { this.currentAuthType = authType;