Clarify saved and effective provider selection

internal/cli/app.go

@@ -22,6 +22,7 @@ import (
 	"github.com/Gitlawb/zero/internal/localcontrol"
 	"github.com/Gitlawb/zero/internal/mcp"
 	"github.com/Gitlawb/zero/internal/modelregistry"
+	"github.com/Gitlawb/zero/internal/oauth"
 	"github.com/Gitlawb/zero/internal/observability"
 	"github.com/Gitlawb/zero/internal/plugins"
 	"github.com/Gitlawb/zero/internal/providerhealth"
@@ -68,6 +69,7 @@ type appDeps struct {
 	discoverProviderModels func(context.Context, config.ProviderProfile) ([]providermodeldiscovery.Model, error)
 	detectLocalRuntimes    func(context.Context, provideronboarding.LocalDetectOptions) []provideronboarding.DetectedLocalRuntime
 	openRouterLogin        func(context.Context, provideroauth.OpenRouterOptions) (string, error)
+	chatGPTLogin           func(context.Context, provideroauth.ChatGPTOptions) (oauth.Token, error)
 	newSessionStore        func() *sessions.Store
 	loadPlugins            func(plugins.LoadOptions) (plugins.LoadResult, error)
 	loadHooks              func(hooks.LoadOptions) (hooks.LoadResult, error)
@@ -155,6 +157,7 @@ func defaultAppDeps() appDeps {
 		discoverProviderModels: defaultDiscoverProviderModels,
 		detectLocalRuntimes:    provideronboarding.DetectLocalRuntimes,
 		openRouterLogin:        provideroauth.OpenRouterLogin,
+		chatGPTLogin:           provideroauth.ChatGPTLogin,
 		newSessionStore: func() *sessions.Store {
 			return sessions.NewStore(sessions.StoreOptions{})
 		},
@@ -493,6 +496,9 @@ func fillAppDeps(deps appDeps) appDeps {
 	if deps.openRouterLogin == nil {
 		deps.openRouterLogin = defaults.openRouterLogin
 	}
+	if deps.chatGPTLogin == nil {
+		deps.chatGPTLogin = defaults.chatGPTLogin
+	}
 	if deps.newSessionStore == nil {
 		deps.newSessionStore = defaults.newSessionStore
 	}

internal/cli/auth.go

@@ -99,6 +99,16 @@ func runAuthOpenRouter(args []string, stdout io.Writer, stderr io.Writer, deps a
 	if len(args) > 0 {
 		return writeExecUsageError(stderr, fmt.Sprintf("zero auth openrouter takes no arguments (got %q)", args[0]))
 	}
+	// Check the config BEFORE the browser flow, as every other auth entry point
+	// does. Finding out the file is unusable only after the user has completed a
+	// PKCE round trip wastes their work and mints a key Zero then cannot store.
+	configPath, err := deps.userConfigPath()
+	if err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
+	if err := config.PreflightCatalogProviderLogin(configPath, "openrouter"); err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
 	key, err := deps.openRouterLogin(context.Background(), provideroauth.OpenRouterOptions{
 		Out:        stdout,
 		HTTPClient: &http.Client{Timeout: 30 * time.Second},
@@ -111,10 +121,14 @@ func runAuthOpenRouter(args []string, stdout io.Writer, stderr io.Writer, deps a
 	key = strings.TrimSpace(key)
 	line, err := saveOpenRouterProviderKey(deps, key)
 	if err != nil {
-		if _, writeErr := fmt.Fprintf(stdout, "\nOpenRouter login complete — new API key minted, but Zero could not save it: %s\nUse it manually, e.g.:\n  export OPENROUTER_API_KEY=%s\n", err, key); writeErr != nil {
+		// The key is real and the user paid for it with a browser round trip, so
+		// print it rather than losing it. But nothing was persisted, so this is a
+		// failure: exiting 0 here reported success for a command that left the
+		// provider unusable, and a script would have carried on.
+		if _, writeErr := fmt.Fprintf(stdout, "\nOpenRouter login complete — new API key minted, but Zero could not save it.\nUse it manually, e.g.:\n  export OPENROUTER_API_KEY=%s\n", key); writeErr != nil {
 			return exitCrash
 		}
-		return exitSuccess
+		return writeAppError(stderr, "could not save the OpenRouter API key: "+err.Error(), exitCrash)
 	}
 	if _, err := fmt.Fprintf(stdout, "\nOpenRouter login complete — new API key saved.\n%s\n", line); err != nil {
 		return exitCrash
@@ -131,6 +145,9 @@ func saveOpenRouterProviderKey(deps appDeps, key string) (string, error) {
 	if err != nil {
 		return "", err
 	}
+	if err := config.PreflightUserConfig(configPath); err != nil {
+		return "", err
+	}
 	ensured, err := config.EnsureCatalogProvider(configPath, "openrouter")
 	if err != nil {
 		return "", err
@@ -176,6 +193,14 @@ func runAuthChatGPT(args []string, stdout io.Writer, stderr io.Writer, deps appD
 	if len(args) > 0 {
 		return writeExecUsageError(stderr, fmt.Sprintf("zero auth chatgpt takes no arguments (got %q)", args[0]))
 	}
+	configPath, err := deps.userConfigPath()
+	if err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
+	const provider = "chatgpt"
+	if err := config.PreflightCatalogProviderLogin(configPath, provider); err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
 
 	// Build the same env map the oauth engine reads so the chatgpt preset is
 	// opted into (the preset is off by default to keep third-party OAuth
@@ -189,7 +214,7 @@ func runAuthChatGPT(args []string, stdout io.Writer, stderr io.Writer, deps appD
 	}
 	env["ZERO_OAUTH_ALLOW_PRESETS"] = "1"
 
-	token, err := provideroauth.ChatGPTLogin(context.Background(), provideroauth.ChatGPTOptions{
+	token, err := deps.chatGPTLogin(context.Background(), provideroauth.ChatGPTOptions{
 		Env:        env,
 		HTTPClient: &http.Client{Timeout: 60 * time.Second},
 		Out:        stdout,
@@ -212,7 +237,10 @@ func runAuthChatGPT(args []string, stdout io.Writer, stderr io.Writer, deps appD
 	if err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
 	}
-	if err := store.Save(oauth.ProviderKey("chatgpt"), token); err != nil {
+	if err := config.PreflightCatalogProviderLogin(configPath, provider); err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
+	if err := store.Save(oauth.ProviderKey(provider), token); err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
 	}
 	statuses, err := oauthFormatChatGPTStatus(token)
@@ -343,7 +371,7 @@ func validateAuthFlags(sub string, a authArgs) error {
 // ZERO_OAUTH_TOKENS_PATH (env), so callers/tests can redirect it. Setting
 // ZERO_OAUTH_STORAGE=encrypted-file selects the AES-256-GCM encrypted-at-rest
 // backend (a per-user secret is created beside the token file).
-func newAuthManager(deps appDeps, out io.Writer) (*oauth.Manager, error) {
+func newAuthManager(deps appDeps, out io.Writer, beforeSave func() error) (*oauth.Manager, error) {
 	// Validate ZERO_OAUTH_STORAGE up front: a mistyped value must fail fast rather
 	// than silently change the backend. Empty = default (plaintext 0600 file);
 	// "encrypted-file" = AES-256-GCM; "keyring" = the OS keyring.
@@ -372,6 +400,7 @@ func newAuthManager(deps appDeps, out io.Writer) (*oauth.Manager, error) {
 		// `zero auth login <preset>` (e.g. xai) should resolve the baked-in preset
 		// without the operator exporting ZERO_OAUTH_ALLOW_PRESETS first.
 		AllowPresets: true,
+		BeforeSave:   beforeSave,
 	})
 }
 
@@ -387,7 +416,7 @@ func runAuthLogin(args []string, stdout io.Writer, stderr io.Writer, deps appDep
 	if len(parsed.positional) != 1 {
 		return writeExecUsageError(stderr, "usage: zero auth login <provider> [--device] [--scope <scope>]")
 	}
-	provider := parsed.positional[0]
+	provider := strings.ToLower(strings.TrimSpace(parsed.positional[0]))
 	// ChatGPT (Codex) requires a fixed redirect_uri (http://localhost:1455/
 	// auth/callback) and mandatory authorize params (id_token_add_organizations,
 	// codex_cli_simplified_flow, originator) that the generic loopback flow
@@ -402,7 +431,16 @@ func runAuthLogin(args []string, stdout io.Writer, stderr io.Writer, deps appDep
 		}
 		return runAuthChatGPT(nil, stdout, stderr, deps)
 	}
-	manager, err := newAuthManager(deps, stdout)
+	configPath, err := deps.userConfigPath()
+	if err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
+	if err := config.PreflightCatalogProviderLogin(configPath, provider); err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
+	manager, err := newAuthManager(deps, stdout, func() error {
+		return config.PreflightCatalogProviderLogin(configPath, provider)
+	})
 	if err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
 	}
@@ -438,7 +476,27 @@ func runAuthLogout(args []string, stdout io.Writer, stderr io.Writer, deps appDe
 		return writeExecUsageError(stderr, "usage: zero auth logout <provider>")
 	}
 	provider := parsed.positional[0]
-	manager, err := newAuthManager(deps, stdout)
+	configPath, err := deps.userConfigPath()
+	if err != nil {
+		return writeAppError(stderr, err.Error(), exitCrash)
+	}
+	configErr := config.PreflightUserConfig(configPath)
+	// Resolve the target the way login does. Login accepts a catalog id or a case
+	// variant and stores its token under the catalog id, and the TUI tells users
+	// to run `zero auth logout <catalog id>` — so refusing that spelling here left
+	// the OAuth token and any stored API key in place for exactly the command the
+	// UI documents. The credential store keys stay on what the user typed (that is
+	// where login put them); only the config mutation below needs the persisted
+	// spelling, because those mutators match a row exactly.
+	configProvider := provider
+	if configErr == nil {
+		if canonical, owned, identityErr := config.PersistedProviderIdentity(configPath, provider); identityErr != nil {
+			configErr = identityErr
+		} else if owned {
+			configProvider = canonical
+		}
+	}
+	manager, err := newAuthManager(deps, stdout, nil)
 	if err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
 	}
@@ -449,14 +507,27 @@ func runAuthLogout(args []string, stdout io.Writer, stderr io.Writer, deps appDe
 	// Also drop any stored API key and its marker so `auth logout` clears the whole
 	// credential (OAuth token AND key), not just the OAuth side. Surface deletion
 	// failures rather than reporting success while a credential remains.
+	//
+	// The key store is asked for both spellings when they differ: a key captured
+	// by provider setup lives under the persisted row's name, while one captured
+	// by a catalog login lives under the catalog id. Clearing only the spelling
+	// the user typed would leave the other behind.
 	keyRemoved, keyErr := config.ForgetProviderKey(provider)
 	if keyErr != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(keyErr, redaction.Options{}), exitCrash)
 	}
-	if configPath, perr := deps.userConfigPath(); perr == nil {
-		if _, clearErr := config.ClearProviderKeyStored(configPath, provider); clearErr != nil {
-			return writeAppError(stderr, redaction.ErrorMessage(clearErr, redaction.Options{}), exitCrash)
+	if configProvider != provider {
+		canonicalRemoved, canonicalErr := config.ForgetProviderKey(configProvider)
+		if canonicalErr != nil {
+			return writeAppError(stderr, redaction.ErrorMessage(canonicalErr, redaction.Options{}), exitCrash)
 		}
+		keyRemoved = keyRemoved || canonicalRemoved
+	}
+	if configErr != nil {
+		return writeAppError(stderr, redaction.ErrorMessage(configErr, redaction.Options{}), exitCrash)
+	}
+	if _, clearErr := config.ClearProviderKeyStored(configPath, configProvider); clearErr != nil {
+		return writeAppError(stderr, redaction.ErrorMessage(clearErr, redaction.Options{}), exitCrash)
 	}
 	removed = removed || keyRemoved
 	if parsed.json {
@@ -491,7 +562,7 @@ func runAuthStatus(args []string, stdout io.Writer, stderr io.Writer, deps appDe
 	if len(parsed.positional) > 1 {
 		return writeExecUsageError(stderr, "usage: zero auth status [provider]")
 	}
-	manager, err := newAuthManager(deps, stdout)
+	manager, err := newAuthManager(deps, stdout, nil)
 	if err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
 	}
@@ -530,7 +601,7 @@ func runAuthRefresh(args []string, stdout io.Writer, stderr io.Writer, deps appD
 		return writeExecUsageError(stderr, "usage: zero auth refresh <provider>")
 	}
 	provider := parsed.positional[0]
-	manager, err := newAuthManager(deps, stdout)
+	manager, err := newAuthManager(deps, stdout, nil)
 	if err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
 	}