mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-01 21:20:44 +00:00
fix(core): preserve settings-sourced apiKey when registry model envKey is absent (#3417)
On restart, `applyResolvedModelDefaults` unconditionally cleared the apiKey resolved from `settings.security.auth.apiKey` (layer 4 fallback) and only read from `process.env[model.envKey]`. When the provider-specific env var was absent (e.g. key stored only in settings), the correctly resolved key was discarded, causing a 401 error. Now capture the previously-resolved apiKey before clearing and fall back to it when `process.env[model.envKey]` is empty, but only for safe source kinds (`settings` and general `env` without `via.modelProviders`). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
jinye.djy
parent
afbb5e71db
commit
3a1087f7a0
2 changed files with 274 additions and 0 deletions
|
|
@ -597,6 +597,256 @@ describe('ModelsConfig', () => {
|
|||
expect(gc.samplingParams?.temperature).toBe(0.9); // Preserved from initial config
|
||||
});
|
||||
|
||||
it('should fall back to settings-sourced apiKey when registry model envKey is not in process.env (restart scenario)', () => {
|
||||
// Simulate the restart scenario from issue #3417:
|
||||
// 1. User has settings.security.auth.apiKey = 'settings-api-key'
|
||||
// 2. modelProviders.openai has a model with envKey = 'CODING_PLAN_KEY'
|
||||
// 3. process.env['CODING_PLAN_KEY'] is NOT set
|
||||
// 4. resolveCliGenerationConfig correctly resolved apiKey from settings (layer 4)
|
||||
// 5. syncAfterAuthRefresh should NOT discard the settings-sourced key
|
||||
|
||||
const envKey = 'CODING_PLAN_KEY_TEST_3417';
|
||||
// Ensure the env var is NOT set
|
||||
delete process.env[envKey];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'qwen3.5-plus',
|
||||
name: 'Test Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey,
|
||||
generationConfig: {
|
||||
samplingParams: { temperature: 0.3 },
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// ModelsConfig initialized with settings-sourced apiKey (as would happen at startup)
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'qwen3.5-plus',
|
||||
apiKey: 'settings-api-key',
|
||||
baseUrl: 'https://dashscope.aliyuncs.com/compatible-mode/v1',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'settings', detail: 'settings.model.name' },
|
||||
apiKey: { kind: 'settings', detail: 'security.auth.apiKey' },
|
||||
baseUrl: { kind: 'settings', detail: 'security.auth.baseUrl' },
|
||||
},
|
||||
});
|
||||
|
||||
// Verify initial state
|
||||
expect(currentGenerationConfig(modelsConfig).apiKey).toBe(
|
||||
'settings-api-key',
|
||||
);
|
||||
|
||||
// Simulate what refreshAuth does on startup
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'qwen3.5-plus');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// The settings-sourced apiKey should be preserved as fallback
|
||||
expect(gc.apiKey).toBe('settings-api-key');
|
||||
// envKey metadata should still be set for diagnostics
|
||||
expect(gc.apiKeyEnvKey).toBe(envKey);
|
||||
// Model and other provider config should be applied
|
||||
expect(gc.model).toBe('qwen3.5-plus');
|
||||
expect(gc.samplingParams?.temperature).toBe(0.3);
|
||||
|
||||
// Source should still reflect settings origin
|
||||
const sources = modelsConfig.getGenerationConfigSources();
|
||||
expect(sources['apiKey']?.kind).toBe('settings');
|
||||
});
|
||||
|
||||
it('should prefer env var over settings apiKey when both exist (restart scenario)', () => {
|
||||
const envKey = 'CODING_PLAN_KEY_TEST_3417_PREFER';
|
||||
// Set the env var
|
||||
process.env[envKey] = 'env-api-key';
|
||||
|
||||
try {
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'test-model',
|
||||
name: 'Test Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'test-model',
|
||||
apiKey: 'settings-api-key',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'settings', detail: 'settings.model.name' },
|
||||
apiKey: { kind: 'settings', detail: 'security.auth.apiKey' },
|
||||
},
|
||||
});
|
||||
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'test-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// Env var should take priority over settings apiKey
|
||||
expect(gc.apiKey).toBe('env-api-key');
|
||||
expect(gc.apiKeyEnvKey).toBe(envKey);
|
||||
|
||||
const sources = modelsConfig.getGenerationConfigSources();
|
||||
expect(sources['apiKey']?.kind).toBe('env');
|
||||
} finally {
|
||||
delete process.env[envKey];
|
||||
}
|
||||
});
|
||||
|
||||
it('should NOT preserve programmatic apiKey when registry model envKey is absent', () => {
|
||||
// When apiKey was set via updateCredentials (programmatic source),
|
||||
// applyResolvedModelDefaults should NOT preserve it — provider atomicity.
|
||||
const envKey = 'CODING_PLAN_KEY_TEST_3417_PROG';
|
||||
delete process.env[envKey];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'provider-model',
|
||||
name: 'Provider Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'provider-model',
|
||||
apiKey: 'programmatic-key',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'programmatic', detail: 'updateCredentials' },
|
||||
apiKey: { kind: 'programmatic', detail: 'updateCredentials' },
|
||||
},
|
||||
});
|
||||
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'provider-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// Programmatic apiKey should NOT be preserved
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should NOT preserve env apiKey with via.modelProviders during model switch', () => {
|
||||
// When switching from model-a to model-b, model-a's provider-specific
|
||||
// envKey value should NOT be reused for model-b — they may target
|
||||
// different services with different credentials.
|
||||
const envKeyA = 'PROVIDER_KEY_A_TEST_3417';
|
||||
const envKeyB = 'PROVIDER_KEY_B_TEST_3417';
|
||||
delete process.env[envKeyA];
|
||||
delete process.env[envKeyB];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'model-a',
|
||||
name: 'Model A',
|
||||
baseUrl: 'https://api-a.example.com/v1',
|
||||
envKey: envKeyA,
|
||||
},
|
||||
{
|
||||
id: 'model-b',
|
||||
name: 'Model B',
|
||||
baseUrl: 'https://api-b.example.com/v1',
|
||||
envKey: envKeyB,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'model-a',
|
||||
apiKey: 'key-for-model-a',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'model-a',
|
||||
detail: 'model.id',
|
||||
},
|
||||
apiKey: {
|
||||
kind: 'env',
|
||||
envKey: envKeyA,
|
||||
via: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'model-a',
|
||||
detail: 'envKey',
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Switch to model-b whose envKey is also not set
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'model-b');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// model-a's key should NOT be reused for model-b
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
expect(gc.model).toBe('model-b');
|
||||
});
|
||||
|
||||
it('should preserve general env var apiKey (e.g. OPENAI_API_KEY) when provider envKey is absent', () => {
|
||||
// If the user has OPENAI_API_KEY set but NOT the provider-specific envKey,
|
||||
// the general env var should be preserved as a fallback.
|
||||
const providerEnvKey = 'SPECIFIC_PROVIDER_KEY_TEST_3417';
|
||||
delete process.env[providerEnvKey];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'test-model',
|
||||
name: 'Test Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey: providerEnvKey,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// resolveCliGenerationConfig resolved apiKey from OPENAI_API_KEY (layer 3)
|
||||
// — source has kind:'env' but no 'via' (general env var, not provider-specific)
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'test-model',
|
||||
apiKey: 'openai-api-key-value',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'settings', detail: 'settings.model.name' },
|
||||
apiKey: { kind: 'env', envKey: 'OPENAI_API_KEY' },
|
||||
},
|
||||
});
|
||||
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'test-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// General env var key should be preserved
|
||||
expect(gc.apiKey).toBe('openai-api-key-value');
|
||||
|
||||
const sources = modelsConfig.getGenerationConfigSources();
|
||||
expect(sources['apiKey']?.kind).toBe('env');
|
||||
expect(sources['apiKey']?.envKey).toBe('OPENAI_API_KEY');
|
||||
});
|
||||
|
||||
it('should maintain consistency between currentModelId and _generationConfig.model after initialization', () => {
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
|
|
|
|||
|
|
@ -708,6 +708,13 @@ export class ModelsConfig {
|
|||
|
||||
// Clear credentials to avoid reusing previous model's API key
|
||||
|
||||
// Capture the previously-resolved apiKey before clearing.
|
||||
// On restart, resolveCliGenerationConfig may have resolved the key from
|
||||
// settings.security.auth.apiKey (layer 4 fallback). We preserve it here
|
||||
// so it can serve as a fallback when process.env[model.envKey] is absent.
|
||||
const previousApiKey = this._generationConfig.apiKey;
|
||||
const previousApiKeySource = this.generationConfigSources['apiKey'];
|
||||
|
||||
// For Qwen OAuth, apiKey must always be a placeholder. It will be dynamically
|
||||
// replaced when building requests. Do not preserve any previous key or read
|
||||
// from envKey.
|
||||
|
|
@ -742,6 +749,23 @@ export class ModelsConfig {
|
|||
detail: 'envKey',
|
||||
},
|
||||
};
|
||||
} else if (
|
||||
previousApiKey &&
|
||||
this.currentAuthType !== AuthType.QWEN_OAUTH &&
|
||||
(previousApiKeySource?.kind === 'settings' ||
|
||||
(previousApiKeySource?.kind === 'env' && !previousApiKeySource.via))
|
||||
) {
|
||||
// Fall back to the previously-resolved key from settings or a general
|
||||
// environment variable (e.g., settings.security.auth.apiKey or
|
||||
// OPENAI_API_KEY).
|
||||
//
|
||||
// Sources that are NOT preserved:
|
||||
// - 'programmatic' (from updateCredentials) — provider atomicity
|
||||
// - 'env' with via.modelProviders — key from a different provider's
|
||||
// envKey; reusing it for another model may send wrong credentials
|
||||
// to the wrong service.
|
||||
this._generationConfig.apiKey = previousApiKey;
|
||||
this.generationConfigSources['apiKey'] = previousApiKeySource!;
|
||||
}
|
||||
this._generationConfig.apiKeyEnvKey = model.envKey;
|
||||
this.generationConfigSources['apiKeyEnvKey'] = {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue