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/utils/modelConfigUtils.ts b/packages/cli/src/utils/modelConfigUtils.ts index 305e50d15..d13b5fad8 100644 --- a/packages/cli/src/utils/modelConfigUtils.ts +++ b/packages/cli/src/utils/modelConfigUtils.ts @@ -44,20 +44,24 @@ export interface ResolvedCliGenerationConfig { } export function getAuthTypeFromEnv(): AuthType | undefined { - if (process.env['OPENAI_API_KEY']) { + if (process.env['OPENAI_API_KEY'] && process.env['OPENAI_MODEL']) { return AuthType.USE_OPENAI; } if (process.env['QWEN_OAUTH']) { return AuthType.QWEN_OAUTH; } - if (process.env['GEMINI_API_KEY']) { + 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 2b2f71150..1816da30f 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -706,12 +706,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..9aa77e66f 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); diff --git a/packages/core/src/models/modelsConfig.ts b/packages/core/src/models/modelsConfig.ts index 36435143f..7c94bf122 100644 --- a/packages/core/src/models/modelsConfig.ts +++ b/packages/core/src/models/modelsConfig.ts @@ -307,6 +307,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 +341,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 +394,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 +630,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)', + }; } } }