From 26aec5d194fb9a9f6f0467cc453360cd22d20ae4 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Fri, 17 Jul 2026 14:10:13 +0200 Subject: [PATCH 1/3] fix(providers): stop "provider not found" for env-derived profiles --- internal/cli/provider_onboarding.go | 110 ++++++++++++++++++++++++++++ internal/config/writer.go | 27 +++++++ internal/tui/command_center.go | 8 ++ internal/tui/provider_manager.go | 32 ++++++-- 4 files changed, 170 insertions(+), 7 deletions(-) diff --git a/internal/cli/provider_onboarding.go b/internal/cli/provider_onboarding.go index 0fc4e818f..325e57f64 100644 --- a/internal/cli/provider_onboarding.go +++ b/internal/cli/provider_onboarding.go @@ -53,6 +53,18 @@ func runProvidersUse(args []string, stdout io.Writer, stderr io.Writer, deps app if err != nil { return writeAppError(stderr, err.Error(), exitCrash) } + // SetActiveProvider only ever matches profiles persisted in config.json + // (see config.ProviderPersisted), but a provider can be visible in + // `zero providers list`/the TUI picker purely because Resolve() + // synthesized it in-memory from an ambient env var (e.g. OPENAI_API_KEY) + // without ever writing a row to disk. Without this check, switching to + // that provider by name always fails with a confusing "not found" even + // though it is genuinely usable this session (issue #707). + if !config.ProviderPersisted(configPath, options.name) { + if exit, handled := reportUnpersistedProviderUse(stdout, stderr, deps, options); handled { + return exit + } + } cfg, err := config.SetActiveProvider(configPath, options.name) if err != nil { return writeAppError(stderr, err.Error(), exitCrash) @@ -352,6 +364,18 @@ func runProvidersRemove(args []string, stdout io.Writer, stderr io.Writer, deps return writeAppError(stderr, err.Error(), exitCrash) } name := options.names[0] + // RemoveProvider only ever matches profiles persisted in config.json (see + // config.ProviderPersisted), but a provider can be visible in + // `zero providers list`/the TUI picker purely because Resolve() + // synthesized it in-memory from an ambient env var (e.g. OPENAI_API_KEY) + // without ever writing a row to disk. Without this check, deleting that + // provider by name always fails with a confusing "not found" even though + // it is genuinely visible/usable this session (issue #707). + if !config.ProviderPersisted(configPath, name) { + if exit, handled := reportUnpersistedProviderRemove(stdout, stderr, deps, name, options.json); handled { + return exit + } + } cfg, err := config.RemoveProvider(configPath, name) if err != nil { return writeAppError(stderr, err.Error(), exitCrash) @@ -447,3 +471,89 @@ func runProvidersRename(args []string, stdout io.Writer, stderr io.Writer, deps } return exitSuccess } + +// providerResolvedByName reports whether name matches a provider in a +// resolved provider list — used to tell a genuinely unknown name (a typo) +// apart from a real, env-derived provider that just has no config.json row. +func providerResolvedByName(providers []config.ProviderProfile, name string) bool { + name = strings.TrimSpace(name) + for _, provider := range providers { + if strings.EqualFold(strings.TrimSpace(provider.Name), name) { + return true + } + } + return false +} + +// reportUnpersistedProviderUse handles `zero providers use ` for a +// provider that is not persisted in config.json. If it's not resolvable at +// all (an unknown/misspelled name), it returns handled=false so the caller +// falls through to SetActiveProvider's real "not found" error. If it IS +// resolvable — an env-derived profile (e.g. ambient OPENAI_API_KEY) — +// SetActiveProvider would only ever fail "not found" against it, so this +// reports the situation plainly instead of that confusing error (issue +// #707). +func reportUnpersistedProviderUse(stdout, stderr io.Writer, deps appDeps, options providerUseOptions) (int, bool) { + resolved, exitCode := resolveCommandCenterConfig(stderr, deps) + if exitCode != exitSuccess { + // resolveCommandCenterConfig already wrote its own error to stderr; + // stop here instead of letting the caller try SetActiveProvider too. + return exitCode, true + } + if !providerResolvedByName(resolved.Providers, options.name) { + return exitCode, false + } + message := fmt.Sprintf( + "Provider %q is not saved in config.json (likely set via an environment variable), so there is no saved profile to switch to.\nIt is already used automatically whenever its environment variable is set; unset it to stop Zero from detecting it automatically.", + options.name, + ) + if options.json { + if err := writePrettyJSON(stdout, map[string]any{ + "activeProvider": resolved.ActiveProvider, + "persisted": false, + "message": message, + }); err != nil { + return exitCrash, true + } + return exitSuccess, true + } + if _, err := fmt.Fprintln(stdout, message); err != nil { + return exitCrash, true + } + return exitSuccess, true +} + +// reportUnpersistedProviderRemove handles `zero providers remove ` for +// a provider that is not persisted in config.json, mirroring the TUI +// provider manager's delete handling (internal/tui/provider_manager.go). If +// name isn't resolvable at all, it returns handled=false so the caller falls +// through to RemoveProvider's real "not found" error. +func reportUnpersistedProviderRemove(stdout, stderr io.Writer, deps appDeps, name string, jsonOutput bool) (int, bool) { + resolved, exitCode := resolveCommandCenterConfig(stderr, deps) + if exitCode != exitSuccess { + // resolveCommandCenterConfig already wrote its own error to stderr; + // stop here instead of letting the caller try RemoveProvider too. + return exitCode, true + } + if !providerResolvedByName(resolved.Providers, name) { + return exitCode, false + } + message := fmt.Sprintf( + "Provider %q is not saved in config.json (likely set via an environment variable) — nothing to remove there.\nUnset its environment variable to stop Zero from detecting it automatically.", + name, + ) + if jsonOutput { + if err := writePrettyJSON(stdout, map[string]any{ + "removed": false, + "activeProvider": resolved.ActiveProvider, + "message": message, + }); err != nil { + return exitCrash, true + } + return exitSuccess, true + } + if _, err := fmt.Fprintln(stdout, message); err != nil { + return exitCrash, true + } + return exitSuccess, true +} diff --git a/internal/config/writer.go b/internal/config/writer.go index 34af3027a..1c7259ac6 100644 --- a/internal/config/writer.go +++ b/internal/config/writer.go @@ -177,6 +177,33 @@ func SetActiveProvider(path string, name string) (FileConfig, error) { return FileConfig{}, fmt.Errorf("provider %q not found", name) } +// ProviderPersisted reports whether a provider profile named name actually has +// a row in the config file at path. A provider can appear in the resolved/ +// in-memory provider list without ever being written to config.json — e.g. +// applyProviderEnv synthesizes an "openai" profile purely from an ambient +// OPENAI_API_KEY environment variable on every Resolve() call, without ever +// persisting it. RemoveProvider/SetActiveProvider/SetProviderModel only ever +// look at what's on disk, so a caller offering to mutate a provider by name +// should check this first: "not on disk" needs different handling (nothing to +// persist/remove there) than a name that doesn't exist anywhere at all. +func ProviderPersisted(path string, name string) bool { + path = strings.TrimSpace(path) + name = strings.TrimSpace(name) + if path == "" || name == "" { + return false + } + cfg, err := loadConfigFile(path) + if err != nil { + return false + } + for _, provider := range cfg.Providers { + if strings.EqualFold(strings.TrimSpace(provider.Name), name) { + return true + } + } + return false +} + // RemoveProvider deletes the named provider profile from the config at path. // When the removed profile was active, activeProvider hands off to the first // remaining provider (or clears when none remain) so the config never points at diff --git a/internal/tui/command_center.go b/internal/tui/command_center.go index 2f5175a09..b93d1c5e7 100644 --- a/internal/tui/command_center.go +++ b/internal/tui/command_center.go @@ -682,6 +682,14 @@ func (m model) persistSelectedModel(profile config.ProviderProfile) (bool, error if model == "" { return false, nil } + if !config.ProviderPersisted(path, name) { + // Env-derived providers (e.g. an ambient OPENAI_API_KEY) are synthesized + // in-memory by Resolve and never gain a config.json row, so there's + // nothing on disk to update — SetProviderModel would only ever fail + // with a confusing "not found". Treat it like the no-path/no-name cases + // above: nothing to persist, not an error. + return false, nil + } if _, err := config.SetProviderModel(path, name, model); err != nil { return false, err } diff --git a/internal/tui/provider_manager.go b/internal/tui/provider_manager.go index 966042e79..f4d38c494 100644 --- a/internal/tui/provider_manager.go +++ b/internal/tui/provider_manager.go @@ -347,20 +347,38 @@ func (m model) deleteManagerSelection() (model, tea.Cmd) { wizard.manageStatus = "No user config path — cannot delete." return m, nil } - cfg, err := config.RemoveProvider(m.userConfigPath, name) - if err != nil { - wizard.manageStatus = "Delete failed: " + err.Error() - return m, nil + + var notes []string + var activeAfter string + if config.ProviderPersisted(m.userConfigPath, name) { + cfg, err := config.RemoveProvider(m.userConfigPath, name) + if err != nil { + wizard.manageStatus = "Delete failed: " + err.Error() + return m, nil + } + activeAfter = cfg.ActiveProvider + notes = []string{"Deleted " + name + "."} + } else { + // Env-derived providers (e.g. from an ambient OPENAI_API_KEY) are + // synthesized in-memory by Resolve on every launch and never gain a + // config.json row, so there's nothing on disk to remove — + // RemoveProvider would only ever fail with a confusing "not found" + // even though the provider is genuinely visible/usable this session. + // Drop it from the session's list instead and say why it can return. + notes = []string{ + "Removed " + name + " from this session.", + "It wasn't saved in config.json (likely set via an environment variable) — unset it to stop Zero from detecting it automatically.", + } } + // Surgical removal — see saveManagerEdit for why the raw cfg.Providers list // must not replace the resolved/filtered savedProviders wholesale. m.savedProviders = removeSavedProvider(m.savedProviders, name) - notes := []string{"Deleted " + name + "."} if strings.EqualFold(strings.TrimSpace(m.providerName), strings.TrimSpace(name)) { notes = append(notes, "This session keeps running on it until you switch.") - } else if active := strings.TrimSpace(cfg.ActiveProvider); active != "" && !strings.EqualFold(active, name) { - notes = append(notes, "Active provider: "+active+".") + } else if activeAfter != "" && !strings.EqualFold(activeAfter, name) { + notes = append(notes, "Active provider: "+activeAfter+".") } cleanup := providerManagerCleanupCmd(m.userConfigPath, row.profile) From c932d59d096062c993d92ca349a4879ab711c06a Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Fri, 17 Jul 2026 17:53:46 +0200 Subject: [PATCH 2/3] fix(providers): address env profile review feedback --- internal/cli/provider_onboarding.go | 64 +++++++++++++++++++++--- internal/cli/provider_onboarding_test.go | 58 +++++++++++++++++++++ internal/config/writer.go | 10 ++-- internal/tui/command_center.go | 12 ++--- internal/tui/provider_manager.go | 27 +++++++--- 5 files changed, 144 insertions(+), 27 deletions(-) diff --git a/internal/cli/provider_onboarding.go b/internal/cli/provider_onboarding.go index 325e57f64..bdfb262ff 100644 --- a/internal/cli/provider_onboarding.go +++ b/internal/cli/provider_onboarding.go @@ -60,7 +60,11 @@ func runProvidersUse(args []string, stdout io.Writer, stderr io.Writer, deps app // without ever writing a row to disk. Without this check, switching to // that provider by name always fails with a confusing "not found" even // though it is genuinely usable this session (issue #707). - if !config.ProviderPersisted(configPath, options.name) { + persisted, err := config.ProviderPersisted(configPath, options.name) + if err != nil { + return writeAppError(stderr, err.Error(), exitCrash) + } + if !persisted { if exit, handled := reportUnpersistedProviderUse(stdout, stderr, deps, options); handled { return exit } @@ -371,8 +375,12 @@ func runProvidersRemove(args []string, stdout io.Writer, stderr io.Writer, deps // without ever writing a row to disk. Without this check, deleting that // provider by name always fails with a confusing "not found" even though // it is genuinely visible/usable this session (issue #707). - if !config.ProviderPersisted(configPath, name) { - if exit, handled := reportUnpersistedProviderRemove(stdout, stderr, deps, name, options.json); handled { + persisted, err := config.ProviderPersisted(configPath, name) + if err != nil { + return writeAppError(stderr, err.Error(), exitCrash) + } + if !persisted { + if exit, handled := reportUnpersistedProviderRemove(stdout, stderr, deps, name, options.json, configPath); handled { return exit } } @@ -452,6 +460,16 @@ func runProvidersRename(args []string, stdout io.Writer, stderr io.Writer, deps if err != nil { return writeAppError(stderr, err.Error(), exitCrash) } + oldName := options.names[0] + persisted, err := config.ProviderPersisted(configPath, oldName) + if err != nil { + return writeAppError(stderr, err.Error(), exitCrash) + } + if !persisted { + if exit, handled := reportUnpersistedProviderRename(stdout, stderr, deps, oldName, options.json, configPath); handled { + return exit + } + } cfg, err := config.RenameProvider(configPath, options.names[0], options.names[1]) if err != nil { return writeAppError(stderr, err.Error(), exitCrash) @@ -504,7 +522,7 @@ func reportUnpersistedProviderUse(stdout, stderr io.Writer, deps appDeps, option return exitCode, false } message := fmt.Sprintf( - "Provider %q is not saved in config.json (likely set via an environment variable), so there is no saved profile to switch to.\nIt is already used automatically whenever its environment variable is set; unset it to stop Zero from detecting it automatically.", + "Provider %q is not saved in config.json (likely set via an environment variable), so there is no saved profile to switch to.\nIt is available whenever its environment variable is set, but is only active when selected (for example via ZERO_PROVIDER); unset its environment variable to stop Zero from detecting it automatically.", options.name, ) if options.json { @@ -528,11 +546,9 @@ func reportUnpersistedProviderUse(stdout, stderr io.Writer, deps appDeps, option // provider manager's delete handling (internal/tui/provider_manager.go). If // name isn't resolvable at all, it returns handled=false so the caller falls // through to RemoveProvider's real "not found" error. -func reportUnpersistedProviderRemove(stdout, stderr io.Writer, deps appDeps, name string, jsonOutput bool) (int, bool) { +func reportUnpersistedProviderRemove(stdout, stderr io.Writer, deps appDeps, name string, jsonOutput bool, configPath string) (int, bool) { resolved, exitCode := resolveCommandCenterConfig(stderr, deps) if exitCode != exitSuccess { - // resolveCommandCenterConfig already wrote its own error to stderr; - // stop here instead of letting the caller try RemoveProvider too. return exitCode, true } if !providerResolvedByName(resolved.Providers, name) { @@ -544,8 +560,11 @@ func reportUnpersistedProviderRemove(stdout, stderr io.Writer, deps appDeps, nam ) if jsonOutput { if err := writePrettyJSON(stdout, map[string]any{ - "removed": false, + "removed": "", + "keyRemoved": false, "activeProvider": resolved.ActiveProvider, + "configPath": configPath, + "persisted": false, "message": message, }); err != nil { return exitCrash, true @@ -557,3 +576,32 @@ func reportUnpersistedProviderRemove(stdout, stderr io.Writer, deps appDeps, nam } return exitSuccess, true } + +func reportUnpersistedProviderRename(stdout, stderr io.Writer, deps appDeps, name string, jsonOutput bool, configPath string) (int, bool) { + resolved, exitCode := resolveCommandCenterConfig(stderr, deps) + if exitCode != exitSuccess { + return exitCode, true + } + if !providerResolvedByName(resolved.Providers, name) { + return exitCode, false + } + message := fmt.Sprintf( + "Provider %q is not saved in config.json (likely set via an environment variable), so there is no saved profile to rename.", + name, + ) + if jsonOutput { + if err := writePrettyJSON(stdout, map[string]any{ + "renamed": false, + "configPath": configPath, + "persisted": false, + "message": message, + }); err != nil { + return exitCrash, true + } + return exitSuccess, true + } + if _, err := fmt.Fprintln(stdout, message); err != nil { + return exitCrash, true + } + return exitSuccess, true +} diff --git a/internal/cli/provider_onboarding_test.go b/internal/cli/provider_onboarding_test.go index 9493e81ce..8fb629ef1 100644 --- a/internal/cli/provider_onboarding_test.go +++ b/internal/cli/provider_onboarding_test.go @@ -75,6 +75,64 @@ func TestRunProvidersUseJSONIncludesActiveProviderAndConfigPath(t *testing.T) { } } +func TestRunProvidersUseSurfacesMalformedConfig(t *testing.T) { + var stdout, stderr bytes.Buffer + configPath := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(configPath, []byte("{"), 0o600); err != nil { + t.Fatalf("write malformed config: %v", err) + } + + exitCode := runWithDeps([]string{"providers", "use", "openai"}, &stdout, &stderr, providerSetupDeps(configPath)) + + if exitCode != exitCrash { + t.Fatalf("exit code = %d, want %d", exitCode, exitCrash) + } + if !strings.Contains(stderr.String(), "invalid config JSON") { + t.Fatalf("stderr = %q, want malformed-config error", stderr.String()) + } +} + +func TestRunProvidersRemoveEnvDerivedJSONKeepsSchema(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "sk-env") + var stdout, stderr bytes.Buffer + configPath := filepath.Join(t.TempDir(), "config.json") + writeProviderOnboardingConfig(t, configPath, config.FileConfig{}) + + exitCode := runWithDeps([]string{"providers", "remove", "openai", "--json"}, &stdout, &stderr, providerSetupDeps(configPath)) + + if exitCode != exitSuccess { + t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + var payload struct { + Removed string `json:"removed"` + KeyRemoved bool `json:"keyRemoved"` + Persisted bool `json:"persisted"` + ConfigPath string `json:"configPath"` + } + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("decode JSON: %v\n%s", err, stdout.String()) + } + if payload.Removed != "" || payload.KeyRemoved || payload.Persisted || payload.ConfigPath != configPath { + t.Fatalf("unexpected payload: %+v", payload) + } +} + +func TestRunProvidersRenameEnvDerivedExplainsNoSavedProfile(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "sk-env") + var stdout, stderr bytes.Buffer + configPath := filepath.Join(t.TempDir(), "config.json") + writeProviderOnboardingConfig(t, configPath, config.FileConfig{}) + + exitCode := runWithDeps([]string{"providers", "rename", "openai", "renamed"}, &stdout, &stderr, providerSetupDeps(configPath)) + + if exitCode != exitSuccess { + t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + if !strings.Contains(stdout.String(), "no saved profile to rename") { + t.Fatalf("stdout = %q, want unpersisted explanation", stdout.String()) + } +} + func TestRunProvidersUseRejectsUsageErrors(t *testing.T) { cases := []struct { name string diff --git a/internal/config/writer.go b/internal/config/writer.go index 1c7259ac6..38f395d60 100644 --- a/internal/config/writer.go +++ b/internal/config/writer.go @@ -186,22 +186,22 @@ func SetActiveProvider(path string, name string) (FileConfig, error) { // look at what's on disk, so a caller offering to mutate a provider by name // should check this first: "not on disk" needs different handling (nothing to // persist/remove there) than a name that doesn't exist anywhere at all. -func ProviderPersisted(path string, name string) bool { +func ProviderPersisted(path string, name string) (bool, error) { path = strings.TrimSpace(path) name = strings.TrimSpace(name) if path == "" || name == "" { - return false + return false, nil } cfg, err := loadConfigFile(path) if err != nil { - return false + return false, err } for _, provider := range cfg.Providers { if strings.EqualFold(strings.TrimSpace(provider.Name), name) { - return true + return true, nil } } - return false + return false, nil } // RemoveProvider deletes the named provider profile from the config at path. diff --git a/internal/tui/command_center.go b/internal/tui/command_center.go index b93d1c5e7..0856e2994 100644 --- a/internal/tui/command_center.go +++ b/internal/tui/command_center.go @@ -682,12 +682,12 @@ func (m model) persistSelectedModel(profile config.ProviderProfile) (bool, error if model == "" { return false, nil } - if !config.ProviderPersisted(path, name) { - // Env-derived providers (e.g. an ambient OPENAI_API_KEY) are synthesized - // in-memory by Resolve and never gain a config.json row, so there's - // nothing on disk to update — SetProviderModel would only ever fail - // with a confusing "not found". Treat it like the no-path/no-name cases - // above: nothing to persist, not an error. + persisted, err := config.ProviderPersisted(path, name) + if err != nil { + return false, err + } + if !persisted { + // Env-derived providers have no config.json row to update. return false, nil } if _, err := config.SetProviderModel(path, name, model); err != nil { diff --git a/internal/tui/provider_manager.go b/internal/tui/provider_manager.go index f4d38c494..6816074d7 100644 --- a/internal/tui/provider_manager.go +++ b/internal/tui/provider_manager.go @@ -348,9 +348,15 @@ func (m model) deleteManagerSelection() (model, tea.Cmd) { return m, nil } + persisted, err := config.ProviderPersisted(m.userConfigPath, name) + if err != nil { + wizard.manageStatus = "Delete failed: " + err.Error() + return m, nil + } var notes []string var activeAfter string - if config.ProviderPersisted(m.userConfigPath, name) { + var cleanup tea.Cmd + if persisted { cfg, err := config.RemoveProvider(m.userConfigPath, name) if err != nil { wizard.manageStatus = "Delete failed: " + err.Error() @@ -358,13 +364,10 @@ func (m model) deleteManagerSelection() (model, tea.Cmd) { } activeAfter = cfg.ActiveProvider notes = []string{"Deleted " + name + "."} + cleanup = providerManagerCleanupCmd(m.userConfigPath, row.profile) } else { - // Env-derived providers (e.g. from an ambient OPENAI_API_KEY) are - // synthesized in-memory by Resolve on every launch and never gain a - // config.json row, so there's nothing on disk to remove — - // RemoveProvider would only ever fail with a confusing "not found" - // even though the provider is genuinely visible/usable this session. - // Drop it from the session's list instead and say why it can return. + // Env-derived providers have no persisted profile or credential to + // delete. Keep this path session-only. notes = []string{ "Removed " + name + " from this session.", "It wasn't saved in config.json (likely set via an environment variable) — unset it to stop Zero from detecting it automatically.", @@ -380,7 +383,6 @@ func (m model) deleteManagerSelection() (model, tea.Cmd) { } else if activeAfter != "" && !strings.EqualFold(activeAfter, name) { notes = append(notes, "Active provider: "+activeAfter+".") } - cleanup := providerManagerCleanupCmd(m.userConfigPath, row.profile) if len(m.savedProviders) == 0 { m.providerWizard = nil @@ -588,6 +590,15 @@ func (m model) saveManagerEdit() (model, tea.Cmd) { return m, nil } oldName := strings.TrimSpace(wizard.editOriginal.Name) + persisted, err := config.ProviderPersisted(m.userConfigPath, oldName) + if err != nil { + wizard.err = err.Error() + return m, nil + } + if !persisted { + wizard.err = "provider " + oldName + " is not saved in config.json, so there is no saved profile to edit" + return m, nil + } newName := strings.TrimSpace(wizard.editDraft.Name) if newName == "" { wizard.err = "name cannot be empty" From 807c82689e4458789acf7ef945a58604f916502d Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sat, 18 Jul 2026 14:01:23 +0200 Subject: [PATCH 3/3] fix(providers): keep env JSON response schemas stable --- internal/cli/provider_onboarding.go | 7 ++-- internal/cli/provider_onboarding_test.go | 47 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/internal/cli/provider_onboarding.go b/internal/cli/provider_onboarding.go index bdfb262ff..b581cc8cd 100644 --- a/internal/cli/provider_onboarding.go +++ b/internal/cli/provider_onboarding.go @@ -65,7 +65,7 @@ func runProvidersUse(args []string, stdout io.Writer, stderr io.Writer, deps app return writeAppError(stderr, err.Error(), exitCrash) } if !persisted { - if exit, handled := reportUnpersistedProviderUse(stdout, stderr, deps, options); handled { + if exit, handled := reportUnpersistedProviderUse(stdout, stderr, deps, options, configPath); handled { return exit } } @@ -511,7 +511,7 @@ func providerResolvedByName(providers []config.ProviderProfile, name string) boo // SetActiveProvider would only ever fail "not found" against it, so this // reports the situation plainly instead of that confusing error (issue // #707). -func reportUnpersistedProviderUse(stdout, stderr io.Writer, deps appDeps, options providerUseOptions) (int, bool) { +func reportUnpersistedProviderUse(stdout, stderr io.Writer, deps appDeps, options providerUseOptions, configPath string) (int, bool) { resolved, exitCode := resolveCommandCenterConfig(stderr, deps) if exitCode != exitSuccess { // resolveCommandCenterConfig already wrote its own error to stderr; @@ -528,6 +528,7 @@ func reportUnpersistedProviderUse(stdout, stderr io.Writer, deps appDeps, option if options.json { if err := writePrettyJSON(stdout, map[string]any{ "activeProvider": resolved.ActiveProvider, + "configPath": configPath, "persisted": false, "message": message, }); err != nil { @@ -591,7 +592,7 @@ func reportUnpersistedProviderRename(stdout, stderr io.Writer, deps appDeps, nam ) if jsonOutput { if err := writePrettyJSON(stdout, map[string]any{ - "renamed": false, + "renamed": nil, "configPath": configPath, "persisted": false, "message": message, diff --git a/internal/cli/provider_onboarding_test.go b/internal/cli/provider_onboarding_test.go index 8fb629ef1..d354ced85 100644 --- a/internal/cli/provider_onboarding_test.go +++ b/internal/cli/provider_onboarding_test.go @@ -92,6 +92,29 @@ func TestRunProvidersUseSurfacesMalformedConfig(t *testing.T) { } } +func TestRunProvidersUseEnvDerivedJSONIncludesConfigPath(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "sk-env") + var stdout, stderr bytes.Buffer + configPath := filepath.Join(t.TempDir(), "config.json") + writeProviderOnboardingConfig(t, configPath, config.FileConfig{}) + + exitCode := runWithDeps([]string{"providers", "use", "openai", "--json"}, &stdout, &stderr, providerSetupDeps(configPath)) + + if exitCode != exitSuccess { + t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + var payload struct { + ConfigPath string `json:"configPath"` + Persisted bool `json:"persisted"` + } + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("decode JSON: %v\n%s", err, stdout.String()) + } + if payload.ConfigPath != configPath || payload.Persisted { + t.Fatalf("unexpected payload: %+v", payload) + } +} + func TestRunProvidersRemoveEnvDerivedJSONKeepsSchema(t *testing.T) { t.Setenv("OPENAI_API_KEY", "sk-env") var stdout, stderr bytes.Buffer @@ -133,6 +156,30 @@ func TestRunProvidersRenameEnvDerivedExplainsNoSavedProfile(t *testing.T) { } } +func TestRunProvidersRenameEnvDerivedJSONKeepsSchema(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "sk-env") + var stdout, stderr bytes.Buffer + configPath := filepath.Join(t.TempDir(), "config.json") + writeProviderOnboardingConfig(t, configPath, config.FileConfig{}) + + exitCode := runWithDeps([]string{"providers", "rename", "openai", "renamed", "--json"}, &stdout, &stderr, providerSetupDeps(configPath)) + + if exitCode != exitSuccess { + t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + var payload struct { + Renamed *struct{} `json:"renamed"` + ConfigPath string `json:"configPath"` + Persisted bool `json:"persisted"` + } + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("decode JSON: %v\n%s", err, stdout.String()) + } + if payload.Renamed != nil || payload.ConfigPath != configPath || payload.Persisted { + t.Fatalf("unexpected payload: %+v", payload) + } +} + func TestRunProvidersUseRejectsUsageErrors(t *testing.T) { cases := []struct { name string