Skip to content

Commit 3524f79

Browse files
fix(cli): warn when ZERO_PROVIDER overrides a providers-use selection (#767)
Closes #721. `zero providers use <name>` wrote activeProvider to config.json and reported "Active provider set to <name>" even when ZERO_PROVIDER was set, which applyEnv makes win over config.json unconditionally. So the switch was reported as a success while `providers current` still showed the env-pinned provider, a silent no-op. Detect the override and say so: the text path prints a note to stderr naming the env var and the provider that stays active, and the --json path adds effectiveProvider / overriddenByEnv fields so a machine consumer is not misled either. The config write still lands unchanged; only the reporting is honest. The env read is injected via a new appDeps.getenv (os.Getenv in production, left nil by fillAppDeps like exportActiveProvider) so tests stay hermetic against an ambient ZERO_PROVIDER. The related env-derived-profile "not found" case the issue also mentions is already handled by the unpersisted-provider path (#707) and #716.
1 parent b1f4173 commit 3524f79

3 files changed

Lines changed: 134 additions & 3 deletions

File tree

‎internal/cli/app.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ type appDeps struct {
5858
// config.SetActiveProviderEnv, set in defaultAppDeps — deliberately NOT filled
5959
// by fillAppDeps, so tests never mutate the process environment unless they
6060
// inject it). nil ⇒ no export.
61-
exportActiveProvider func(providerName string)
61+
exportActiveProvider func(providerName string)
62+
// getenv reads a process environment variable (production: os.Getenv, set in
63+
// defaultAppDeps — deliberately NOT filled by fillAppDeps, so tests are hermetic
64+
// against ambient vars like ZERO_PROVIDER unless they inject it). nil ⇒ empty.
65+
getenv func(string) string
6266
probeProviderHealth func(context.Context, providerhealth.Options) providerhealth.Result
6367
discoverProviderModels func(context.Context, config.ProviderProfile) ([]providermodeldiscovery.Model, error)
6468
detectLocalRuntimes func(context.Context, provideronboarding.LocalDetectOptions) []provideronboarding.DetectedLocalRuntime
@@ -118,6 +122,7 @@ func defaultAppDeps() appDeps {
118122
stdin: os.Stdin,
119123
userConfigPath: config.DefaultUserConfigPath,
120124
exportActiveProvider: config.SetActiveProviderEnv,
125+
getenv: os.Getenv,
121126
resolveConfig: func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) {
122127
options, err := config.DefaultResolveOptions(workspaceRoot)
123128
if err != nil {

‎internal/cli/provider_onboarding.go‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,21 +74,51 @@ func runProvidersUse(args []string, stdout io.Writer, stderr io.Writer, deps app
7474
return writeAppError(stderr, err.Error(), exitCrash)
7575
}
7676

77+
override := activeProviderEnvOverride(deps.getenv, cfg.ActiveProvider)
7778
if options.json {
78-
if err := writePrettyJSON(stdout, map[string]any{
79+
payload := map[string]any{
7980
"activeProvider": cfg.ActiveProvider,
8081
"configPath": configPath,
81-
}); err != nil {
82+
}
83+
if override != "" {
84+
// A JSON consumer must not read this as an effective switch either.
85+
payload["effectiveProvider"] = override
86+
payload["overriddenByEnv"] = config.ActiveProviderEnv
87+
}
88+
if err := writePrettyJSON(stdout, payload); err != nil {
8289
return exitCrash
8390
}
8491
return exitSuccess
8592
}
8693
if _, err := fmt.Fprintf(stdout, "Active provider set to %s\nnext: %s\n", cfg.ActiveProvider, providerCheckCommand(cfg.ActiveProvider, false)); err != nil {
8794
return exitCrash
8895
}
96+
if override != "" {
97+
if _, err := fmt.Fprintf(stderr, "Note: %s=%s is set and overrides config.json, so %s stays the active provider until you unset %s.\n", config.ActiveProviderEnv, override, override, config.ActiveProviderEnv); err != nil {
98+
return exitCrash
99+
}
100+
}
89101
return exitSuccess
90102
}
91103

104+
// activeProviderEnvOverride returns the ZERO_PROVIDER value when it is set and
105+
// names a DIFFERENT provider than the one just selected, meaning the saved
106+
// `providers use` selection will NOT be the effective active provider until the
107+
// env var is unset. applyEnv (resolver.go) makes ZERO_PROVIDER win over
108+
// config.json unconditionally, so reporting the write as a plain success reads as
109+
// a switch that silently has no effect (issue #721). Empty when nothing overrides
110+
// (including when getenv is nil, e.g. a test that did not inject the environment).
111+
func activeProviderEnvOverride(getenv func(string) string, selected string) string {
112+
if getenv == nil {
113+
return ""
114+
}
115+
override := strings.TrimSpace(getenv(config.ActiveProviderEnv))
116+
if override == "" || strings.EqualFold(override, strings.TrimSpace(selected)) {
117+
return ""
118+
}
119+
return override
120+
}
121+
92122
func runProvidersSetup(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int {
93123
options, help, err := parseProviderSetupArgs(args)
94124
if err != nil {

‎internal/cli/provider_onboarding_test.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,102 @@ func TestRunProvidersUseJSONIncludesActiveProviderAndConfigPath(t *testing.T) {
7575
}
7676
}
7777

78+
func providersUseOverrideConfig(t *testing.T) string {
79+
t.Helper()
80+
configPath := filepath.Join(t.TempDir(), "config.json")
81+
writeProviderOnboardingConfig(t, configPath, config.FileConfig{
82+
ActiveProvider: "work",
83+
Providers: []config.ProviderProfile{
84+
{Name: "work", ProviderKind: config.ProviderKindOpenAI, BaseURL: config.OpenAIBaseURL, Model: "gpt-4.1"},
85+
{Name: "fast", ProviderKind: config.ProviderKindOpenAICompatible, BaseURL: "https://api.groq.com/openai/v1", Model: "llama-3.3-70b-versatile"},
86+
},
87+
})
88+
return configPath
89+
}
90+
91+
// The write to config.json still succeeds, but when ZERO_PROVIDER names a
92+
// different provider the saved selection is NOT effective, so the command must
93+
// warn instead of reporting a silent success (issue #721).
94+
func TestRunProvidersUseWarnsWhenEnvOverrides(t *testing.T) {
95+
var stdout, stderr bytes.Buffer
96+
configPath := providersUseOverrideConfig(t)
97+
deps := providerSetupDeps(configPath)
98+
deps.getenv = func(key string) string {
99+
if key == config.ActiveProviderEnv {
100+
return "work" // ZERO_PROVIDER=work overrides the switch to fast
101+
}
102+
return ""
103+
}
104+
105+
if code := runWithDeps([]string{"providers", "use", "fast"}, &stdout, &stderr, deps); code != exitSuccess {
106+
t.Fatalf("exit = %d, want %d: %s", code, exitSuccess, stderr.String())
107+
}
108+
if cfg := readFileConfig(t, configPath); cfg.ActiveProvider != "fast" {
109+
t.Fatalf("ActiveProvider = %q, want fast (the write still lands)", cfg.ActiveProvider)
110+
}
111+
if !strings.Contains(stdout.String(), "Active provider set to fast") {
112+
t.Fatalf("stdout missing the success line: %q", stdout.String())
113+
}
114+
note := stderr.String()
115+
for _, want := range []string{config.ActiveProviderEnv, "overrides config.json", "work"} {
116+
if !strings.Contains(note, want) {
117+
t.Fatalf("override note missing %q, got %q", want, note)
118+
}
119+
}
120+
}
121+
122+
func TestRunProvidersUseJSONFlagsEnvOverride(t *testing.T) {
123+
var stdout, stderr bytes.Buffer
124+
deps := providerSetupDeps(providersUseOverrideConfig(t))
125+
deps.getenv = func(key string) string {
126+
if key == config.ActiveProviderEnv {
127+
return "work"
128+
}
129+
return ""
130+
}
131+
132+
if code := runWithDeps([]string{"providers", "use", "fast", "--json"}, &stdout, &stderr, deps); code != exitSuccess {
133+
t.Fatalf("exit = %d, want %d: %s", code, exitSuccess, stderr.String())
134+
}
135+
var payload struct {
136+
ActiveProvider string `json:"activeProvider"`
137+
EffectiveProvider string `json:"effectiveProvider"`
138+
OverriddenByEnv string `json:"overriddenByEnv"`
139+
}
140+
if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil {
141+
t.Fatalf("JSON did not decode: %v\n%s", err, stdout.String())
142+
}
143+
if payload.ActiveProvider != "fast" || payload.EffectiveProvider != "work" || payload.OverriddenByEnv != config.ActiveProviderEnv {
144+
t.Fatalf("JSON must flag the env override, got %#v", payload)
145+
}
146+
}
147+
148+
// No override note when ZERO_PROVIDER is unset or already names the selection.
149+
func TestRunProvidersUseNoWarnWithoutEnvOverride(t *testing.T) {
150+
cases := map[string]func(string) string{
151+
"env unset": func(string) string { return "" },
152+
"env matches fast": func(key string) string {
153+
if key == config.ActiveProviderEnv {
154+
return "fast"
155+
}
156+
return ""
157+
},
158+
}
159+
for name, getenv := range cases {
160+
t.Run(name, func(t *testing.T) {
161+
var stdout, stderr bytes.Buffer
162+
deps := providerSetupDeps(providersUseOverrideConfig(t))
163+
deps.getenv = getenv
164+
if code := runWithDeps([]string{"providers", "use", "fast"}, &stdout, &stderr, deps); code != exitSuccess {
165+
t.Fatalf("exit = %d: %s", code, stderr.String())
166+
}
167+
if stderr.Len() != 0 {
168+
t.Fatalf("expected no override note, got %q", stderr.String())
169+
}
170+
})
171+
}
172+
}
173+
78174
func TestRunProvidersUseSurfacesMalformedConfig(t *testing.T) {
79175
var stdout, stderr bytes.Buffer
80176
configPath := filepath.Join(t.TempDir(), "config.json")

0 commit comments

Comments
 (0)