fix: preserve config base_url when API key comes from environment#1
fix: preserve config base_url when API key comes from environment#1garyblankenship wants to merge 1 commit intovoocel:mainfrom
Conversation
ProviderCredentials discarded the entire provider config (including base_url) when api_key was empty, falling back to EnvCredentials which returned no base URL for custom providers. Requests then hit the default OpenAI endpoint instead of the configured one. Also fix ensureProviderSetup to try env var fallback before erroring when a config file exists but api_key is empty.
There was a problem hiding this comment.
Pull request overview
This PR fixes provider credential resolution so that a base_url configured in settings.json is not lost when the provider api_key is supplied via environment variables, and adjusts bootstrap validation to attempt env-var fallback before erroring on missing config credentials.
Changes:
- Update
Resolved.ProviderCredentialsto preserve configuredbase_urlwhen the API key comes from environment variables. - Adjust
ensureProviderSetupto try environment-variable credentials even when a config file exists (instead of failing immediately).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
internal/config/settings.go |
Preserves config base_url when API key is resolved from the environment. |
internal/bootstrap/input.go |
Changes provider setup flow to allow env-var fallback before reporting configuration errors. |
Comments suppressed due to low confidence (1)
internal/bootstrap/input.go:104
- The new configExists + env-var fallback behavior isn’t covered by tests. Adding a bootstrap test that creates a settings.json with provider config containing base_url but empty api_key, sets the corresponding *_API_KEY env var, and asserts the resolved model is created with the configured base_url would help prevent regressions (and should also cover the “don’t silently switch providers when config exists” behavior if that’s the intent).
// Even with a config file, try env var fallback when api_key is empty.
apiKey, _ := settings.ProviderCredentials(settings.Provider)
if apiKey != "" {
return settings, envHintFor(settings), nil
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (r Resolved) ProviderCredentials(prov string) (apiKey, baseURL string) { | ||
| if pc, ok := r.Providers[prov]; ok && pc.APIKey != "" { | ||
| pc, hasConfig := r.Providers[prov] | ||
| if hasConfig && pc.APIKey != "" { | ||
| return pc.APIKey, pc.BaseURL | ||
| } | ||
| return EnvCredentials(prov) | ||
| apiKey, baseURL = EnvCredentials(prov) | ||
| if hasConfig && baseURL == "" { | ||
| baseURL = pc.BaseURL | ||
| } | ||
| return apiKey, baseURL |
There was a problem hiding this comment.
ProviderCredentials calls EnvCredentials, but EnvCredentials only reads env vars for providers listed in providerEnvVars. For custom/free-form provider names this means an API key present in the environment (e.g. derived via ProviderEnvKey) will never be picked up, even though the surrounding UX/error messages imply a generic _API_KEY fallback. Consider updating ProviderCredentials/EnvCredentials to fall back to os.Getenv(ProviderEnvKey(prov)) when the provider isn’t in providerEnvVars (and clarify how custom provider names map to valid env var identifiers).
| if prov, envKey := config.DetectEnvProvider(); prov != "" { | ||
| settings.Provider = prov | ||
| settings.Model = config.DefaultModelName(prov) | ||
| settings.SmallModel = settings.Model | ||
| return settings, fmt.Sprintf("Using %s from environment", envKey), nil | ||
| } | ||
|
|
||
| if configExists { | ||
| return settings, "", fmt.Errorf("configuration error: settings.provider=%q is missing or not configured in settings.json", settings.Provider) | ||
| } |
There was a problem hiding this comment.
ensureProviderSetup now runs DetectEnvProvider even when a config file exists. This can silently override an explicitly configured settings.provider if that provider lacks credentials but some other provider’s env var is set (e.g. OPENAI_API_KEY), masking a configuration error and potentially sending traffic to an unexpected provider. Consider skipping DetectEnvProvider when configExists is true (or only using it when no provider is configured at all) so a present settings.json remains authoritative.
Summary
base_url) whenapi_keywas empty in settings.json, falling back toEnvCredentialswhich returned no base URL for custom/proxy providers. Requests then hit the default OpenAI endpoint instead of the configured one.api_keywas empty, instead of trying the environment variable fallback first.These two fixes together allow providers configured with
base_urlin settings.json to work correctly when the API key is supplied via environment variable (e.g.OPENAI_API_KEY, or any custom provider env var).Test plan
base_urlbut emptyapi_key, set the API key via environment variable, verify requests go to the configured base URLapi_keyset directly in config still work unchangedgo test ./...passes