From 524375508177fe86edce5f33a80cc4278afb34f7 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Sun, 26 Apr 2026 14:09:34 +0800 Subject: [PATCH] Revert "fix(cli): respect OPENAI_MODEL precedence in CLI model resolution (#3567)" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 007a109db8efdd120c923214a0ad9e973a1ee6a3. The change made `OPENAI_MODEL` outrank `settings.model.name` when looking up the active entry in `settings.modelProviders`. Combined with the core resolver's `modelProvider > cli > env > settings` priority, this caused a regression: a `/model` selection (which writes `settings.model.name`) was silently overridden whenever `OPENAI_MODEL` was set in the user's shell, with no warning surfaced. Restoring the previous behavior — looking up the provider entry by `argv.model || settings.model?.name` — preserves the implicit contract that an explicit `modelProviders` config takes precedence over stale shell defaults. Users without a `modelProviders` config are unaffected: env vars still drive model selection through the core resolver. See discussion on #3567. --- .../cli/src/utils/modelConfigUtils.test.ts | 247 ------------------ packages/cli/src/utils/modelConfigUtils.ts | 9 +- 2 files changed, 2 insertions(+), 254 deletions(-) diff --git a/packages/cli/src/utils/modelConfigUtils.test.ts b/packages/cli/src/utils/modelConfigUtils.test.ts index abd556308c5..97cea9974d4 100644 --- a/packages/cli/src/utils/modelConfigUtils.test.ts +++ b/packages/cli/src/utils/modelConfigUtils.test.ts @@ -442,187 +442,6 @@ describe('modelConfigUtils', () => { ); }); - it('should find modelProvider from OPENAI_MODEL when argv.model is not provided', () => { - const argv = {}; - const modelProvider: ProviderModelConfig = { - id: 'env-openai-model', - name: 'Env OpenAI Model', - generationConfig: { - samplingParams: { temperature: 0.6 }, - }, - }; - const settings = makeMockSettings({ - model: { name: 'settings-model' }, - modelProviders: { - [AuthType.USE_OPENAI]: [ - { id: 'settings-model', name: 'Settings Model' }, - modelProvider, - ], - }, - }); - const selectedAuthType = AuthType.USE_OPENAI; - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { - model: 'env-openai-model', - apiKey: '', - baseUrl: '', - }, - sources: {}, - warnings: [], - }); - - resolveCliGenerationConfig({ - argv, - settings, - selectedAuthType, - env: { OPENAI_MODEL: 'env-openai-model' }, - }); - - expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( - expect.objectContaining({ - modelProvider, - }), - ); - }); - - it('should find modelProvider from QWEN_MODEL when OPENAI_MODEL is not provided', () => { - const argv = {}; - const modelProvider: ProviderModelConfig = { - id: 'qwen-env-model', - name: 'Qwen Env Model', - generationConfig: { - samplingParams: { temperature: 0.7 }, - }, - }; - const settings = makeMockSettings({ - model: { name: 'settings-model' }, - modelProviders: { - [AuthType.USE_OPENAI]: [ - { id: 'settings-model', name: 'Settings Model' }, - modelProvider, - ], - }, - }); - const selectedAuthType = AuthType.USE_OPENAI; - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { - model: 'qwen-env-model', - apiKey: '', - baseUrl: '', - }, - sources: {}, - warnings: [], - }); - - resolveCliGenerationConfig({ - argv, - settings, - selectedAuthType, - env: { QWEN_MODEL: 'qwen-env-model' }, - }); - - expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( - expect.objectContaining({ - modelProvider, - }), - ); - }); - - it('should prefer OPENAI_MODEL over QWEN_MODEL and settings.model.name for USE_OPENAI provider lookup', () => { - const argv = {}; - const openAIProvider: ProviderModelConfig = { - id: 'openai-env-model', - name: 'OpenAI Env Model', - }; - const qwenProvider: ProviderModelConfig = { - id: 'qwen-env-model', - name: 'Qwen Env Model', - }; - const settings = makeMockSettings({ - model: { name: 'settings-model' }, - modelProviders: { - [AuthType.USE_OPENAI]: [ - { id: 'settings-model', name: 'Settings Model' }, - qwenProvider, - openAIProvider, - ], - }, - }); - const selectedAuthType = AuthType.USE_OPENAI; - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { - model: 'openai-env-model', - apiKey: '', - baseUrl: '', - }, - sources: {}, - warnings: [], - }); - - resolveCliGenerationConfig({ - argv, - settings, - selectedAuthType, - env: { - OPENAI_MODEL: 'openai-env-model', - QWEN_MODEL: 'qwen-env-model', - }, - }); - - expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( - expect.objectContaining({ - modelProvider: openAIProvider, - }), - ); - }); - - it('should ignore OPENAI_MODEL for non-USE_OPENAI provider lookup', () => { - const argv = {}; - const settingsModelProvider: ProviderModelConfig = { - id: 'settings-model', - name: 'Settings Model', - }; - const unrelatedOpenAIProvider: ProviderModelConfig = { - id: 'openai-env-model', - name: 'OpenAI Env Model', - }; - const settings = makeMockSettings({ - model: { name: 'settings-model' }, - modelProviders: { - [AuthType.USE_ANTHROPIC]: [ - settingsModelProvider, - unrelatedOpenAIProvider, - ], - }, - }); - const selectedAuthType = AuthType.USE_ANTHROPIC; - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { - model: 'settings-model', - apiKey: '', - baseUrl: '', - }, - sources: {}, - warnings: [], - }); - - resolveCliGenerationConfig({ - argv, - settings, - selectedAuthType, - env: { OPENAI_MODEL: 'openai-env-model' }, - }); - - expect(vi.mocked(resolveModelConfig)).toHaveBeenCalledWith( - expect.objectContaining({ - modelProvider: settingsModelProvider, - }), - ); - }); it('should not find modelProvider when authType is undefined', () => { const argv = { model: 'test-model' }; const settings = makeMockSettings({ @@ -904,71 +723,5 @@ describe('modelConfigUtils', () => { }), ); }); - - it('should respect precedence: argv.model > OPENAI_MODEL > QWEN_MODEL > settings.model.name', () => { - const mockSettings = makeMockSettings({ - model: { name: 'settings-model' }, - modelProviders: { - [AuthType.USE_OPENAI]: [ - { id: 'settings-model' } as ProviderModelConfig, - { id: 'openai-env-model' } as ProviderModelConfig, - { id: 'qwen-env-model' } as ProviderModelConfig, - { id: 'cli-model' } as ProviderModelConfig, - ], - }, - }); - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { model: 'cli-model', apiKey: '', baseUrl: '' }, - sources: {}, - warnings: [], - }); - const result1 = resolveCliGenerationConfig({ - argv: { model: 'cli-model' }, - settings: mockSettings, - selectedAuthType: AuthType.USE_OPENAI, - env: { OPENAI_MODEL: 'openai-env-model' }, - }); - expect(result1.model).toBe('cli-model'); - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { model: 'openai-env-model', apiKey: '', baseUrl: '' }, - sources: {}, - warnings: [], - }); - const result2 = resolveCliGenerationConfig({ - argv: {}, - settings: mockSettings, - selectedAuthType: AuthType.USE_OPENAI, - env: { OPENAI_MODEL: 'openai-env-model', QWEN_MODEL: 'qwen-env-model' }, - }); - expect(result2.model).toBe('openai-env-model'); - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { model: 'openai-env-model', apiKey: '', baseUrl: '' }, - sources: {}, - warnings: [], - }); - const result3 = resolveCliGenerationConfig({ - argv: {}, - settings: mockSettings, - selectedAuthType: AuthType.USE_OPENAI, - env: { OPENAI_MODEL: 'openai-env-model' }, - }); - expect(result3.model).toBe('openai-env-model'); - - vi.mocked(resolveModelConfig).mockReturnValue({ - config: { model: 'settings-model', apiKey: '', baseUrl: '' }, - sources: {}, - warnings: [], - }); - const result4 = resolveCliGenerationConfig({ - argv: {}, - settings: mockSettings, - selectedAuthType: AuthType.USE_OPENAI, - env: {}, - }); - expect(result4.model).toBe('settings-model'); - }); }); }); diff --git a/packages/cli/src/utils/modelConfigUtils.ts b/packages/cli/src/utils/modelConfigUtils.ts index 83dbae1cce8..aa5ac5e82ec 100644 --- a/packages/cli/src/utils/modelConfigUtils.ts +++ b/packages/cli/src/utils/modelConfigUtils.ts @@ -100,13 +100,8 @@ export function resolveCliGenerationConfig( if (authType && settings.modelProviders) { const providers = settings.modelProviders[authType]; if (providers && Array.isArray(providers)) { - const requestedModel = - authType === AuthType.USE_OPENAI - ? argv.model || - env['OPENAI_MODEL'] || - env['QWEN_MODEL'] || - settings.model?.name - : argv.model || settings.model?.name; + // Try to find by requested model (from CLI or settings) + const requestedModel = argv.model || settings.model?.name; if (requestedModel) { modelProvider = providers.find((p) => p.id === requestedModel) as | ProviderModelConfig