fix(cli): recognize all providers in install and improve missing key error#1716
Conversation
mesutoezdil
requested review from
EItanya,
ilackarms and
yuval-k
as code owners
There was a problem hiding this comment.
Pull request overview
Fixes kagent install incorrectly defaulting unknown KAGENT_DEFAULT_MODEL_PROVIDER values to OpenAI (and therefore requiring OPENAI_API_KEY), by expanding provider recognition and improving the missing-key guidance.
Changes:
- Extend
GetModelProviderto recognize Gemini, GeminiVertexAI, AnthropicVertexAI, and Bedrock provider values. - Update
GetProviderAPIKeyto requireGOOGLE_API_KEYfor the Gemini provider. - Improve install-time error output to mention
KAGENT_DEFAULT_MODEL_PROVIDERas an alternative path when an API key is missing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| go/core/cli/internal/cli/agent/install.go | Adds an extra hint in the “missing API key” error output pointing users to KAGENT_DEFAULT_MODEL_PROVIDER. |
| go/core/cli/internal/cli/agent/const.go | Expands provider parsing to include additional providers and adds Gemini API key env-var mapping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -51,7 +61,11 @@ func GetProviderAPIKey(provider v1alpha2.ModelProvider) string { | |||
| return env.AnthropicAPIKey.Name() | |||
| case v1alpha2.ModelProviderAzureOpenAI: | |||
| return env.AzureOpenAIAPIKey.Name() | |||
| case v1alpha2.ModelProviderGemini: | |||
| return env.GoogleAPIKey.Name() | |||
| default: | |||
There was a problem hiding this comment.
GetProviderAPIKey only matches exact enum values (e.g., ModelProviderAnthropic == "Anthropic"). Elsewhere the CLI commonly uses helm-key/lowercase provider strings (e.g., modelProvider: anthropic in manifests/tests), and ValidateAPIKey passes that string through as v1alpha2.ModelProvider(modelProvider), which will currently fall into the default branch and skip required API-key validation. Consider normalizing the input inside GetProviderAPIKey (e.g., accept both enum values and GetModelProviderHelmValuesKey(...) strings, or downcase/translate before switching) so it works with both representations.
mesutoezdil
force-pushed
the
fix/install-provider-api-key-check
branch
2 times, most recently
from
60f4293 to
cd3e54b
Compare
jmhbh
left a comment
jmhbh
left a comment
There was a problem hiding this comment.
LGTM, but the copilot feedback above regarding support for GEMINI_API_KEY makes sense to me and should be addressed
mesutoezdil
force-pushed
the
fix/install-provider-api-key-check
branch
from
b002528 to
c0782e3
Compare
…error GetModelProvider was missing Gemini, GeminiVertexAI, AnthropicVertexAI and Bedrock from its switch statement so any of those values in KAGENT_DEFAULT_MODEL_PROVIDER fell through to the default and came back as OpenAI, which then required OPENAI_API_KEY to be set. GetProviderAPIKey now covers Gemini with the same fallback logic used in the runtime (go/adk/pkg/agent/agent.go): prefer GOOGLE_API_KEY, fall back to GEMINI_API_KEY if the former is not set. The error message now tells the user about KAGENT_DEFAULT_MODEL_PROVIDER so they know how to switch to a different provider. Closes kagent-dev#1622 Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
mesutoezdil
force-pushed
the
fix/install-provider-api-key-check
branch
from
c0782e3 to
8effec5
Compare
|
Good point. @jmhbh Updated |
Closes #1622.
GetModelProviderwas missing Gemini, GeminiVertexAI, AnthropicVertexAI and Bedrock from its switch. Any of those values inKAGENT_DEFAULT_MODEL_PROVIDERfell through to the default and came back as OpenAI, which then requiredOPENAI_API_KEYeven though the user had no intention of using OpenAI.GetProviderAPIKeynow also covers Gemini (GOOGLE_API_KEY). Providers that rely on cloud credentials rather than an API key (Ollama, Bedrock, VertexAI variants) already returned empty string and are left as is.The error message now mentions
KAGENT_DEFAULT_MODEL_PROVIDERso users know they can switch providers instead of being stuck.