fix(config): honor configured api_key for the Vertex AI provider#3111
Open
kypkk wants to merge 2 commits into
Open
fix(config): honor configured api_key for the Vertex AI provider#3111kypkk wants to merge 2 commits into
kypkk wants to merge 2 commits into
Conversation
…api_key Reproduction for charmbracelet#3074: a vertexai provider configured with an api_key (and a custom base_url) is dropped by configureProviders because the vertexai branch only checks VERTEXAI_PROJECT/VERTEXAI_LOCATION env vars and ignores the api_key. The test documents the current buggy behavior (provider count == 0); flip the assertion to 1 once fixed.
The vertexai branch in configureProviders only checked the VERTEXAI_PROJECT/VERTEXAI_LOCATION env vars and dropped the provider when they were unset, ignoring a configured api_key entirely. Crush then treated the provider as unconfigured and prompted for an API key, even though the same api_key config works for openai/anthropic. Now, when the GCP env vars are absent, the api_key is resolved like any other provider and keeps the provider alive. buildGoogleVertexProvider wires the api_key (via WithGeminiAPIKey) and the configured base_url through to the google provider; the native WithVertex path is unchanged and still takes precedence when project/location are set. Fixes charmbracelet#3074
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for keeping a Vertex AI provider configured via api_key (and optional proxy base_url) even when VERTEXAI_PROJECT / VERTEXAI_LOCATION env vars are absent, plus a regression test for the previously dropped-provider behavior.
Changes:
- Update Vertex AI provider configuration logic to only require project/location when using native Vertex auth; otherwise allow API-key auth.
- Add regression test covering API-key + proxy config path for Vertex AI (issue #3074).
- Extend the coordinator’s Google Vertex provider builder to accept
baseURLandapiKey, selecting Vertex vs API-key auth based on provided options.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/config/load_test.go | Adds regression test ensuring VertexAI provider isn’t dropped when configured with api_key only. |
| internal/config/load.go | Adjusts credential checks so VertexAI provider is retained when API key is present even without project/location env vars. |
| internal/agent/coordinator.go | Updates Vertex provider construction to allow API-key + baseURL proxy mode when project/location are missing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+577
to
+582
| cfg := &Config{} | ||
| cfg.setDefaults("/tmp", "") | ||
| // No VERTEXAI_PROJECT / VERTEXAI_LOCATION on purpose: the user is | ||
| // authenticating via api_key + proxy, not native GCP credentials. | ||
| env := env.NewFromMap(map[string]string{}) | ||
| resolver := NewShellVariableResolver(env) |
Comment on lines
+290
to
296
| } else if v, err := resolver.ResolveValue(p.APIKey); v == "" || err != nil { | ||
| // Without native GCP credentials an API key works like any | ||
| // other provider (e.g. against a Gemini-API-compatible | ||
| // proxy); only drop the provider when neither is available. | ||
| if configExists { | ||
| slog.Warn("Skipping Vertex AI provider due to missing credentials") | ||
| c.Providers.Del(string(p.ID)) |
Comment on lines
+964
to
+973
| if project != "" && location != "" { | ||
| opts = append(opts, google.WithVertex(project, location)) | ||
| } else { | ||
| // Without native GCP credentials, authenticate with the configured | ||
| // API key (e.g. against a Gemini-API-compatible proxy). | ||
| opts = append(opts, google.WithGeminiAPIKey(apiKey)) | ||
| } | ||
| if baseURL != "" { | ||
| opts = append(opts, google.WithBaseURL(baseURL)) | ||
| } |
Author
|
I have read the Contributor License Agreement (CLA) and hereby sign the CLA. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3074
Problem
A Vertex AI provider configured with an
api_key(and optionally a custombase_urlpointing at a proxy) was dropped during config load. Thevertexaicase inconfigureProvidersonly checked theVERTEXAI_PROJECT/
VERTEXAI_LOCATIONenv vars and ignored the configuredapi_keyentirely — so Crush treated the provider as unconfigured and prompted for
an API key, even though the identical config style works for the
openaiand
anthropicproviders.Fix
Two changes, both on the Crush side (
fantasy's google provider alreadyexposes everything needed —
WithGeminiAPIKey,WithBaseURL,WithVertex— so no upstream change is required):internal/config/load.go— whenVERTEXAI_PROJECT/VERTEXAI_LOCATIONare absent, resolve the configured
api_keylike the generic providerbranch does (including
$VAR/$(cmd)templates) and keep theprovider when it resolves. The provider is only dropped when neither
native GCP credentials nor an api_key are available.
internal/agent/coordinator.go—buildGoogleVertexProvidernowreceives the resolved
api_keyandbase_url. Whenproject/locationare present the native
WithVertexpath is unchanged (and still takesprecedence); otherwise the provider is built with
WithGeminiAPIKey(apiKey). A configuredbase_urlis wired through inboth cases.
Behavior
VERTEXAI_PROJECT+VERTEXAI_LOCATIONsetapi_keyonly (no env vars)base_urlhonoredDesign note
I interpreted "api_key + custom base_url" as a Gemini-API-compatible
endpoint/proxy (
WithGeminiAPIKey+WithBaseURL), since that matcheswhat the reporter's proxy expects and how the other providers behave. If
you'd rather keep the Vertex backend and route through the proxy with
skip-auth, happy to adjust.
Testing
TestConfig_configureProvidersVertexAIWithAPIKey(fails on
main, passes here): provider with anapi_keyand no GCPenv vars is kept, with
api_key/base_urlintact and noproject/locationextra params.VertexAIWithCredentials/WithoutCredentials/MissingProjecttests still pass (env-var behavior unchanged).go test ./internal/config ./internal/agent— 336 passed.golangci-lint runclean on the touched packages.