diff --git a/src/commands/init.ts b/src/commands/init.ts index 81e9b90..fd7ed59 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -186,21 +186,45 @@ async function promptProvider(existingConfig: Config | null): Promise, 'provider' | 'apiKey' | 'baseUrl'> | null, -): Promise { - if (!provider.needsApiKey) return undefined; - - const existingKey = getExistingApiKeyForProvider( - provider.providerKey, - provider.baseUrl, - storedConfig, - provider.apiKeyEnv, - ); - const keyResult = await password(buildApiKeyPrompt(existingKey, provider.apiKeyEnv)); +): Promise { + if (!provider.needsApiKey) return { effectiveKey: '', persistKey: undefined }; + + const storedKey = getStoredApiKeyForProvider(provider.providerKey, provider.baseUrl, storedConfig); + const envKey = process.env['COMMIT_ECHO_API_KEY']?.trim() || process.env[provider.apiKeyEnv]?.trim() || ''; + const promptKey = storedKey || envKey || ''; + + const keyResult = await password(buildApiKeyPrompt(promptKey, provider.apiKeyEnv)); if (isCancel(keyResult)) return null; - return keyResult || existingKey || ''; + + return resolveApiKeySelection(keyResult, storedKey, envKey); } interface PromptModelDependencies { @@ -371,11 +395,11 @@ async function persistSetup( } } -async function testConfiguration(config: Config, provider: ProviderSetup): Promise { +async function testConfiguration(config: Config, provider: ProviderSetup, effectiveKey?: string): Promise { const testSpinner = spinner(); testSpinner.start('Testing connection...'); try { - const resolvedKey = config.apiKey ?? process.env[provider.apiKeyEnv] ?? ''; + const resolvedKey = effectiveKey ?? config.apiKey ?? process.env[provider.apiKeyEnv] ?? ''; const { testConnection } = await import('../llm/client.js'); const modelName = await testConnection({ ...config, apiKey: resolvedKey }); @@ -404,6 +428,7 @@ function buildTemplateInfo(config: Config): string { interface CollectedSetup { config: Config; provider: ProviderSetup; + effectiveKey: string; } async function collectConfig( @@ -413,10 +438,10 @@ async function collectConfig( const provider = await promptProvider(existingConfig); if (!provider) return null; - const apiKey = await promptApiKey(provider, storedConfig); - if (apiKey === null) return null; + const apiKeyResult = await promptApiKey(provider, storedConfig); + if (apiKeyResult === null) return null; - const selectedModel = await promptModel(provider, apiKey, existingConfig); + const selectedModel = await promptModel(provider, apiKeyResult.effectiveKey, existingConfig); if (!selectedModel) return null; const limits = await promptHistoryLimits(existingConfig); @@ -427,11 +452,12 @@ async function collectConfig( return { provider, + effectiveKey: apiKeyResult.effectiveKey, config: { provider: provider.providerKey, model: selectedModel, baseUrl: provider.providerKey === CUSTOM_PROVIDER_KEY ? provider.baseUrl : undefined, - apiKey: apiKey ?? undefined, + apiKey: apiKeyResult.persistKey, historySize: limits.historySize, maxDiffSize: limits.maxDiffSize, ...withTemplateFilePrecedence( @@ -468,9 +494,9 @@ async function runInteractiveSetup(options: { installHook?: boolean; uninstallHo return; } - const { config, provider } = setup; + const { config, provider, effectiveKey } = setup; - if (provider.needsApiKey && !config.apiKey && !process.env[provider.apiKeyEnv]) { + if (provider.needsApiKey && !effectiveKey) { await persistSetup(config, options); const apiKeyEnv = pc.cyan(`$${provider.apiKeyEnv}`); const warn = pc.yellow(`\n⚠ No API key provided. Make sure to set ${apiKeyEnv} before running suggestions.`); @@ -478,7 +504,7 @@ async function runInteractiveSetup(options: { installHook?: boolean; uninstallHo return; } - if (!(await testConfiguration(config, provider))) { + if (!(await testConfiguration(config, provider, effectiveKey))) { outro('Setup cancelled.'); return; } diff --git a/tests/init-api-key-selection.test.mjs b/tests/init-api-key-selection.test.mjs new file mode 100644 index 0000000..71f9949 --- /dev/null +++ b/tests/init-api-key-selection.test.mjs @@ -0,0 +1,58 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { getStoredApiKeyForProvider, resolveApiKeySelection } from '../dist/commands/init.js'; + +const OPENAI_BASE_URL = 'https://api.openai.com/v1'; + +test('preserves an explicitly stored API key when blank input equals the generic env override', () => { + // Regression: provenance used to be inferred by comparing the raw config + // value against COMMIT_ECHO_API_KEY, so a config.json key that happened to + // equal the env var was treated as env-only and silently deleted on save. + const storedKey = getStoredApiKeyForProvider('openai', OPENAI_BASE_URL, { + provider: 'openai', + apiKey: 'sk-same', + }); + + assert.equal(storedKey, 'sk-same'); + + const selection = resolveApiKeySelection('', storedKey, 'sk-same'); + + assert.equal(selection.persistKey, 'sk-same'); + assert.equal(selection.effectiveKey, 'sk-same'); +}); + +test('never persists an API key that only came from the environment', () => { + const selection = resolveApiKeySelection('', undefined, 'sk-env-only'); + + assert.equal(selection.persistKey, undefined); + assert.equal(selection.effectiveKey, 'sk-env-only'); +}); + +test('never persists an API key when neither input nor stored key exists', () => { + const selection = resolveApiKeySelection('', undefined, ''); + + assert.equal(selection.persistKey, undefined); + assert.equal(selection.effectiveKey, ''); +}); + +test('keeps a stored key effective when the prompt is blank and no env var is set', () => { + const selection = resolveApiKeySelection('', 'sk-stored', ''); + + assert.equal(selection.persistKey, 'sk-stored'); + assert.equal(selection.effectiveKey, 'sk-stored'); +}); + +test('persists the key typed during reconfiguration', () => { + const selection = resolveApiKeySelection('sk-typed', 'sk-stored', 'sk-env'); + + assert.equal(selection.persistKey, 'sk-typed'); + assert.equal(selection.effectiveKey, 'sk-typed'); +}); + +test('prefers a typed key over the environment for the effective key', () => { + const selection = resolveApiKeySelection('sk-typed', undefined, 'sk-env'); + + assert.equal(selection.persistKey, 'sk-typed'); + assert.equal(selection.effectiveKey, 'sk-typed'); +});