fix: best effort to use resolved authType/model across the repo

This commit is contained in:
mingholy.lmh 2026-01-08 12:11:23 +08:00
parent 5ea841dd02
commit 81de79c899
20 changed files with 414 additions and 260 deletions

View file

@ -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<typeof settings.loadSettings>);
// 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<typeof settings.loadSettings>);
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');
});
});

View file

@ -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;
}

View file

@ -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({