Skip to content

Commit ba99fa8

Browse files
PierrunoYTampagent
andauthored
fix(provider): stop dropping custom no-auth providers on restart (#558)
* fix(provider): stop dropping custom no-auth providers on restart Custom OpenAI/Anthropic-compatible catalog entries hardcode RequiresAuth as a template default for the setup wizard, not a fact about the user's actual endpoint. Leaving the credential step blank (e.g. a local llama.cpp server) still stamped the saved profile with a guessed OPENAI_API_KEY/ANTHROPIC_API_KEY env var, and that same static RequiresAuth flag then made the profile look like it was missing a required credential on the next run, silently filtering it out of /model. Add a Custom flag to the catalog descriptor so both the wizard (write side) and setupMissingCredentialEnv (read side) judge these profiles by their own explicit APIKeyEnv instead of the catalog's static default. Fixes #555 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> * fix(provider): self-heal legacy custom profiles stamped by the old wizard bug CodeRabbit flagged that profiles saved before this fix could already carry the phantom APIKeyEnv the old wizard stamped on every custom provider (e.g. OPENAI_API_KEY), and the new custom-branch logic would keep treating any non-empty APIKeyEnv as an explicit requirement — leaving already-affected users' providers filtered out of /model even after upgrading. Treat an APIKeyEnv that exactly matches the catalog's own guessed default as indistinguishable from unset, so already-broken saved profiles self-heal without a config.json migration. A deliberately different env var name (e.g. via --api-key-env) still means auth is required. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> * fix(provider): centralize custom-endpoint credential logic, fix two more surfaces Copilot's review of the earlier fix found the same RequiresAuth-as- authoritative bug in two more places that decide whether a saved provider needs a credential: - validateProviderRuntimeReady (zero providers check) would hard-error a no-auth custom endpoint with "requires API key". - providerWizardCredentialLabel and providerCredentialRequired (/providers status, wizard summary) would still claim/warn that one was needed. Added config.ProviderProfile.MissingCredentialEnv as the single source of truth for this decision (custom-endpoint self-heal included), and had setupMissingCredentialEnv and validateProviderRuntimeReady delegate to it. providerWizardCredentialLabel and providerCredentialRequired take the same Custom-aware fix directly, since their non-catalog fallback path uses runtime-resolved provider-kind metadata that shouldn't be collapsed into the canonical helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> * fix(ci): request pinned Go toolchain directly Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> --------- Co-authored-by: Amp <amp@ampcode.com>
1 parent 1805a8c commit ba99fa8

12 files changed

Lines changed: 399 additions & 59 deletions

‎go.mod‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
module github.com/Gitlawb/zero
22

3-
go 1.25.0
4-
5-
toolchain go1.26.4
3+
go 1.26.4
64

75
require (
86
charm.land/bubbles/v2 v2.1.0

‎internal/cli/provider_setup.go‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,6 @@ func validateProviderRuntimeReady(profile config.ProviderProfile) error {
460460
// A stored OAuth login (e.g. `zero auth chatgpt`) is a credential too: a
461461
// keyless token-login profile must pass the readiness check instead of being
462462
// told to set an API key it will never have.
463-
hasCredential := providerProfileHasCredential(profile) ||
464-
providerHasOAuthLogin(profile, oauthLoggedInProviders())
465463
if profile.CatalogID != "" {
466464
descriptor, err := providercatalog.Require(profile.CatalogID)
467465
if err != nil {
@@ -470,23 +468,19 @@ func validateProviderRuntimeReady(profile config.ProviderProfile) error {
470468
if !providercatalog.RuntimeSupported(descriptor) {
471469
return fmt.Errorf("provider %q uses transport %q: %s", descriptor.ID, descriptor.Transport, providercatalog.RuntimeUnsupportedReason(descriptor))
472470
}
473-
if descriptor.RequiresAuth && !hasCredential {
474-
apiKeyEnv := strings.TrimSpace(profile.APIKeyEnv)
475-
if apiKeyEnv == "" && len(descriptor.AuthEnvVars) > 0 {
476-
apiKeyEnv = descriptor.AuthEnvVars[0]
477-
}
478-
if apiKeyEnv != "" {
479-
return fmt.Errorf("provider %s requires API key; set %s", profile.Name, apiKeyEnv)
480-
}
481-
return fmt.Errorf("provider %s requires API key", profile.Name)
482-
}
471+
}
472+
if providerHasOAuthLogin(profile, oauthLoggedInProviders()) {
483473
return nil
484474
}
485-
switch profile.ProviderKind {
486-
case config.ProviderKindOpenAI, config.ProviderKindAnthropic, config.ProviderKindGoogle:
487-
if !hasCredential {
488-
return fmt.Errorf("provider %s requires API key", profile.Name)
475+
// Delegates to the single source of truth shared with the TUI and the
476+
// setup wizard, so `providers check` never disagrees with them about
477+
// whether a saved provider (including a no-auth custom endpoint) is
478+
// actually missing a credential.
479+
if envVar, missing := profile.MissingCredentialEnv(); missing {
480+
if envVar != "" {
481+
return fmt.Errorf("provider %s requires API key; set %s", profile.Name, envVar)
489482
}
483+
return fmt.Errorf("provider %s requires API key", profile.Name)
490484
}
491485
return nil
492486
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package cli
2+
3+
import (
4+
"testing"
5+
6+
"github.com/Gitlawb/zero/internal/config"
7+
)
8+
9+
// Regression for issue #555's follow-up: `zero providers check` must not
10+
// error that a no-auth custom endpoint requires an API key, matching what
11+
// /model and /providers already treat as usable.
12+
func TestValidateProviderRuntimeReadyCustomEndpoint(t *testing.T) {
13+
cases := []struct {
14+
name string
15+
profile config.ProviderProfile
16+
wantErr bool
17+
}{
18+
{
19+
name: "custom openai compatible with no credential configured",
20+
profile: config.ProviderProfile{
21+
Name: "local-llama",
22+
CatalogID: "custom-openai-compatible",
23+
BaseURL: "http://192.168.1.50:8080/v1",
24+
Model: "custom-model",
25+
},
26+
wantErr: false,
27+
},
28+
{
29+
name: "custom openai compatible with stale legacy default env",
30+
profile: config.ProviderProfile{
31+
Name: "local-llama",
32+
CatalogID: "custom-openai-compatible",
33+
BaseURL: "http://192.168.1.50:8080/v1",
34+
APIKeyEnv: "OPENAI_API_KEY",
35+
Model: "custom-model",
36+
},
37+
wantErr: false,
38+
},
39+
{
40+
name: "custom openai compatible with explicit non-default env still requires it",
41+
profile: config.ProviderProfile{
42+
Name: "local-llama",
43+
CatalogID: "custom-openai-compatible",
44+
BaseURL: "http://192.168.1.50:8080/v1",
45+
APIKeyEnv: "LLAMA_CPP_API_KEY",
46+
Model: "custom-model",
47+
},
48+
wantErr: true,
49+
},
50+
{
51+
name: "catalog provider missing key still errors",
52+
profile: config.ProviderProfile{
53+
Name: "groq",
54+
CatalogID: "groq",
55+
Model: "llama-3.3-70b-versatile",
56+
},
57+
wantErr: true,
58+
},
59+
}
60+
for _, c := range cases {
61+
t.Run(c.name, func(t *testing.T) {
62+
err := validateProviderRuntimeReady(c.profile)
63+
if (err != nil) != c.wantErr {
64+
t.Fatalf("validateProviderRuntimeReady() error = %v, wantErr %v", err, c.wantErr)
65+
}
66+
})
67+
}
68+
}

‎internal/cli/setup.go‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -503,41 +503,14 @@ func setupTryThisExample(profile config.ProviderProfile) string {
503503
return example
504504
}
505505

506+
// setupMissingCredentialEnv reports the environment variable a saved profile
507+
// is expected to read an API key from, and whether that expectation is
508+
// unmet. It delegates to config.ProviderProfile.MissingCredentialEnv, the
509+
// single source of truth shared with the TUI's provider status/wizard
510+
// surfaces and the CLI's `providers check` readiness gate, so all three never
511+
// disagree about whether a saved provider is missing a credential.
506512
func setupMissingCredentialEnv(profile config.ProviderProfile) (string, bool) {
507-
if providerProfileHasCredential(profile) {
508-
return "", false
509-
}
510-
if catalogID := strings.TrimSpace(profile.CatalogID); catalogID != "" {
511-
descriptor, err := providercatalog.Require(catalogID)
512-
if err != nil || !descriptor.RequiresAuth {
513-
return "", false
514-
}
515-
return firstNonEmptyCLI(profile.APIKeyEnv, setupProviderEnvVar(descriptor)), true
516-
}
517-
518-
switch normalizedSetupProviderKind(profile) {
519-
case config.ProviderKindOpenAI, config.ProviderKindOpenAICompatible:
520-
return firstNonEmptyCLI(profile.APIKeyEnv, "OPENAI_API_KEY"), true
521-
case config.ProviderKindAnthropic, config.ProviderKindAnthropicCompat:
522-
return firstNonEmptyCLI(profile.APIKeyEnv, "ANTHROPIC_API_KEY"), true
523-
case config.ProviderKindGoogle:
524-
return firstNonEmptyCLI(profile.APIKeyEnv, "GEMINI_API_KEY"), true
525-
default:
526-
if strings.TrimSpace(profile.APIKeyEnv) != "" {
527-
return strings.TrimSpace(profile.APIKeyEnv), true
528-
}
529-
return "", false
530-
}
531-
}
532-
533-
func normalizedSetupProviderKind(profile config.ProviderProfile) config.ProviderKind {
534-
if kind := strings.TrimSpace(string(profile.ProviderKind)); kind != "" {
535-
return config.ProviderKind(strings.ToLower(kind))
536-
}
537-
if provider := strings.TrimSpace(profile.Provider); provider != "" {
538-
return config.ProviderKind(strings.ToLower(provider))
539-
}
540-
return ""
513+
return profile.MissingCredentialEnv()
541514
}
542515

543516
func setupCheckCommand(name string) string {

‎internal/cli/setup_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,54 @@ func TestSetupMissingCredentialEnv(t *testing.T) {
6464
},
6565
want: false,
6666
},
67+
{
68+
// Regression for issue #555: a custom OpenAI-compatible profile saved
69+
// with no credential (e.g. a local llama.cpp server with no auth) must
70+
// not be reported as missing one, or it gets filtered out of /model on
71+
// the next run even though it was configured exactly as intended.
72+
name: "custom openai compatible with no credential configured",
73+
profile: config.ProviderProfile{
74+
Name: "local-llama",
75+
CatalogID: "custom-openai-compatible",
76+
BaseURL: "http://192.168.1.50:8080/v1",
77+
},
78+
want: false,
79+
},
80+
{
81+
name: "custom openai compatible with explicit api key env",
82+
profile: config.ProviderProfile{
83+
Name: "local-llama",
84+
CatalogID: "custom-openai-compatible",
85+
BaseURL: "http://192.168.1.50:8080/v1",
86+
APIKeyEnv: "LLAMA_CPP_API_KEY",
87+
},
88+
wantEnv: "LLAMA_CPP_API_KEY",
89+
want: true,
90+
},
91+
{
92+
// Self-heal regression: profiles saved by the pre-fix wizard were
93+
// stamped with the catalog's own guessed default (OPENAI_API_KEY)
94+
// even when the endpoint needed no auth. That stale value must be
95+
// treated the same as unset so already-broken saved profiles start
96+
// working again without a config.json rewrite.
97+
name: "custom openai compatible with stale legacy default env",
98+
profile: config.ProviderProfile{
99+
Name: "local-llama",
100+
CatalogID: "custom-openai-compatible",
101+
BaseURL: "http://192.168.1.50:8080/v1",
102+
APIKeyEnv: "OPENAI_API_KEY",
103+
},
104+
want: false,
105+
},
106+
{
107+
name: "custom anthropic compatible with no credential configured",
108+
profile: config.ProviderProfile{
109+
Name: "local-proxy",
110+
CatalogID: "custom-anthropic-compatible",
111+
BaseURL: "http://localhost:9000/anthropic",
112+
},
113+
want: false,
114+
},
67115
}
68116

69117
for _, tt := range tests {

‎internal/config/credentials.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"github.com/Gitlawb/zero/internal/credstore"
11+
"github.com/Gitlawb/zero/internal/providercatalog"
1112
)
1213

1314
// ProviderKeyStoreAt opens the encrypted credential store whose file backend lives
@@ -168,6 +169,86 @@ func (profile ProviderProfile) HasConfiguredCredential() bool {
168169
strings.TrimSpace(profile.AuthHeaderValue) != ""
169170
}
170171

172+
// MissingCredentialEnv reports the environment variable this profile is
173+
// expected to read an API key from, and whether that expectation is
174+
// currently unmet. It is the single source of truth for "does this saved
175+
// provider need a credential it doesn't have" — the CLI (`providers check`,
176+
// `usableSavedProviders`) and TUI (`/providers` status, provider wizard
177+
// summary) surfaces all delegate here so they never disagree about a
178+
// profile's status.
179+
//
180+
// For the catalog's two "bring your own endpoint" descriptors
181+
// (custom-openai-compatible, custom-anthropic-compatible), RequiresAuth is
182+
// just a wizard template default, not a fact about the user's actual
183+
// endpoint: a custom target may need no auth at all. So for those two only,
184+
// a credential is expected exclusively when this profile carries its own
185+
// explicit APIKeyEnv that differs from the catalog's guessed default — a
186+
// value matching that default is indistinguishable from the wizard's old
187+
// auto-stamp bug (issue #555) and is treated the same as unset, which also
188+
// self-heals profiles saved before that bug was fixed.
189+
func (profile ProviderProfile) MissingCredentialEnv() (string, bool) {
190+
if profile.HasConfiguredCredential() {
191+
return "", false
192+
}
193+
if catalogID := strings.TrimSpace(profile.CatalogID); catalogID != "" {
194+
descriptor, err := providercatalog.Require(catalogID)
195+
if err != nil {
196+
return "", false
197+
}
198+
defaultEnvVar := firstNonEmptyTrimmed(descriptor.AuthEnvVars...)
199+
if descriptor.Custom {
200+
envVar := strings.TrimSpace(profile.APIKeyEnv)
201+
if envVar == "" || envVar == defaultEnvVar {
202+
return "", false
203+
}
204+
return envVar, true
205+
}
206+
if !descriptor.RequiresAuth {
207+
return "", false
208+
}
209+
if envVar := strings.TrimSpace(profile.APIKeyEnv); envVar != "" {
210+
return envVar, true
211+
}
212+
return defaultEnvVar, true
213+
}
214+
215+
switch profile.normalizedProviderKind() {
216+
case ProviderKindOpenAI, ProviderKindOpenAICompatible:
217+
return firstNonEmptyTrimmed(profile.APIKeyEnv, "OPENAI_API_KEY"), true
218+
case ProviderKindAnthropic, ProviderKindAnthropicCompat:
219+
return firstNonEmptyTrimmed(profile.APIKeyEnv, "ANTHROPIC_API_KEY"), true
220+
case ProviderKindGoogle:
221+
return firstNonEmptyTrimmed(profile.APIKeyEnv, "GEMINI_API_KEY"), true
222+
default:
223+
if envVar := strings.TrimSpace(profile.APIKeyEnv); envVar != "" {
224+
return envVar, true
225+
}
226+
return "", false
227+
}
228+
}
229+
230+
// normalizedProviderKind resolves this profile's kind, falling back to the
231+
// legacy Provider string field (lowercased) when ProviderKind is unset —
232+
// matching how the resolver (resolver.go) treats the two fields as synonyms.
233+
func (profile ProviderProfile) normalizedProviderKind() ProviderKind {
234+
if kind := strings.TrimSpace(string(profile.ProviderKind)); kind != "" {
235+
return ProviderKind(strings.ToLower(kind))
236+
}
237+
if provider := strings.TrimSpace(profile.Provider); provider != "" {
238+
return ProviderKind(strings.ToLower(provider))
239+
}
240+
return ""
241+
}
242+
243+
func firstNonEmptyTrimmed(values ...string) string {
244+
for _, value := range values {
245+
if trimmed := strings.TrimSpace(value); trimmed != "" {
246+
return trimmed
247+
}
248+
}
249+
return ""
250+
}
251+
171252
// OAuthLoginCandidates returns the login names to try, in order, when resolving
172253
// this profile's OAuth token — used by the runtime bearer resolver, the Codex
173254
// account-header resolver, and the onboarding presence check so all three agree.

0 commit comments

Comments
 (0)