Fix API key management and remove Groq provider (microsoft#2559)

* Fix google and anthropic key management. Also remove groq

* Fix model providers getting stuck with bad keys

Author: lramos15

src/extension/byok/common/byokProvider.ts

@@ -191,3 +191,67 @@ export function isBYOKEnabled(copilotToken: Omit<CopilotToken, 'token'>, capiCli
 	const byokAllowed = (copilotToken.isInternal || copilotToken.isIndividual) && !isGHE;
 	return byokAllowed;
 }
+
+/**
+ * Result of handling an API key update operation.
+ */
+export interface HandleAPIKeyUpdateResult {
+	/**
+	 * The new API key value, or undefined if the key was deleted or operation was cancelled.
+	 */
+	apiKey: string | undefined;
+	/**
+	 * Whether the API key was deleted (user entered empty string during reconfigure).
+	 */
+	deleted: boolean;
+	/**
+	 * Whether the operation was cancelled (user dismissed the input).
+	 */
+	cancelled: boolean;
+}
+
+/**
+ * Storage service interface for BYOK API key operations.
+ * This is a minimal interface to avoid importing the full IBYOKStorageService in common code.
+ */
+export interface IBYOKStorageServiceLike {
+	getAPIKey(providerName: string, modelId?: string): Promise<string | undefined>;
+	storeAPIKey(providerName: string, apiKey: string, authType: BYOKAuthType, modelId?: string): Promise<void>;
+	deleteAPIKey(providerName: string, authType: BYOKAuthType, modelId?: string): Promise<void>;
+}
+
+/**
+ * Handles API key update flow for BYOK providers using a consistent pattern.
+ * This utility handles all three cases from promptForAPIKey:
+ * - undefined: user cancelled/dismissed the input
+ * - empty string: user wants to delete the saved key (only when reconfiguring)
+ * - non-empty string: user provided a new API key
+ *
+ * @param providerName - Name of the provider (e.g., 'Anthropic', 'Gemini')
+ * @param storageService - Storage service for API key operations
+ * @param promptForAPIKeyFn - Function to prompt user for API key
+ * @returns Result containing the new API key (if any) and status flags
+ */
+export async function handleAPIKeyUpdate(
+	providerName: string,
+	storageService: IBYOKStorageServiceLike,
+	promptForAPIKeyFn: (providerName: string, reconfigure: boolean) => Promise<string | undefined>
+): Promise<HandleAPIKeyUpdateResult> {
+	const existingKey = await storageService.getAPIKey(providerName);
+	const isReconfiguring = existingKey !== undefined;
+
+	const newAPIKey = await promptForAPIKeyFn(providerName, isReconfiguring);
+
+	if (newAPIKey === undefined) {
+		// User cancelled/dismissed the input
+		return { apiKey: undefined, deleted: false, cancelled: true };
+	} else if (newAPIKey === '') {
+		// User wants to delete the key (only valid when reconfiguring)
+		await storageService.deleteAPIKey(providerName, BYOKAuthType.GlobalApiKey);
+		return { apiKey: undefined, deleted: true, cancelled: false };
+	} else {
+		// User provided a new API key
+		await storageService.storeAPIKey(providerName, newAPIKey, BYOKAuthType.GlobalApiKey);
+		return { apiKey: newAPIKey, deleted: false, cancelled: false };
+	}
+}

src/extension/byok/vscode-node/anthropicProvider.ts

@@ -17,7 +17,7 @@ import { toErrorMessage } from '../../../util/common/errorMessage';
 import { RecordedProgress } from '../../../util/common/progressRecorder';
 import { generateUuid } from '../../../util/vs/base/common/uuid';
 import { anthropicMessagesToRawMessagesForLogging, apiMessageToAnthropicMessage } from '../common/anthropicMessageConverter';
-import { BYOKAuthType, BYOKKnownModels, byokKnownModelsToAPIInfo, BYOKModelCapabilities, BYOKModelProvider, LMResponsePart } from '../common/byokProvider';
+import { BYOKAuthType, BYOKKnownModels, byokKnownModelsToAPIInfo, BYOKModelCapabilities, BYOKModelProvider, handleAPIKeyUpdate, LMResponsePart } from '../common/byokProvider';
 import { IBYOKStorageService } from './byokStorageService';
 import { promptForAPIKey } from './byokUIService';
 
@@ -100,9 +100,9 @@ export class AnthropicLMProvider implements BYOKModelProvider<LanguageModelChatI
 	}
 
 	async updateAPIKey(): Promise<void> {
-		this._apiKey = await promptForAPIKey(AnthropicLMProvider.providerName, await this._byokStorageService.getAPIKey(AnthropicLMProvider.providerName) !== undefined);
-		if (this._apiKey) {
-			await this._byokStorageService.storeAPIKey(AnthropicLMProvider.providerName, this._apiKey, BYOKAuthType.GlobalApiKey);
+		const result = await handleAPIKeyUpdate(AnthropicLMProvider.providerName, this._byokStorageService, promptForAPIKey);
+		if (!result.cancelled) {
+			this._apiKey = result.apiKey;
 			this._anthropicAPIClient = undefined;
 		}
 	}

src/extension/byok/vscode-node/baseOpenAICompatibleProvider.ts

@@ -81,6 +81,12 @@ export abstract class BaseOpenAICompatibleLMProvider implements BYOKModelProvide
 				}
 			}
 		} catch (e) {
+			// Likely bad API key so we will prompt user to update it one more time
+			if (!options.silent && e instanceof Error && e.message.includes('key')) {
+				await this.updateAPIKey();
+				// Silent as to not prompt the user again
+				return this.provideLanguageModelChatInformation({ silent: true }, token);
+			}
 			this._logService.error(e, `Error fetching available ${this._name} models`);
 			return [];
 		}

src/extension/byok/vscode-node/byokContribution.ts

@@ -19,7 +19,6 @@ import { BYOKStorageService, IBYOKStorageService } from './byokStorageService';
 import { CustomOAIModelConfigurator } from './customOAIModelConfigurator';
 import { CustomOAIBYOKModelProvider } from './customOAIProvider';
 import { GeminiNativeBYOKLMProvider } from './geminiNativeProvider';
-import { GroqBYOKLMProvider } from './groqProvider';
 import { OllamaLMProvider } from './ollamaProvider';
 import { OAIBYOKLMProvider } from './openAIProvider';
 import { OpenRouterLMProvider } from './openRouterProvider';
@@ -88,7 +87,6 @@ export class BYOKContrib extends Disposable implements IExtensionContribution {
 			const knownModels = await this.fetchKnownModelList(this._fetcherService);
 			this._providers.set(OllamaLMProvider.providerName.toLowerCase(), instantiationService.createInstance(OllamaLMProvider, this._configurationService.getConfig(ConfigKey.OllamaEndpoint), this._byokStorageService));
 			this._providers.set(AnthropicLMProvider.providerName.toLowerCase(), instantiationService.createInstance(AnthropicLMProvider, knownModels[AnthropicLMProvider.providerName], this._byokStorageService));
-			this._providers.set(GroqBYOKLMProvider.providerName.toLowerCase(), instantiationService.createInstance(GroqBYOKLMProvider, knownModels[GroqBYOKLMProvider.providerName], this._byokStorageService));
 			this._providers.set(GeminiNativeBYOKLMProvider.providerName.toLowerCase(), instantiationService.createInstance(GeminiNativeBYOKLMProvider, knownModels[GeminiNativeBYOKLMProvider.providerName], this._byokStorageService));
 			this._providers.set(XAIBYOKLMProvider.providerName.toLowerCase(), instantiationService.createInstance(XAIBYOKLMProvider, knownModels[XAIBYOKLMProvider.providerName], this._byokStorageService));
 			this._providers.set(OAIBYOKLMProvider.providerName.toLowerCase(), instantiationService.createInstance(OAIBYOKLMProvider, knownModels[OAIBYOKLMProvider.providerName], this._byokStorageService));

src/extension/byok/vscode-node/geminiNativeProvider.ts

@@ -13,7 +13,7 @@ import { IRequestLogger } from '../../../platform/requestLogger/node/requestLogg
 import { toErrorMessage } from '../../../util/common/errorMessage';
 import { RecordedProgress } from '../../../util/common/progressRecorder';
 import { generateUuid } from '../../../util/vs/base/common/uuid';
-import { BYOKAuthType, BYOKKnownModels, byokKnownModelsToAPIInfo, BYOKModelCapabilities, BYOKModelProvider, LMResponsePart } from '../common/byokProvider';
+import { BYOKAuthType, BYOKKnownModels, byokKnownModelsToAPIInfo, BYOKModelCapabilities, BYOKModelProvider, handleAPIKeyUpdate, LMResponsePart } from '../common/byokProvider';
 import { toGeminiFunction as toGeminiFunctionDeclaration, ToolJsonSchema } from '../common/geminiFunctionDeclarationConverter';
 import { apiMessageToGeminiMessage, geminiMessagesToRawMessagesForLogging } from '../common/geminiMessageConverter';
 import { IBYOKStorageService } from './byokStorageService';
@@ -59,9 +59,10 @@ export class GeminiNativeBYOKLMProvider implements BYOKModelProvider<LanguageMod
 	}
 
 	async updateAPIKey(): Promise<void> {
-		this._apiKey = await promptForAPIKey(GeminiNativeBYOKLMProvider.providerName, await this._byokStorageService.getAPIKey(GeminiNativeBYOKLMProvider.providerName) !== undefined);
-		if (this._apiKey) {
-			await this._byokStorageService.storeAPIKey(GeminiNativeBYOKLMProvider.providerName, this._apiKey, BYOKAuthType.GlobalApiKey);
+		const result = await handleAPIKeyUpdate(GeminiNativeBYOKLMProvider.providerName, this._byokStorageService, promptForAPIKey);
+		if (!result.cancelled) {
+			this._apiKey = result.apiKey;
+			this._genAIClient = undefined;
 		}
 	}