diff --git a/packages/cli/src/ui/auth/useAuth.ts b/packages/cli/src/ui/auth/useAuth.ts index 05f811741..8e5b28499 100644 --- a/packages/cli/src/ui/auth/useAuth.ts +++ b/packages/cli/src/ui/auth/useAuth.ts @@ -4,7 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Config, ModelProvidersConfig } from '@qwen-code/qwen-code-core'; +import type { + Config, + ContentGeneratorConfig, + ModelProvidersConfig, +} from '@qwen-code/qwen-code-core'; import { AuthEvent, AuthType, @@ -214,11 +218,19 @@ export const useAuthCommand = ( if (authType === AuthType.USE_OPENAI) { if (credentials) { - config.updateCredentials({ - apiKey: credentials.apiKey, - baseUrl: credentials.baseUrl, - model: credentials.model, - }); + // Pass settings.model.generationConfig to updateCredentials so it can be merged + // after clearing provider-sourced config. This ensures settings.json generationConfig + // fields (e.g., samplingParams, timeout) are preserved. + const settingsGenerationConfig = settings.merged.model + ?.generationConfig as Partial | undefined; + config.updateCredentials( + { + apiKey: credentials.apiKey, + baseUrl: credentials.baseUrl, + model: credentials.model, + }, + settingsGenerationConfig, + ); await performAuth(authType, credentials); } return; @@ -226,7 +238,13 @@ export const useAuthCommand = ( await performAuth(authType); }, - [config, performAuth, isProviderManagedModel, onAuthError], + [ + config, + performAuth, + isProviderManagedModel, + onAuthError, + settings.merged.model?.generationConfig, + ], ); const openAuthDialog = useCallback(() => { diff --git a/packages/cli/src/ui/components/ModelDialog.tsx b/packages/cli/src/ui/components/ModelDialog.tsx index 42b85438a..06c002070 100644 --- a/packages/cli/src/ui/components/ModelDialog.tsx +++ b/packages/cli/src/ui/components/ModelDialog.tsx @@ -275,7 +275,7 @@ export function ModelDialog({ onClose }: ModelDialogProps): React.JSX.Element { persistModelSelection(settings, effectiveModelId); persistAuthTypeSelection(settings, effectiveAuthType); - const baseUrl = after?.baseUrl ?? '(default)'; + const baseUrl = after?.baseUrl ?? t('(default)'); const maskedKey = maskApiKey(after?.apiKey); uiState?.historyManager.addItem( { @@ -322,7 +322,7 @@ export function ModelDialog({ onClose }: ModelDialogProps): React.JSX.Element { <> { + const original = + await importOriginal(); + return { + ...original, + resolveModelConfig: vi.fn(), + }; +}); + +describe('modelConfigUtils', () => { + describe('getAuthTypeFromEnv', () => { + const originalEnv = process.env; + + beforeEach(() => { + vi.resetModules(); + process.env = { ...originalEnv }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('should return USE_OPENAI when all OpenAI env vars are set', () => { + process.env['OPENAI_API_KEY'] = 'test-key'; + process.env['OPENAI_MODEL'] = 'gpt-4'; + process.env['OPENAI_BASE_URL'] = 'https://api.openai.com'; + + expect(getAuthTypeFromEnv()).toBe(AuthType.USE_OPENAI); + }); + + it('should return undefined when OpenAI env vars are incomplete', () => { + process.env['OPENAI_API_KEY'] = 'test-key'; + process.env['OPENAI_MODEL'] = 'gpt-4'; + // Missing OPENAI_BASE_URL + + expect(getAuthTypeFromEnv()).toBeUndefined(); + }); + + it('should return QWEN_OAUTH when QWEN_OAUTH is set', () => { + process.env['QWEN_OAUTH'] = 'true'; + + expect(getAuthTypeFromEnv()).toBe(AuthType.QWEN_OAUTH); + }); + + it('should return USE_GEMINI when Gemini env vars are set', () => { + process.env['GEMINI_API_KEY'] = 'test-key'; + process.env['GEMINI_MODEL'] = 'gemini-pro'; + + expect(getAuthTypeFromEnv()).toBe(AuthType.USE_GEMINI); + }); + + it('should return undefined when Gemini env vars are incomplete', () => { + process.env['GEMINI_API_KEY'] = 'test-key'; + // Missing GEMINI_MODEL + + expect(getAuthTypeFromEnv()).toBeUndefined(); + }); + + it('should return USE_VERTEX_AI when Google env vars are set', () => { + process.env['GOOGLE_API_KEY'] = 'test-key'; + process.env['GOOGLE_MODEL'] = 'vertex-model'; + + expect(getAuthTypeFromEnv()).toBe(AuthType.USE_VERTEX_AI); + }); + + it('should return undefined when Google env vars are incomplete', () => { + process.env['GOOGLE_API_KEY'] = 'test-key'; + // Missing GOOGLE_MODEL + + expect(getAuthTypeFromEnv()).toBeUndefined(); + }); + + it('should return USE_ANTHROPIC when Anthropic env vars are set', () => { + process.env['ANTHROPIC_API_KEY'] = 'test-key'; + process.env['ANTHROPIC_MODEL'] = 'claude-3'; + process.env['ANTHROPIC_BASE_URL'] = 'https://api.anthropic.com'; + + expect(getAuthTypeFromEnv()).toBe(AuthType.USE_ANTHROPIC); + }); + + it('should return undefined when Anthropic env vars are incomplete', () => { + process.env['ANTHROPIC_API_KEY'] = 'test-key'; + process.env['ANTHROPIC_MODEL'] = 'claude-3'; + // Missing ANTHROPIC_BASE_URL + + expect(getAuthTypeFromEnv()).toBeUndefined(); + }); + + it('should prioritize QWEN_OAUTH over other auth types when explicitly set', () => { + process.env['QWEN_OAUTH'] = 'true'; + process.env['OPENAI_API_KEY'] = 'test-key'; + process.env['OPENAI_MODEL'] = 'gpt-4'; + process.env['OPENAI_BASE_URL'] = 'https://api.openai.com'; + + // QWEN_OAUTH is checked first, so it should be returned even when other auth vars are set + expect(getAuthTypeFromEnv()).toBe(AuthType.QWEN_OAUTH); + }); + + it('should return undefined when no auth env vars are set', () => { + expect(getAuthTypeFromEnv()).toBeUndefined(); + }); + }); + + describe('resolveCliGenerationConfig', () => { + const originalEnv = process.env; + const originalConsoleWarn = console.warn; + + beforeEach(() => { + vi.resetModules(); + process.env = { ...originalEnv }; + console.warn = vi.fn(); + }); + + afterEach(() => { + process.env = originalEnv; + console.warn = originalConsoleWarn; + vi.clearAllMocks(); + }); + + function makeMockSettings(overrides?: Partial): Settings { + return { + model: { name: 'default-model' }, + security: { + auth: { + apiKey: 'settings-api-key', + baseUrl: 'https://settings.example.com', + }, + }, + ...overrides, + } as Settings; + } + + it('should resolve config from argv with highest precedence', () => { + const argv = { + model: 'argv-model', + openaiApiKey: 'argv-key', + openaiBaseUrl: 'https://argv.example.com', + }; + const settings = makeMockSettings(); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'argv-model', + apiKey: 'argv-key', + baseUrl: 'https://argv.example.com', + }, + sources: { + model: { kind: 'cli', detail: '--model' }, + apiKey: { kind: 'cli', detail: '--openaiApiKey' }, + baseUrl: { kind: 'cli', detail: '--openaiBaseUrl' }, + }, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.model).toBe('argv-model'); + expect(result.apiKey).toBe('argv-key'); + expect(result.baseUrl).toBe('https://argv.example.com'); + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + cli: { + model: 'argv-model', + apiKey: 'argv-key', + baseUrl: 'https://argv.example.com', + }, + }), + ); + }); + + it('should resolve config from settings when argv is not provided', () => { + const argv = {}; + const settings = makeMockSettings({ + model: { name: 'settings-model' }, + security: { + auth: { + apiKey: 'settings-key', + baseUrl: 'https://settings.example.com', + }, + }, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'settings-model', + apiKey: 'settings-key', + baseUrl: 'https://settings.example.com', + }, + sources: { + model: { kind: 'settings', detail: 'model.name' }, + apiKey: { kind: 'settings', detail: 'security.auth.apiKey' }, + baseUrl: { kind: 'settings', detail: 'security.auth.baseUrl' }, + }, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.model).toBe('settings-model'); + expect(result.apiKey).toBe('settings-key'); + expect(result.baseUrl).toBe('https://settings.example.com'); + }); + + it('should merge generationConfig from settings', () => { + const argv = {}; + const settings = makeMockSettings({ + model: { + name: 'test-model', + generationConfig: { + samplingParams: { + temperature: 0.7, + max_tokens: 1000, + }, + timeout: 5000, + } as Record, + }, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + samplingParams: { + temperature: 0.7, + max_tokens: 1000, + }, + timeout: 5000, + }, + sources: {}, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.generationConfig.samplingParams?.temperature).toBe(0.7); + expect(result.generationConfig.samplingParams?.max_tokens).toBe(1000); + expect(result.generationConfig.timeout).toBe(5000); + }); + + it('should resolve OpenAI logging from argv', () => { + const argv = { + openaiLogging: true, + openaiLoggingDir: '/custom/log/dir', + }; + const settings = makeMockSettings(); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.generationConfig.enableOpenAILogging).toBe(true); + expect(result.generationConfig.openAILoggingDir).toBe('/custom/log/dir'); + }); + + it('should resolve OpenAI logging from settings when argv is undefined', () => { + const argv = {}; + const settings = makeMockSettings({ + model: { + name: 'test-model', + enableOpenAILogging: true, + openAILoggingDir: '/settings/log/dir', + }, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.generationConfig.enableOpenAILogging).toBe(true); + expect(result.generationConfig.openAILoggingDir).toBe( + '/settings/log/dir', + ); + }); + + it('should default OpenAI logging to false when not provided', () => { + const argv = {}; + const settings = makeMockSettings(); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.generationConfig.enableOpenAILogging).toBe(false); + }); + + it('should find modelProvider from settings when authType and model match', () => { + const argv = { model: 'provider-model' }; + const modelProvider: ProviderModelConfig = { + id: 'provider-model', + name: 'Provider Model', + generationConfig: { + samplingParams: { temperature: 0.8 }, + }, + }; + const settings = makeMockSettings({ + modelProviders: { + [AuthType.USE_OPENAI]: [modelProvider], + }, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'provider-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + modelProvider, + }), + ); + }); + + it('should find modelProvider from settings.model.name when argv.model is not provided', () => { + const argv = {}; + const modelProvider: ProviderModelConfig = { + id: 'settings-model', + name: 'Settings Model', + generationConfig: { + samplingParams: { temperature: 0.9 }, + }, + }; + const settings = makeMockSettings({ + model: { name: 'settings-model' }, + modelProviders: { + [AuthType.USE_OPENAI]: [modelProvider], + }, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'settings-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + modelProvider, + }), + ); + }); + + it('should not find modelProvider when authType is undefined', () => { + const argv = { model: 'test-model' }; + const settings = makeMockSettings({ + modelProviders: { + [AuthType.USE_OPENAI]: [{ id: 'test-model', name: 'Test Model' }], + }, + }); + const selectedAuthType = undefined; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + modelProvider: undefined, + }), + ); + }); + + it('should not find modelProvider when modelProviders is not an array', () => { + const argv = { model: 'test-model' }; + const settings = makeMockSettings({ + modelProviders: { + [AuthType.USE_OPENAI]: null as unknown as ProviderModelConfig[], + }, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + modelProvider: undefined, + }), + ); + }); + + it('should log warnings from resolveModelConfig', () => { + const argv = {}; + const settings = makeMockSettings(); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: ['Warning 1', 'Warning 2'], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(console.warn).toHaveBeenCalledWith('Warning 1'); + expect(console.warn).toHaveBeenCalledWith('Warning 2'); + }); + + it('should use custom env when provided', () => { + const argv = {}; + const settings = makeMockSettings(); + const selectedAuthType = AuthType.USE_OPENAI; + const customEnv = { + OPENAI_API_KEY: 'custom-key', + OPENAI_MODEL: 'custom-model', + }; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'custom-model', + apiKey: 'custom-key', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + env: customEnv, + }); + + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + env: customEnv, + }), + ); + }); + + it('should use process.env when env is not provided', () => { + const argv = {}; + const settings = makeMockSettings(); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + env: process.env, + }), + ); + }); + + it('should return empty strings for missing model, apiKey, and baseUrl', () => { + const argv = {}; + const settings = makeMockSettings(); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: '', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.model).toBe(''); + expect(result.apiKey).toBe(''); + expect(result.baseUrl).toBe(''); + }); + + it('should merge resolved config with logging settings', () => { + const argv = { + openaiLogging: true, + }; + const settings = makeMockSettings({ + model: { + name: 'test-model', + generationConfig: { + timeout: 5000, + } as Record, + }, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: 'test-model', + apiKey: 'test-key', + baseUrl: 'https://test.com', + samplingParams: { temperature: 0.5 }, + }, + sources: {}, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.generationConfig).toEqual({ + model: 'test-model', + apiKey: 'test-key', + baseUrl: 'https://test.com', + samplingParams: { temperature: 0.5 }, + enableOpenAILogging: true, + openAILoggingDir: undefined, + }); + }); + + it('should handle settings without model property', () => { + const argv = {}; + const settings = makeMockSettings({ + model: undefined as unknown as Settings['model'], + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: '', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + const result = resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(result.model).toBe(''); + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + settings: expect.objectContaining({ + model: undefined, + }), + }), + ); + }); + + it('should handle settings without security.auth property', () => { + const argv = {}; + const settings = makeMockSettings({ + security: undefined, + }); + const selectedAuthType = AuthType.USE_OPENAI; + + vi.mocked(resolveModelConfig).mockReturnValue({ + config: { + model: '', + apiKey: '', + baseUrl: '', + }, + sources: {}, + warnings: [], + }); + + resolveCliGenerationConfig({ + argv, + settings, + selectedAuthType, + }); + + expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( + expect.objectContaining({ + settings: expect.objectContaining({ + apiKey: undefined, + baseUrl: undefined, + }), + }), + ); + }); + }); +}); diff --git a/packages/cli/src/utils/modelConfigUtils.ts b/packages/cli/src/utils/modelConfigUtils.ts index 305e50d15..e9cd050c3 100644 --- a/packages/cli/src/utils/modelConfigUtils.ts +++ b/packages/cli/src/utils/modelConfigUtils.ts @@ -44,20 +44,31 @@ export interface ResolvedCliGenerationConfig { } 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']) { + if ( + process.env['OPENAI_API_KEY'] && + process.env['OPENAI_MODEL'] && + process.env['OPENAI_BASE_URL'] + ) { + return AuthType.USE_OPENAI; + } + + if (process.env['GEMINI_API_KEY'] && process.env['GEMINI_MODEL']) { return AuthType.USE_GEMINI; } - if (process.env['GOOGLE_API_KEY']) { + + if (process.env['GOOGLE_API_KEY'] && process.env['GOOGLE_MODEL']) { return AuthType.USE_VERTEX_AI; } - if (process.env['ANTHROPIC_API_KEY']) { + + if ( + process.env['ANTHROPIC_API_KEY'] && + process.env['ANTHROPIC_MODEL'] && + process.env['ANTHROPIC_BASE_URL'] + ) { return AuthType.USE_ANTHROPIC; } diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 581e3a347..54d8f795a 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -708,12 +708,15 @@ export class Config { * Exclusive for `OpenAIKeyPrompt` to update credentials via `/auth` * Delegates to ModelsConfig. */ - updateCredentials(credentials: { - apiKey?: string; - baseUrl?: string; - model?: string; - }): void { - this._modelsConfig.updateCredentials(credentials); + updateCredentials( + credentials: { + apiKey?: string; + baseUrl?: string; + model?: string; + }, + settingsGenerationConfig?: Partial, + ): void { + this._modelsConfig.updateCredentials(credentials, settingsGenerationConfig); } /** diff --git a/packages/core/src/models/modelsConfig.test.ts b/packages/core/src/models/modelsConfig.test.ts index 06896a382..36e40cfe8 100644 --- a/packages/core/src/models/modelsConfig.test.ts +++ b/packages/core/src/models/modelsConfig.test.ts @@ -191,7 +191,7 @@ describe('ModelsConfig', () => { expect(gc.apiKeyEnvKey).toBe('API_KEY_SHARED'); }); - it('should preserve settings generationConfig when model is updated via updateCredentials even if it matches modelProviders', () => { + it('should use provider config when modelId exists in registry even after updateCredentials', () => { const modelProvidersConfig: ModelProvidersConfig = { openai: [ { @@ -213,7 +213,7 @@ describe('ModelsConfig', () => { initialAuthType: AuthType.USE_OPENAI, modelProvidersConfig, generationConfig: { - model: 'model-a', + model: 'custom-model', samplingParams: { temperature: 0.9, max_tokens: 999 }, timeout: 9999, maxRetries: 9, @@ -235,30 +235,30 @@ describe('ModelsConfig', () => { }, }); - // User manually updates the model via updateCredentials (e.g. key prompt flow). - // Even if the model ID matches a modelProviders entry, we must not apply provider defaults - // that would overwrite settings.model.generationConfig. - modelsConfig.updateCredentials({ model: 'model-a' }); + // User manually updates credentials via updateCredentials. + // Note: In practice, handleAuthSelect prevents using a modelId that matches a provider model, + // but if syncAfterAuthRefresh is called with a modelId that exists in registry, + // we should use provider config. + modelsConfig.updateCredentials({ apiKey: 'manual-key' }); - modelsConfig.syncAfterAuthRefresh( - AuthType.USE_OPENAI, - modelsConfig.getModel(), - ); + // syncAfterAuthRefresh with a modelId that exists in registry should use provider config + modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'model-a'); const gc = currentGenerationConfig(modelsConfig); expect(gc.model).toBe('model-a'); - expect(gc.samplingParams?.temperature).toBe(0.9); - expect(gc.samplingParams?.max_tokens).toBe(999); - expect(gc.timeout).toBe(9999); - expect(gc.maxRetries).toBe(9); + // Provider config should be applied + expect(gc.samplingParams?.temperature).toBe(0.1); + expect(gc.samplingParams?.max_tokens).toBe(123); + expect(gc.timeout).toBe(111); + expect(gc.maxRetries).toBe(1); }); - it('should preserve settings generationConfig across multiple auth refreshes after updateCredentials', () => { + it('should preserve settings generationConfig when modelId does not exist in registry', () => { const modelProvidersConfig: ModelProvidersConfig = { openai: [ { - id: 'model-a', - name: 'Model A', + id: 'provider-model', + name: 'Provider Model', baseUrl: 'https://api.example.com/v1', envKey: 'API_KEY_A', generationConfig: { @@ -270,11 +270,12 @@ describe('ModelsConfig', () => { ], }; + // Simulate settings with a custom model (not in registry) const modelsConfig = new ModelsConfig({ initialAuthType: AuthType.USE_OPENAI, modelProvidersConfig, generationConfig: { - model: 'model-a', + model: 'custom-model', samplingParams: { temperature: 0.9, max_tokens: 999 }, timeout: 9999, maxRetries: 9, @@ -296,25 +297,21 @@ describe('ModelsConfig', () => { }, }); + // User manually sets credentials for a custom model (not in registry) modelsConfig.updateCredentials({ apiKey: 'manual-key', baseUrl: 'https://manual.example.com/v1', - model: 'model-a', + model: 'custom-model', }); - // First auth refresh - modelsConfig.syncAfterAuthRefresh( - AuthType.USE_OPENAI, - modelsConfig.getModel(), - ); + // First auth refresh - modelId doesn't exist in registry, so credentials should be preserved + modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'custom-model'); // Second auth refresh should still preserve settings generationConfig - modelsConfig.syncAfterAuthRefresh( - AuthType.USE_OPENAI, - modelsConfig.getModel(), - ); + modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'custom-model'); const gc = currentGenerationConfig(modelsConfig); - expect(gc.model).toBe('model-a'); + expect(gc.model).toBe('custom-model'); + // Settings-sourced generation config should be preserved since modelId doesn't exist in registry expect(gc.samplingParams?.temperature).toBe(0.9); expect(gc.samplingParams?.max_tokens).toBe(999); expect(gc.timeout).toBe(9999); @@ -681,4 +678,120 @@ describe('ModelsConfig', () => { expect(modelsConfig.getModel()).toBe('updated-model'); expect(modelsConfig.getGenerationConfig().model).toBe('updated-model'); }); + + describe('getAllAvailableModels', () => { + it('should return all models across all authTypes', () => { + const modelProvidersConfig: ModelProvidersConfig = { + openai: [ + { + id: 'openai-model-1', + name: 'OpenAI Model 1', + baseUrl: 'https://api.openai.com/v1', + envKey: 'OPENAI_API_KEY', + }, + { + id: 'openai-model-2', + name: 'OpenAI Model 2', + baseUrl: 'https://api.openai.com/v1', + envKey: 'OPENAI_API_KEY', + }, + ], + anthropic: [ + { + id: 'anthropic-model-1', + name: 'Anthropic Model 1', + baseUrl: 'https://api.anthropic.com/v1', + envKey: 'ANTHROPIC_API_KEY', + }, + ], + gemini: [ + { + id: 'gemini-model-1', + name: 'Gemini Model 1', + baseUrl: 'https://generativelanguage.googleapis.com/v1', + envKey: 'GEMINI_API_KEY', + }, + ], + }; + + const modelsConfig = new ModelsConfig({ + modelProvidersConfig, + }); + + const allModels = modelsConfig.getAllAvailableModels(); + + // Should include qwen-oauth models (hard-coded) + const qwenModels = allModels.filter( + (m) => m.authType === AuthType.QWEN_OAUTH, + ); + expect(qwenModels.length).toBeGreaterThan(0); + + // Should include openai models + const openaiModels = allModels.filter( + (m) => m.authType === AuthType.USE_OPENAI, + ); + expect(openaiModels.length).toBe(2); + expect(openaiModels.map((m) => m.id)).toContain('openai-model-1'); + expect(openaiModels.map((m) => m.id)).toContain('openai-model-2'); + + // Should include anthropic models + const anthropicModels = allModels.filter( + (m) => m.authType === AuthType.USE_ANTHROPIC, + ); + expect(anthropicModels.length).toBe(1); + expect(anthropicModels[0].id).toBe('anthropic-model-1'); + + // Should include gemini models + const geminiModels = allModels.filter( + (m) => m.authType === AuthType.USE_GEMINI, + ); + expect(geminiModels.length).toBe(1); + expect(geminiModels[0].id).toBe('gemini-model-1'); + }); + + it('should return empty array when no models are registered', () => { + const modelsConfig = new ModelsConfig(); + + const allModels = modelsConfig.getAllAvailableModels(); + + // Should still include qwen-oauth models (hard-coded) + expect(allModels.length).toBeGreaterThan(0); + const qwenModels = allModels.filter( + (m) => m.authType === AuthType.QWEN_OAUTH, + ); + expect(qwenModels.length).toBeGreaterThan(0); + }); + + it('should return models with correct structure', () => { + const modelProvidersConfig: ModelProvidersConfig = { + openai: [ + { + id: 'test-model', + name: 'Test Model', + description: 'A test model', + baseUrl: 'https://api.example.com/v1', + envKey: 'TEST_API_KEY', + capabilities: { + vision: true, + }, + }, + ], + }; + + const modelsConfig = new ModelsConfig({ + modelProvidersConfig, + }); + + const allModels = modelsConfig.getAllAvailableModels(); + const testModel = allModels.find((m) => m.id === 'test-model'); + + expect(testModel).toBeDefined(); + expect(testModel?.id).toBe('test-model'); + expect(testModel?.label).toBe('Test Model'); + expect(testModel?.description).toBe('A test model'); + expect(testModel?.authType).toBe(AuthType.USE_OPENAI); + expect(testModel?.isVision).toBe(true); + expect(testModel?.capabilities?.vision).toBe(true); + }); + }); }); diff --git a/packages/core/src/models/modelsConfig.ts b/packages/core/src/models/modelsConfig.ts index 36435143f..d33618c21 100644 --- a/packages/core/src/models/modelsConfig.ts +++ b/packages/core/src/models/modelsConfig.ts @@ -203,6 +203,18 @@ export class ModelsConfig { return this.modelRegistry.getModelsForAuthType(authType); } + /** + * Get all available models across all authTypes + */ + getAllAvailableModels(): AvailableModel[] { + const allModels: AvailableModel[] = []; + for (const authType of Object.values(AuthType)) { + const models = this.modelRegistry.getModelsForAuthType(authType); + allModels.push(...models); + } + return allModels; + } + /** * Check if a model exists for the given authType */ @@ -307,6 +319,33 @@ export class ModelsConfig { return this.generationConfigSources; } + /** + * Merge settings generation config, preserving existing values. + * Used when provider-sourced config is cleared but settings should still apply. + */ + mergeSettingsGenerationConfig( + settingsGenerationConfig?: Partial, + ): void { + if (!settingsGenerationConfig) { + return; + } + + for (const field of MODEL_GENERATION_CONFIG_FIELDS) { + if ( + !(field in this._generationConfig) && + field in settingsGenerationConfig + ) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (this._generationConfig as any)[field] = + settingsGenerationConfig[field]; + this.generationConfigSources[field] = { + kind: 'settings', + detail: `model.generationConfig.${field}`, + }; + } + } + } + /** * Update credentials in generation config. * Sets a flag to prevent syncAfterAuthRefresh from overriding these credentials. @@ -314,12 +353,20 @@ export class ModelsConfig { * When credentials are manually set, we clear all provider-sourced configuration * to maintain provider atomicity (either fully applied or not at all). * Other layers (CLI, env, settings, defaults) will participate in resolve. + * + * @param settingsGenerationConfig Optional generation config from settings.json + * to merge after clearing provider-sourced config. + * This ensures settings.model.generationConfig fields + * (e.g., samplingParams, timeout) are preserved. */ - updateCredentials(credentials: { - apiKey?: string; - baseUrl?: string; - model?: string; - }): void { + updateCredentials( + credentials: { + apiKey?: string; + baseUrl?: string; + model?: string; + }, + settingsGenerationConfig?: Partial, + ): void { /** * If any fields are updated here, we treat the resulting config as manually overridden * and avoid applying modelProvider defaults during the next auth refresh. @@ -359,6 +406,14 @@ export class ModelsConfig { this.strictModelProviderSelection = false; // Clear apiKeyEnvKey to prevent validation from requiring environment variable this._generationConfig.apiKeyEnvKey = undefined; + + // After clearing provider-sourced config, merge settings.model.generationConfig + // to ensure fields like samplingParams, timeout, etc. are preserved. + // This follows the resolution strategy where settings.model.generationConfig + // has lower priority than programmatic overrides but should still be applied. + if (settingsGenerationConfig) { + this.mergeSettingsGenerationConfig(settingsGenerationConfig); + } } /** @@ -587,50 +642,88 @@ export class ModelsConfig { } /** - * Called by Config.refreshAuth to sync state after auth refresh. - * - * IMPORTANT: If credentials were manually set via updateCredentials(), - * we should NOT override them with modelProvider defaults. - * This handles the case where user inputs credentials via OpenAIKeyPrompt - * after removing environment variables for a previously selected model. + * Sync state after auth refresh with fallback strategy: + * 1. If modelId can be found in modelRegistry, use the config from modelRegistry. + * 2. Otherwise, if existing credentials exist in resolved generationConfig from other sources + * (not modelProviders), preserve them and update authType/modelId only. + * 3. Otherwise, fall back to default model for the authType. + * 4. If no default is available, leave the generationConfig incomplete and let + * resolveContentGeneratorConfigWithSources throw exceptions as expected. */ syncAfterAuthRefresh(authType: AuthType, modelId?: string): void { - // Check if we have manually set credentials that should be preserved - const preserveManualCredentials = this.hasManualCredentials; + this.strictModelProviderSelection = false; + const previousAuthType = this.currentAuthType; + this.currentAuthType = authType; - // If credentials were manually set, don't apply modelProvider defaults - // Just update the authType and preserve the manually set credentials - if (preserveManualCredentials && authType === AuthType.USE_OPENAI) { - this.strictModelProviderSelection = false; - this.currentAuthType = authType; + // Step 1: If modelId exists in registry, always use config from modelRegistry + // Manual credentials won't have a modelId that matches a provider model (handleAuthSelect prevents it), + // so if modelId exists in registry, we should always use provider config. + // This handles provider switching even within the same authType. + if (modelId && this.modelRegistry.hasModel(authType, modelId)) { + const resolved = this.modelRegistry.getModel(authType, modelId); + if (resolved) { + this.applyResolvedModelDefaults(resolved); + this.strictModelProviderSelection = true; + return; + } + } + + // Step 2: Check if there are existing credentials from other sources (not modelProviders) + const apiKeySource = this.generationConfigSources['apiKey']; + const baseUrlSource = this.generationConfigSources['baseUrl']; + const hasExistingCredentials = + (this._generationConfig.apiKey && + apiKeySource?.kind !== 'modelProviders') || + (this._generationConfig.baseUrl && + baseUrlSource?.kind !== 'modelProviders'); + + // Only preserve credentials if: + // 1. AuthType hasn't changed (credentials are authType-specific), AND + // 2. The modelId doesn't exist in the registry (if it did, we would have used provider config in Step 1), AND + // 3. Either: + // a. We have manual credentials (set via updateCredentials), OR + // b. We have existing credentials + // Note: Even if authType hasn't changed, switching to a different provider model (that exists in registry) + // will use provider config (Step 1), not preserve old credentials. This ensures credentials change when + // switching providers, independent of authType changes. + const isAuthTypeChange = previousAuthType !== authType; + const shouldPreserveCredentials = + !isAuthTypeChange && + (modelId === undefined || + !this.modelRegistry.hasModel(authType, modelId)) && + (this.hasManualCredentials || hasExistingCredentials); + + if (shouldPreserveCredentials) { + // Preserve existing credentials, just update authType and modelId if provided if (modelId) { this._generationConfig.model = modelId; + if (!this.generationConfigSources['model']) { + this.generationConfigSources['model'] = { + kind: 'programmatic', + detail: 'auth refresh (preserved credentials)', + }; + } } return; } - this.strictModelProviderSelection = false; + // Step 3: Fall back to default model for the authType + const defaultModel = + this.modelRegistry.getDefaultModelForAuthType(authType); + if (defaultModel) { + this.applyResolvedModelDefaults(defaultModel); + return; + } - if (modelId && this.modelRegistry.hasModel(authType, modelId)) { - const resolved = this.modelRegistry.getModel(authType, modelId); - if (resolved) { - // Ensure applyResolvedModelDefaults can correctly apply authType-specific - // behavior (e.g., Qwen OAuth placeholder token) by setting currentAuthType - // before applying defaults. - this.currentAuthType = authType; - this.applyResolvedModelDefaults(resolved); - } - } else { - // If the provided modelId doesn't exist in the registry for the new authType, - // use the default model for that authType instead of keeping the old model. - // This handles the case where switching from one authType (e.g., OPENAI with - // env vars) to another (e.g., qwen-oauth) - we should use the default model - // for the new authType, not the old model. - this.currentAuthType = authType; - const defaultModel = - this.modelRegistry.getDefaultModelForAuthType(authType); - if (defaultModel) { - this.applyResolvedModelDefaults(defaultModel); + // Step 4: No default available - leave generationConfig incomplete + // resolveContentGeneratorConfigWithSources will throw exceptions as expected + if (modelId) { + this._generationConfig.model = modelId; + if (!this.generationConfigSources['model']) { + this.generationConfigSources['model'] = { + kind: 'programmatic', + detail: 'auth refresh (no default model)', + }; } } } diff --git a/packages/core/src/qwen/qwenOAuth2.test.ts b/packages/core/src/qwen/qwenOAuth2.test.ts index 6596e12d3..920ca85e3 100644 --- a/packages/core/src/qwen/qwenOAuth2.test.ts +++ b/packages/core/src/qwen/qwenOAuth2.test.ts @@ -751,6 +751,7 @@ describe('getQwenOAuthClient', () => { beforeEach(() => { mockConfig = { isBrowserLaunchSuppressed: vi.fn().mockReturnValue(false), + isInteractive: vi.fn().mockReturnValue(true), } as unknown as Config; originalFetch = global.fetch; @@ -839,9 +840,7 @@ describe('getQwenOAuthClient', () => { requireCachedCredentials: true, }), ), - ).rejects.toThrow( - 'No cached Qwen-OAuth credentials found. Please re-authenticate.', - ); + ).rejects.toThrow('Please use /auth to re-authenticate.'); expect(global.fetch).not.toHaveBeenCalled(); @@ -1007,6 +1006,7 @@ describe('getQwenOAuthClient - Enhanced Error Scenarios', () => { beforeEach(() => { mockConfig = { isBrowserLaunchSuppressed: vi.fn().mockReturnValue(false), + isInteractive: vi.fn().mockReturnValue(true), } as unknown as Config; originalFetch = global.fetch; @@ -1202,6 +1202,7 @@ describe('authWithQwenDeviceFlow - Comprehensive Testing', () => { beforeEach(() => { mockConfig = { isBrowserLaunchSuppressed: vi.fn().mockReturnValue(false), + isInteractive: vi.fn().mockReturnValue(true), } as unknown as Config; originalFetch = global.fetch; @@ -1405,6 +1406,7 @@ describe('Browser Launch and Error Handling', () => { beforeEach(() => { mockConfig = { isBrowserLaunchSuppressed: vi.fn().mockReturnValue(false), + isInteractive: vi.fn().mockReturnValue(true), } as unknown as Config; originalFetch = global.fetch; @@ -2043,6 +2045,7 @@ describe('SharedTokenManager Integration in QwenOAuth2Client', () => { it('should handle TokenManagerError types correctly in getQwenOAuthClient', async () => { const mockConfig = { isBrowserLaunchSuppressed: vi.fn().mockReturnValue(true), + isInteractive: vi.fn().mockReturnValue(true), } as unknown as Config; // Test different TokenManagerError types diff --git a/packages/core/src/qwen/qwenOAuth2.ts b/packages/core/src/qwen/qwenOAuth2.ts index ab89cdfcf..b18c0319d 100644 --- a/packages/core/src/qwen/qwenOAuth2.ts +++ b/packages/core/src/qwen/qwenOAuth2.ts @@ -516,9 +516,7 @@ export async function getQwenOAuthClient( } if (options?.requireCachedCredentials) { - throw new Error( - 'No cached Qwen-OAuth credentials found. Please re-authenticate.', - ); + throw new Error('Please use /auth to re-authenticate.'); } // If we couldn't obtain valid credentials via SharedTokenManager, fall back to @@ -740,11 +738,9 @@ async function authWithQwenDeviceFlow( // Emit device authorization event for UI integration immediately qwenOAuth2Events.emit(QwenOAuth2Event.AuthUri, deviceAuth); - // Always show the fallback message in non-interactive environments to ensure - // users can see the authorization URL even if browser launching is attempted. - // This is critical for headless/remote environments where browser launching - // may silently fail without throwing an error. - showFallbackMessage(deviceAuth.verification_uri_complete); + if (config.isBrowserLaunchSuppressed() || !config.isInteractive()) { + showFallbackMessage(deviceAuth.verification_uri_complete); + } // Try to open browser if not suppressed if (!config.isBrowserLaunchSuppressed()) {