mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-28 03:30:40 +00:00
fix(core): preserve settings-sourced apiKey when registry model envKey is absent (#3495)
* 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> * fix(core): also preserve CLI-sourced apiKey during syncAfterAuthRefresh Address review feedback: keys passed via CLI flags (e.g. --openaiApiKey) were dropped on restart because source kind 'cli' was not in the fallback allowlist. Add 'cli' to the condition and a regression test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): move apiKey preservation from applyResolvedModelDefaults to syncAfterAuthRefresh The previous fallback logic inside applyResolvedModelDefaults could leak a settings/cli-sourced apiKey to a different provider when switching models within the same authType (e.g. dashscope → openai). This is a credential safety issue because the two providers may have different baseUrls. Move the save/restore logic to syncAfterAuthRefresh Step 1, guarded by an `isUnchanged` check (same authType AND same modelId). This ensures: - Restart scenario: apiKey preserved (same model, no change) - Cross-provider switch: apiKey cleared (different modelId) Also adds two cross-provider switch tests (settings-sourced and CLI-sourced) per review feedback. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): replace non-null assertion with truthiness guard and add cold-start test - Replace `savedApiKeySource!` with a truthiness guard for safer source restoration - Add test for cold-start scenario (previousAuthType undefined) to verify no key preservation occurs on first syncAfterAuthRefresh - Fix stale "short-circuit" comment in programmatic key test Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): detect provider config hot-reload in isUnchanged check When a model provider config is hot-reloaded (e.g. via Coding Plan update) changing envKey or baseUrl while keeping the same model id, the save/restore logic must not preserve the old apiKey. Extend the isUnchanged guard to compare apiKeyEnvKey and baseUrl against the resolved model, but only after applyResolvedModelDefaults has run at least once (apiKeyEnvKey !== undefined). On first startup call these fields are still unset, so the check is skipped to preserve the settings/cli-sourced key correctly. Adds two hot-reload tests (envKey change and baseUrl change). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): use baseUrl source as hasBeenApplied signal for provider change detection Replace `apiKeyEnvKey !== undefined` guard with `baseUrl source === 'modelProviders'` to reliably detect whether applyResolvedModelDefaults has been called before. This fixes two edge cases: 1. No-envKey models: hot-reload changing baseUrl was undetected because apiKeyEnvKey remained undefined. Now baseUrl source is checked. 2. Startup with envKey but omitted baseUrl: undefined !== default URL could falsely trigger isProviderChanged. Now skipped at startup since baseUrl source is not yet 'modelProviders'. Updates hot-reload test fixtures to simulate post-apply state (baseUrl source as 'modelProviders') and adds no-envKey hot-reload test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): shallow-clone savedApiKeySource to avoid mutation risk Copy the ConfigSource object before applyResolvedModelDefaults runs, so a future refactor that mutates source objects in place won't break the save/restore logic. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: jinye.djy <jinye.djy@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
jinye
GitHub
parent
72c31d378d
commit
f7cfe53c6a
2 changed files with 675 additions and 0 deletions
|
|
@ -597,6 +597,639 @@ 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 preserve programmatic apiKey when authType and modelId unchanged (restart scenario)', () => {
|
||||
// When apiKey was set via updateCredentials (programmatic source) and
|
||||
// syncAfterAuthRefresh is called with the same authType+modelId,
|
||||
// the short-circuit should preserve the existing key.
|
||||
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);
|
||||
// Same authType + same modelId → apiKey preserved via save/restore around applyResolvedModelDefaults
|
||||
expect(gc.apiKey).toBe('programmatic-key');
|
||||
});
|
||||
|
||||
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 NOT preserve settings-sourced apiKey when switching to a different provider within same authType', () => {
|
||||
// Cross-provider switch: provider-A (settings-sourced key) → provider-B
|
||||
// Settings key must NOT leak to provider-B which may have a different baseUrl.
|
||||
const envKeyA = 'PROVIDER_KEY_A_SETTINGS_TEST';
|
||||
const envKeyB = 'PROVIDER_KEY_B_SETTINGS_TEST';
|
||||
delete process.env[envKeyA];
|
||||
delete process.env[envKeyB];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'provider-a',
|
||||
name: 'Provider A',
|
||||
baseUrl: 'https://dashscope.aliyuncs.com/compatible-mode/v1',
|
||||
envKey: envKeyA,
|
||||
},
|
||||
{
|
||||
id: 'provider-b',
|
||||
name: 'Provider B',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
envKey: envKeyB,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'provider-a',
|
||||
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' },
|
||||
},
|
||||
});
|
||||
|
||||
// Switch to provider-b (different model, same authType)
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'provider-b');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// settings-sourced key for provider-a must NOT be sent to provider-b
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
expect(gc.model).toBe('provider-b');
|
||||
expect(gc.baseUrl).toBe('https://api.openai.com/v1');
|
||||
});
|
||||
|
||||
it('should NOT preserve CLI-sourced apiKey when switching to a different provider within same authType', () => {
|
||||
// Cross-provider switch: provider-A (CLI-sourced key) → provider-B
|
||||
const envKeyA = 'PROVIDER_KEY_A_CLI_TEST';
|
||||
const envKeyB = 'PROVIDER_KEY_B_CLI_TEST';
|
||||
delete process.env[envKeyA];
|
||||
delete process.env[envKeyB];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'cli-provider-a',
|
||||
name: 'CLI Provider A',
|
||||
baseUrl: 'https://api-a.example.com/v1',
|
||||
envKey: envKeyA,
|
||||
},
|
||||
{
|
||||
id: 'cli-provider-b',
|
||||
name: 'CLI Provider B',
|
||||
baseUrl: 'https://api-b.example.com/v1',
|
||||
envKey: envKeyB,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'cli-provider-a',
|
||||
apiKey: 'cli-provided-key',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'cli', detail: '--model' },
|
||||
apiKey: { kind: 'cli', detail: '--openaiApiKey' },
|
||||
},
|
||||
});
|
||||
|
||||
// Switch to cli-provider-b (different model, same authType)
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'cli-provider-b');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// CLI key for provider-a must NOT be sent to provider-b
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
expect(gc.model).toBe('cli-provider-b');
|
||||
expect(gc.baseUrl).toBe('https://api-b.example.com/v1');
|
||||
});
|
||||
|
||||
it('should NOT preserve apiKey on first syncAfterAuthRefresh when previousAuthType is undefined (cold start)', () => {
|
||||
// Cold start: ModelsConfig created without initialAuthType, then
|
||||
// syncAfterAuthRefresh is called for the first time. previousAuthType
|
||||
// is undefined, so isUnchanged must be false — no key preservation.
|
||||
const envKey = 'COLD_START_KEY_TEST_3417';
|
||||
delete process.env[envKey];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'cold-start-model',
|
||||
name: 'Cold Start Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'cold-start-model',
|
||||
apiKey: 'stale-key-from-previous-session',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'settings', detail: 'settings.model.name' },
|
||||
apiKey: { kind: 'settings', detail: 'security.auth.apiKey' },
|
||||
},
|
||||
});
|
||||
|
||||
// First auth refresh — previousAuthType is undefined
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'cold-start-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// previousAuthType (undefined) !== USE_OPENAI → isUnchanged is false → no preservation
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
expect(gc.model).toBe('cold-start-model');
|
||||
});
|
||||
|
||||
it('should NOT preserve apiKey when same modelId but envKey changed (hot-reload)', () => {
|
||||
// Hot-reload scenario: model provider config is reloaded, changing the
|
||||
// envKey for the same model id. The old apiKey must NOT be restored.
|
||||
const oldEnvKey = 'OLD_ENV_KEY_HOT_RELOAD_TEST';
|
||||
const newEnvKey = 'NEW_ENV_KEY_HOT_RELOAD_TEST';
|
||||
delete process.env[oldEnvKey];
|
||||
delete process.env[newEnvKey];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'hot-reload-model',
|
||||
name: 'Hot Reload Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey: oldEnvKey,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'hot-reload-model',
|
||||
apiKey: 'old-api-key',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
apiKeyEnvKey: oldEnvKey,
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'settings', detail: 'settings.model.name' },
|
||||
apiKey: { kind: 'settings', detail: 'security.auth.apiKey' },
|
||||
baseUrl: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'hot-reload-model',
|
||||
detail: 'baseUrl',
|
||||
},
|
||||
apiKeyEnvKey: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'hot-reload-model',
|
||||
detail: 'envKey',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Simulate hot-reload: update registry with new envKey
|
||||
modelsConfig.reloadModelProvidersConfig({
|
||||
openai: [
|
||||
{
|
||||
id: 'hot-reload-model',
|
||||
name: 'Hot Reload Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey: newEnvKey,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// syncAfterAuthRefresh with same authType and modelId
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'hot-reload-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// envKey changed → isUnchanged is false → old key must NOT be preserved
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
expect(gc.apiKeyEnvKey).toBe(newEnvKey);
|
||||
expect(gc.model).toBe('hot-reload-model');
|
||||
});
|
||||
|
||||
it('should NOT preserve apiKey when same modelId but baseUrl changed (hot-reload)', () => {
|
||||
// Hot-reload scenario: model provider config is reloaded, changing the
|
||||
// baseUrl for the same model id. The old apiKey must NOT be restored.
|
||||
const envKey = 'BASE_URL_HOT_RELOAD_TEST';
|
||||
delete process.env[envKey];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'url-reload-model',
|
||||
name: 'URL Reload Model',
|
||||
baseUrl: 'https://old-api.example.com/v1',
|
||||
envKey,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'url-reload-model',
|
||||
apiKey: 'old-api-key',
|
||||
baseUrl: 'https://old-api.example.com/v1',
|
||||
apiKeyEnvKey: envKey,
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'settings', detail: 'settings.model.name' },
|
||||
apiKey: { kind: 'settings', detail: 'security.auth.apiKey' },
|
||||
baseUrl: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'url-reload-model',
|
||||
detail: 'baseUrl',
|
||||
},
|
||||
apiKeyEnvKey: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'url-reload-model',
|
||||
detail: 'envKey',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Simulate hot-reload: update registry with new baseUrl
|
||||
modelsConfig.reloadModelProvidersConfig({
|
||||
openai: [
|
||||
{
|
||||
id: 'url-reload-model',
|
||||
name: 'URL Reload Model',
|
||||
baseUrl: 'https://new-api.example.com/v1',
|
||||
envKey,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// syncAfterAuthRefresh with same authType and modelId
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'url-reload-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// baseUrl changed → isUnchanged is false → old key must NOT be preserved
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
expect(gc.baseUrl).toBe('https://new-api.example.com/v1');
|
||||
expect(gc.model).toBe('url-reload-model');
|
||||
});
|
||||
|
||||
it('should NOT preserve apiKey when no-envKey model has baseUrl changed (hot-reload)', () => {
|
||||
// Hot-reload scenario for a model without envKey: baseUrl changes but
|
||||
// modelId stays the same. The old apiKey must NOT be restored.
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'no-envkey-model',
|
||||
name: 'No EnvKey Model',
|
||||
baseUrl: 'https://old-api.example.com/v1',
|
||||
// no envKey
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'no-envkey-model',
|
||||
apiKey: 'old-settings-key',
|
||||
baseUrl: 'https://old-api.example.com/v1',
|
||||
},
|
||||
// Simulate post-apply state: baseUrl source is modelProviders
|
||||
generationConfigSources: {
|
||||
model: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'no-envkey-model',
|
||||
detail: 'model.id',
|
||||
},
|
||||
apiKey: { kind: 'settings', detail: 'security.auth.apiKey' },
|
||||
baseUrl: {
|
||||
kind: 'modelProviders',
|
||||
authType: 'openai',
|
||||
modelId: 'no-envkey-model',
|
||||
detail: 'baseUrl',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Simulate hot-reload: update registry with new baseUrl
|
||||
modelsConfig.reloadModelProvidersConfig({
|
||||
openai: [
|
||||
{
|
||||
id: 'no-envkey-model',
|
||||
name: 'No EnvKey Model',
|
||||
baseUrl: 'https://new-api.example.com/v1',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'no-envkey-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// baseUrl changed → isProviderChanged is true even without envKey
|
||||
expect(gc.apiKey).toBeUndefined();
|
||||
expect(gc.baseUrl).toBe('https://new-api.example.com/v1');
|
||||
expect(gc.model).toBe('no-envkey-model');
|
||||
});
|
||||
|
||||
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 preserve CLI-sourced apiKey (--openaiApiKey) when registry model envKey is absent', () => {
|
||||
// Regression: CLI-passed keys (source kind 'cli') must not be discarded
|
||||
// during syncAfterAuthRefresh when the provider's envKey is unset.
|
||||
const envKey = 'CODING_PLAN_KEY_TEST_3417_CLI';
|
||||
delete process.env[envKey];
|
||||
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
{
|
||||
id: 'cli-test-model',
|
||||
name: 'CLI Test Model',
|
||||
baseUrl: 'https://api.example.com/v1',
|
||||
envKey,
|
||||
generationConfig: {
|
||||
samplingParams: { temperature: 0.5 },
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const modelsConfig = new ModelsConfig({
|
||||
initialAuthType: AuthType.USE_OPENAI,
|
||||
modelProvidersConfig,
|
||||
generationConfig: {
|
||||
model: 'cli-test-model',
|
||||
apiKey: 'cli-provided-key',
|
||||
},
|
||||
generationConfigSources: {
|
||||
model: { kind: 'cli', detail: '--model' },
|
||||
apiKey: { kind: 'cli', detail: '--openaiApiKey' },
|
||||
},
|
||||
});
|
||||
|
||||
// Verify initial state
|
||||
expect(currentGenerationConfig(modelsConfig).apiKey).toBe(
|
||||
'cli-provided-key',
|
||||
);
|
||||
|
||||
modelsConfig.syncAfterAuthRefresh(AuthType.USE_OPENAI, 'cli-test-model');
|
||||
|
||||
const gc = currentGenerationConfig(modelsConfig);
|
||||
// CLI-sourced apiKey should be preserved as fallback
|
||||
expect(gc.apiKey).toBe('cli-provided-key');
|
||||
expect(gc.apiKeyEnvKey).toBe(envKey);
|
||||
expect(gc.model).toBe('cli-test-model');
|
||||
expect(gc.samplingParams?.temperature).toBe(0.5);
|
||||
|
||||
const sources = modelsConfig.getGenerationConfigSources();
|
||||
expect(sources['apiKey']?.kind).toBe('cli');
|
||||
expect(sources['apiKey']?.detail).toBe('--openaiApiKey');
|
||||
});
|
||||
|
||||
it('should maintain consistency between currentModelId and _generationConfig.model after initialization', () => {
|
||||
const modelProvidersConfig: ModelProvidersConfig = {
|
||||
openai: [
|
||||
|
|
|
|||
|
|
@ -877,7 +877,49 @@ export class ModelsConfig {
|
|||
if (modelId && this.modelRegistry.hasModel(authType, modelId)) {
|
||||
const resolved = this.modelRegistry.getModel(authType, modelId);
|
||||
if (resolved) {
|
||||
// When authType and modelId haven't changed (startup/restart scenario),
|
||||
// the current apiKey was already correctly resolved by
|
||||
// resolveCliGenerationConfig. Save it so we can restore it if
|
||||
// applyResolvedModelDefaults clears it (i.e. process.env[envKey] is
|
||||
// absent). For cross-provider switches (different modelId), we must
|
||||
// NOT preserve the previous key — it may belong to a different
|
||||
// service. Also detect hot-reload scenarios where the provider
|
||||
// config changed in place (same modelId, different envKey/baseUrl)
|
||||
// by comparing fields that applyResolvedModelDefaults sets. Use
|
||||
// baseUrl source === 'modelProviders' as the "has been applied"
|
||||
// signal — it covers both envKey and no-envKey models, and avoids
|
||||
// false positives when startup baseUrl differs from registry
|
||||
// default. (See #3417)
|
||||
const hasBeenApplied =
|
||||
this.generationConfigSources['baseUrl']?.kind === 'modelProviders';
|
||||
const isProviderChanged =
|
||||
hasBeenApplied &&
|
||||
(this._generationConfig.apiKeyEnvKey !== resolved.envKey ||
|
||||
this._generationConfig.baseUrl !== resolved.baseUrl);
|
||||
const isUnchanged =
|
||||
previousAuthType === authType &&
|
||||
this._generationConfig.model === modelId &&
|
||||
!isProviderChanged;
|
||||
const savedApiKey = isUnchanged
|
||||
? this._generationConfig.apiKey
|
||||
: undefined;
|
||||
const savedApiKeySource = isUnchanged
|
||||
? this.generationConfigSources['apiKey']
|
||||
? { ...this.generationConfigSources['apiKey'] }
|
||||
: undefined
|
||||
: undefined;
|
||||
|
||||
this.applyResolvedModelDefaults(resolved);
|
||||
|
||||
// Restore the previously-resolved apiKey if applyResolvedModelDefaults
|
||||
// cleared it (env var not found) and this is the same model.
|
||||
if (isUnchanged && !this._generationConfig.apiKey && savedApiKey) {
|
||||
this._generationConfig.apiKey = savedApiKey;
|
||||
if (savedApiKeySource) {
|
||||
this.generationConfigSources['apiKey'] = savedApiKeySource;
|
||||
}
|
||||
}
|
||||
|
||||
this.strictModelProviderSelection = true;
|
||||
// Clear active runtime model snapshot since we're now using a registry model
|
||||
this.activeRuntimeModelSnapshotId = undefined;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue