From aa9cdf2a3c37982ab5580e5f0da94b9e0c5d720b Mon Sep 17 00:00:00 2001 From: "mingholy.lmh" Date: Tue, 30 Dec 2025 19:49:15 +0800 Subject: [PATCH] review: stage1 --- packages/cli/src/config/auth.test.ts | 2 +- packages/cli/src/config/auth.ts | 6 +- packages/cli/src/config/config.ts | 18 +++ packages/cli/src/config/settingsSchema.ts | 14 ++ packages/cli/src/utils/modelProviderUtils.ts | 142 ++++++++++++++++++ .../core/src/config/flashFallback.test.ts | 5 +- .../core/src/core/coreToolScheduler.test.ts | 26 ++-- packages/core/src/core/geminiChat.test.ts | 2 +- .../core/nonInteractiveToolExecutor.test.ts | 2 +- .../core/openaiContentGenerator/converter.ts | 8 + 10 files changed, 206 insertions(+), 19 deletions(-) create mode 100644 packages/cli/src/utils/modelProviderUtils.ts diff --git a/packages/cli/src/config/auth.test.ts b/packages/cli/src/config/auth.test.ts index e28184ac8..6f6b584ef 100644 --- a/packages/cli/src/config/auth.test.ts +++ b/packages/cli/src/config/auth.test.ts @@ -32,7 +32,7 @@ describe('validateAuthMethod', () => { it('should return an error message for USE_OPENAI if OPENAI_API_KEY is not set', () => { delete process.env['OPENAI_API_KEY']; expect(validateAuthMethod(AuthType.USE_OPENAI)).toBe( - 'OPENAI_API_KEY environment variable not found. You can enter it interactively or add it to your .env file.', + "Missing API key for OpenAI-compatible auth. Set settings.security.auth.apiKey, or set the 'OPENAI_API_KEY' environment variable. If you configured a model in settings.modelProviders with an envKey, set that env var as well.", ); }); diff --git a/packages/cli/src/config/auth.ts b/packages/cli/src/config/auth.ts index 9f5d50a07..42fbf280f 100644 --- a/packages/cli/src/config/auth.ts +++ b/packages/cli/src/config/auth.ts @@ -15,7 +15,11 @@ export function validateAuthMethod(authMethod: string): string | null { const hasApiKey = process.env['OPENAI_API_KEY'] || settings.merged.security?.auth?.apiKey; if (!hasApiKey) { - return 'OPENAI_API_KEY environment variable not found. You can enter it interactively or add it to your .env file.'; + return ( + 'Missing API key for OpenAI-compatible auth. ' + + "Set settings.security.auth.apiKey, or set the 'OPENAI_API_KEY' environment variable. " + + 'If you configured a model in settings.modelProviders with an envKey, set that env var as well.' + ); } return null; } diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 6dc51ecb4..bc5da7bfc 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -31,6 +31,10 @@ import { } from '@qwen-code/qwen-code-core'; import { extensionsCommand } from '../commands/extensions.js'; import type { Settings } from './settings.js'; +import { + buildGenerationConfigSources, + getModelProvidersConfigFromSettings, +} from '../utils/modelProviderUtils.js'; import yargs, { type Argv } from 'yargs'; import { hideBin } from 'yargs/helpers'; import * as fs from 'node:fs'; @@ -979,6 +983,18 @@ export async function loadCliConfig( } } + const modelProvidersConfig = getModelProvidersConfigFromSettings(settings); + const generationConfigSources = buildGenerationConfigSources({ + argv: { + model: argv.model, + openaiApiKey: argv.openaiApiKey, + openaiBaseUrl: argv.openaiBaseUrl, + }, + settings, + selectedAuthType, + env: process.env as Record, + }); + return new Config({ sessionId, sessionData, @@ -1036,6 +1052,8 @@ export async function loadCliConfig( inputFormat, outputFormat, includePartialMessages, + modelProvidersConfig, + generationConfigSources, generationConfig: { ...(settings.model?.generationConfig || {}), model: resolvedModel, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 5159613b6..4562546ff 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -10,6 +10,7 @@ import type { TelemetrySettings, AuthType, ChatCompressionSettings, + ModelProvidersConfig, } from '@qwen-code/qwen-code-core'; import { ApprovalMode, @@ -102,6 +103,19 @@ const SETTINGS_SCHEMA = { mergeStrategy: MergeStrategy.SHALLOW_MERGE, }, + // Model providers configuration grouped by authType + modelProviders: { + type: 'object', + label: 'Model Providers', + category: 'Model', + requiresRestart: false, + default: {} as ModelProvidersConfig, + description: + 'Model providers configuration grouped by authType. Each authType contains an array of model configurations.', + showInDialog: false, + mergeStrategy: MergeStrategy.SHALLOW_MERGE, + }, + general: { type: 'object', label: 'General', diff --git a/packages/cli/src/utils/modelProviderUtils.ts b/packages/cli/src/utils/modelProviderUtils.ts new file mode 100644 index 000000000..473499771 --- /dev/null +++ b/packages/cli/src/utils/modelProviderUtils.ts @@ -0,0 +1,142 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + AuthType, + type ContentGeneratorConfig, + type ContentGeneratorConfigSource, + type ContentGeneratorConfigSources, + type ModelProvidersConfig, + type ProviderModelConfig as ModelConfig, +} from '@qwen-code/qwen-code-core'; +import type { Settings } from '../config/settings.js'; + +export interface GenerationConfigSourceInputs { + argv: { + model?: string | undefined; + openaiApiKey?: string | undefined; + openaiBaseUrl?: string | undefined; + }; + settings: Settings; + selectedAuthType: AuthType | undefined; + /** + * Injectable env for testability. Defaults to process.env at callsites. + */ + env?: Record; +} + +/** + * Get models configuration from settings, grouped by authType. + * Returns the models config from the merged settings without mutating files. + */ +export function getModelProvidersConfigFromSettings( + settings: Settings, +): ModelProvidersConfig { + return (settings.modelProviders as ModelProvidersConfig) || {}; +} + +/** + * Get models for a specific authType from settings. + */ +export function getModelsForAuthType( + settings: Settings, + authType: AuthType, +): ModelConfig[] { + const modelProvidersConfig = getModelProvidersConfigFromSettings(settings); + return modelProvidersConfig[authType] || []; +} + +/** + * Best-effort attribution for the seed generationConfig fields. + * + * NOTE: + * - This does not attempt to distinguish user vs workspace settings; it reflects merged settings. + * - This should stay consistent with the actual precedence used to compute the corresponding values. + */ +export function buildGenerationConfigSources( + inputs: GenerationConfigSourceInputs, +): ContentGeneratorConfigSources { + const { argv, settings, selectedAuthType } = inputs; + const env = inputs.env ?? (process.env as Record); + + const sources: ContentGeneratorConfigSources = {}; + + const setSource = (path: string, source: ContentGeneratorConfigSource) => { + sources[path] = source; + }; + + // Model/apiKey/baseUrl attribution mirrors current CLI precedence: + // - model: argv.model > (OPENAI_MODEL|QWEN_MODEL|settings.model.name) only for OpenAI auth + // - apiKey/baseUrl: only meaningful for OpenAI auth in current CLI wiring + if (selectedAuthType === AuthType.USE_OPENAI) { + if (argv.model) { + setSource('model', { kind: 'cli', detail: '--model' }); + } else if (env['OPENAI_MODEL']) { + setSource('model', { kind: 'env', envKey: 'OPENAI_MODEL' }); + } else if (env['QWEN_MODEL']) { + setSource('model', { kind: 'env', envKey: 'QWEN_MODEL' }); + } else if (settings.model?.name) { + setSource('model', { kind: 'settings', settingsPath: 'model.name' }); + } + + if (argv.openaiApiKey) { + setSource('apiKey', { kind: 'cli', detail: '--openaiApiKey' }); + } else if (env['OPENAI_API_KEY']) { + setSource('apiKey', { kind: 'env', envKey: 'OPENAI_API_KEY' }); + } else if (settings.security?.auth?.apiKey) { + setSource('apiKey', { + kind: 'settings', + settingsPath: 'security.auth.apiKey', + }); + } + + if (argv.openaiBaseUrl) { + setSource('baseUrl', { kind: 'cli', detail: '--openaiBaseUrl' }); + } else if (env['OPENAI_BASE_URL']) { + setSource('baseUrl', { kind: 'env', envKey: 'OPENAI_BASE_URL' }); + } else if (settings.security?.auth?.baseUrl) { + setSource('baseUrl', { + kind: 'settings', + settingsPath: 'security.auth.baseUrl', + }); + } + } else if (argv.model) { + // For non-openai auth types, the CLI only wires through an explicit raw model override. + setSource('model', { kind: 'cli', detail: '--model' }); + } + + const mergedGenerationConfig = settings.model?.generationConfig as + | Partial + | undefined; + if (mergedGenerationConfig) { + setSource('generationConfig', { + kind: 'settings', + settingsPath: 'model.generationConfig', + }); + // We also map the known top-level fields used by core. + if (mergedGenerationConfig.samplingParams) { + setSource('samplingParams', { + kind: 'settings', + settingsPath: 'model.generationConfig.samplingParams', + }); + } + for (const k of [ + 'timeout', + 'maxRetries', + 'disableCacheControl', + 'schemaCompliance', + ] as const) { + if (mergedGenerationConfig[k] !== undefined) { + setSource(k, { + kind: 'settings', + settingsPath: `model.generationConfig.${k}`, + }); + } + } + } + + return sources; +} diff --git a/packages/core/src/config/flashFallback.test.ts b/packages/core/src/config/flashFallback.test.ts index 2675ea840..1fff42392 100644 --- a/packages/core/src/config/flashFallback.test.ts +++ b/packages/core/src/config/flashFallback.test.ts @@ -11,7 +11,8 @@ import fs from 'node:fs'; vi.mock('node:fs'); -describe('Flash Model Fallback Configuration', () => { +// Skip this test because we do not have fall back mechanism. +describe.skip('Flash Model Fallback Configuration', () => { let config: Config; beforeEach(() => { @@ -31,7 +32,7 @@ describe('Flash Model Fallback Configuration', () => { config as unknown as { contentGeneratorConfig: unknown } ).contentGeneratorConfig = { model: DEFAULT_GEMINI_MODEL, - authType: 'gemini-api-key', + authType: 'gemini', }; }); diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index bd970b9da..1cf3c565c 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -240,7 +240,7 @@ describe('CoreToolScheduler', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -318,7 +318,7 @@ describe('CoreToolScheduler', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -497,7 +497,7 @@ describe('CoreToolScheduler', () => { getExcludeTools: () => ['write_file', 'edit', 'run_shell_command'], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -584,7 +584,7 @@ describe('CoreToolScheduler', () => { getExcludeTools: () => ['write_file', 'edit'], // Different excluded tools getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -674,7 +674,7 @@ describe('CoreToolScheduler with payload', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -1001,7 +1001,7 @@ describe('CoreToolScheduler edit cancellation', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -1108,7 +1108,7 @@ describe('CoreToolScheduler YOLO mode', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -1258,7 +1258,7 @@ describe('CoreToolScheduler cancellation during executing with live output', () getApprovalMode: () => ApprovalMode.DEFAULT, getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getToolRegistry: () => mockToolRegistry, getShellExecutionConfig: () => ({ @@ -1350,7 +1350,7 @@ describe('CoreToolScheduler request queueing', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -1482,7 +1482,7 @@ describe('CoreToolScheduler request queueing', () => { getToolRegistry: () => toolRegistry, getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 80, @@ -1586,7 +1586,7 @@ describe('CoreToolScheduler request queueing', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -1854,7 +1854,7 @@ describe('CoreToolScheduler Sequential Execution', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, @@ -1975,7 +1975,7 @@ describe('CoreToolScheduler Sequential Execution', () => { getAllowedTools: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 39b732cd9..a77fc6707 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -112,7 +112,7 @@ describe('GeminiChat', () => { getUsageStatisticsEnabled: () => true, getDebugMode: () => false, getContentGeneratorConfig: vi.fn().mockReturnValue({ - authType: 'gemini-api-key', // Ensure this is set for fallback tests + authType: 'gemini', // Ensure this is set for fallback tests model: 'test-model', }), getModel: vi.fn().mockReturnValue('gemini-pro'), diff --git a/packages/core/src/core/nonInteractiveToolExecutor.test.ts b/packages/core/src/core/nonInteractiveToolExecutor.test.ts index 5296310f9..5b319deda 100644 --- a/packages/core/src/core/nonInteractiveToolExecutor.test.ts +++ b/packages/core/src/core/nonInteractiveToolExecutor.test.ts @@ -47,7 +47,7 @@ describe('executeToolCall', () => { getDebugMode: () => false, getContentGeneratorConfig: () => ({ model: 'test-model', - authType: 'gemini-api-key', + authType: 'gemini', }), getShellExecutionConfig: () => ({ terminalWidth: 90, diff --git a/packages/core/src/core/openaiContentGenerator/converter.ts b/packages/core/src/core/openaiContentGenerator/converter.ts index 5ec7a4935..ed2495da1 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.ts @@ -93,6 +93,14 @@ export class OpenAIContentConverter { this.schemaCompliance = schemaCompliance; } + /** + * Update the model used for response metadata (modelVersion/logging) and any + * model-specific conversion behavior. + */ + setModel(model: string): void { + this.model = model; + } + /** * Reset streaming tool calls parser for new stream processing * This should be called at the beginning of each stream to prevent