Files
zero/internal/cli/setup_fallback_test.go
Kevin Codex 16550569c1 feat(providers): OAuth login profiles and list-first /provider manager (#560)
* feat(providers): OAuth login profiles and list-first /provider manager

Make multiple providers a first-class workflow: every OAuth login is now a
visible, switchable provider, and /provider manages the whole lifecycle.

OAuth logins become provider profiles:
- zero auth chatgpt / zero auth login <preset> and all four TUI login paths
  (wizard + first-run, browser + device-code) now upsert a catalog-backed
  profile via config.EnsureCatalogProvider after storing the token — without
  stealing the active provider. Previously the token was stored invisibly:
  the provider showed up nowhere and 'providers use' failed not-found.
- Switching to an OAuth provider no longer inlines the bearer as an API key,
  which suppressed the runtime resolver (no token refresh, no Codex
  chatgpt-account-id header → guaranteed 401s). The credential gate now
  recognizes stored OAuth logins on keyless profiles.
- providers list / current report truthful credential state: stored keys
  count as configured (APIKeySet honors apiKeyStored) and token logins show
  'oauth login'; providers check accepts an OAuth login as a credential.

List-first /provider manager (TUI):
- /provider opens the saved-provider list: Enter activates via the shared
  switch path, 'a' adds (existing wizard), 'e' opens a field-level editor
  (endpoint, model, key, description, and safe rename — the stored key,
  activeProvider, and live ZERO_PROVIDER follow the new name), 'd' deletes
  with an inline confirm and stored-key cleanup. Esc walks back one level.
- Credential states resolve asynchronously so keychain reads never block
  the render loop; 'stored key missing' surfaces broken keychain entries.
- Implemented as manager steps on providerWizardState so every existing
  overlay gate, key route, and mouse hook covers it unchanged.

CLI + config parity:
- zero providers remove <name> (deletes the stored key, hands the active
  pointer off) and zero providers rename <old> <new>.
- config.RemoveProvider / RenameProvider: read-modify-atomic-write; rename
  migrates the credential-store entry write-new-then-delete-old so an
  interruption can never lose the key.

* fix(providers): address PR #560 review findings

All six review findings validated as real; fixes with regression tests:

- Persist the apiKeyStored marker when a key is captured onto an existing
  profile: mergeProfile deliberately ignores the marker (a project config
  must not claim the user's stored keys at resolve time), so the user-config
  WRITE path (UpsertProvider) now re-applies it — previously editing a key
  onto an env/no-key profile stranded the secret in the credential store
  while every ApplyStoredAPIKey gate skipped it. [jatmn P2]
- Let active-pointer recovery select OAuth-login providers:
  firstUsableProvider now applies the same providerHasOAuthLogin check as
  setupRequired/usableSavedProviders, so a stale activeProvider plus a saved
  ChatGPT/xAI login recovers instead of forcing onboarding. [jatmn P2]
- Delete stored keys from the store BESIDE the config being edited:
  providers remove and the TUI manager delete now use ProviderKeyStoreAt
  (matching setup/rename) instead of the default-path store. [jatmn P2]
- Roll back the key migration when RenameProvider's config write fails, so
  the store and config.json can't desync (key under the new name, config
  still naming the old one). [CodeRabbit]
- Clear provider descriptions for real: new SetProviderDescription writes
  the field verbatim (the upsert merge treats empty as unchanged); the
  manager's editor routes description changes through it. [jatmn P3]
- Surface stored-key deletion failures in providers remove (stderr warning
  + keyError in the JSON payload) and the TUI delete status, instead of
  letting a lingering secret read as a clean removal. [CodeRabbit]

* fix(providers): second review round — case-only rename, logout hint, test hermeticity

- Skip the credential-store migration on case-only renames (groq -> Groq):
  the store normalizes names case-insensitively, so Set(new)+Delete(old)
  target ONE entry and the delete removed the key that was just "moved".
  The existing entry already serves the new name. [CodeRabbit]
- Point the delete flow's cleanup hint at the stored login that actually
  exists: after a rename ({name:"codex", catalogID:"chatgpt"}) the token
  lives under the catalog id, and `zero auth logout codex` would delete
  nothing. oauthLoginName resolves the same FirstStored candidate the
  runtime uses. [jatmn P3]
- Lock in APIKeyEnv semantics after a key edit: the env reference survives
  as an explicit override (a SET env var wins at resolve time; the stored
  key applies when it is absent). [CodeRabbit nitpick]
- Make main's new firstUsableProvider tests hermetic: they read the REAL
  OAuth token store, so any developer with an actual xai/chatgpt login gets
  spurious failures (found because this machine has one).

Also rebased onto main; the only conflict was comment wording in
firstUsableProvider, where the audit commit (#481) independently landed the
same OAuth-login fallback check this branch added.

* fix(tui): read stored keys from the active config's credential store

The TUI's write paths (SecureProviderProfile capture, rename migration,
manager delete) all target the store beside m.userConfigPath, but the read
paths still opened the DEFAULT-path store: profileWithCredential (feeding
the /model and manager switch gate), the model picker's discovery
credential lookup, and the manager's async credential-status probe. With a
non-default config path, a provider saved with only apiKeyStored had its
key in the config-adjacent store, so the gate rejected it as having no
usable credential and the manager showed 'stored key missing' for a
healthy profile.

All TUI stored-key reads now resolve through providerKeyStoreForPath
(config-adjacent, default-path fallback for ephemeral sessions), so the UI
reads from the same store it writes to. Regression test covers both the
credential probe and the switch path with a key seeded only beside the
config.

Also consolidates the OAuth test-store seeding on withAuthStore (now pins
ZERO_OAUTH_STORAGE=file so an inherited keyring backend can't bypass the
temp path) per review nitpick.

* fix(providers): resolve self-review findings — atomic edits, key normalization, threading

Findings from a multi-agent code review of this branch (8 finder angles,
independently verified); all confirmed against the code, several with
empirical repros.

Correctness:
- config.EditProvider: one atomic read-modify-write for the manager's Save
  (rename + fields + verbatim description + stored-key marker + key
  migration with write-failure rollback). Replaces the rename→upsert→
  describe sequence that (a) turned a case-only rename (groq → Groq) into a
  DUPLICATE profile via UpsertProvider's case-sensitive merge, and (b) could
  fail halfway, leaving config.json renamed while savedProviders and
  ZERO_PROVIDER still held the old name (retry then failed 'not found').
  A rename now also migrates a freshly captured replacement key (captured
  under the current name; the migration moves it).
- savedProviders is mutated surgically on manager delete/edit instead of
  being replaced with the raw user-config list — the session list is seeded
  from the RESOLVED (project-config layered) and usability-FILTERED set, so
  wholesale replacement dropped project-contributed providers and
  resurrected filtered stubs for the rest of the session.
- oauth.ProviderKey normalizes provider names to lower case at the single
  write/lookup choke point: `zero auth login xAI` stored provider:xAI while
  every candidate lookup was case-sensitive — a successful login invisible
  to the runtime. Map-based presence checks lowercase to match.
- OAuth login persistence moved onto the Update goroutine (message handlers)
  — a background tea.Cmd (device-code polls land minutes later) doing an
  unlocked config read-modify-write raced UI-thread writes (lost updates).
  The write error is now surfaced and the in-memory mirror is skipped on
  failure, so a profile that never reached disk can't show as a working
  provider that vanishes on restart.

UX / consistency:
- Esc inside a manager-launched add flow walks back to the provider list
  from EVERY step (was: first step only; deeper steps destroyed the overlay
  and the user's place). In-flight OAuth logins are invalidated on the way
  out; footers say 'Esc back'.
- switchProviderModel returns a structured outcome; the manager branches on
  it instead of strings.Contains(text, "Switched to") — UI copy is no longer
  a control-flow contract (a provider named 'Switched to prod' could close
  the manager on a refusal).
- Manager delete's keychain subprocess + OAuth lookup run in a follow-up
  tea.Cmd (the confirm keypress no longer blocks the render loop); the
  credential probe takes ONE token-store snapshot instead of 2 reads per
  row, and resolved states carry over across reloads (no 'checking…' flash).
- Credential-state reporting converges: /provider status recognizes OAuth
  logins (was 'api key: not set' + warning for a WORKING provider), doctor
  uses the shared HasConfiguredCredential definition plus OAuth awareness
  (its 'matching ProviderSnapshot.APIKeySet' claim is true again), and the
  manager's classifier falls through a stale stored-key marker to env/OAuth
  the way the runtime does.

* Fix provider manager test formatting
2026-07-07 10:34:59 +08:00

154 lines
5.9 KiB
Go

package cli
import (
"testing"
"time"
"github.com/Gitlawb/zero/internal/config"
"github.com/Gitlawb/zero/internal/oauth"
)
func TestFirstUsableProviderPrefersRemoteKeyed(t *testing.T) {
withAuthStore(t)
providers := []config.ProviderProfile{
{Name: "ollama", CatalogID: "ollama", BaseURL: "http://localhost:11434/v1", APIKey: "k"}, // usable but local
{Name: "moonshot", CatalogID: "moonshot", BaseURL: "https://api.moonshot.ai/v1", APIKey: "k"}, // usable, remote
{Name: "xai", CatalogID: "xai", APIKeyEnv: "XAI_API_KEY"}, // not usable (env only, no inline key)
}
got, ok := firstUsableProvider(providers)
if !ok || got.Name != "moonshot" {
t.Fatalf("want remote keyed provider (moonshot), got %q ok=%v", got.Name, ok)
}
}
func TestFirstUsableProviderFallsBackToLocal(t *testing.T) {
withAuthStore(t)
providers := []config.ProviderProfile{
{Name: "xai", CatalogID: "xai", APIKeyEnv: "XAI_API_KEY"}, // not usable
{Name: "ollama", CatalogID: "ollama", BaseURL: "http://localhost:11434/v1", APIKey: "k"}, // local, usable
}
got, ok := firstUsableProvider(providers)
if !ok || got.Name != "ollama" {
t.Fatalf("want local usable fallback (ollama), got %q ok=%v", got.Name, ok)
}
}
func TestFirstUsableProviderNoneUsable(t *testing.T) {
withAuthStore(t)
providers := []config.ProviderProfile{
{Name: "xai", CatalogID: "xai", APIKeyEnv: "XAI_API_KEY"},
{Name: "openai", CatalogID: "openai", APIKeyEnv: "OPENAI_API_KEY"},
}
if got, ok := firstUsableProvider(providers); ok {
t.Fatalf("no provider has a credential, want ok=false, got %q", got.Name)
}
}
// A keyless local proxy (chatgpt-proxy, RequiresAuth=false) is usable without a
// credential, so it can serve as a fallback rather than forcing onboarding.
func TestFirstUsableProviderAcceptsKeylessLocalProxy(t *testing.T) {
// Isolate the OAuth token store: on a developer machine with a real xai
// login, the env-only xai profile would count as usable and win over the
// local proxy this test is about.
withAuthStore(t)
providers := []config.ProviderProfile{
{Name: "xai", CatalogID: "xai", APIKeyEnv: "XAI_API_KEY"},
{Name: "chatgpt", CatalogID: "chatgpt-proxy", BaseURL: "http://localhost:10531/v1"},
}
got, ok := firstUsableProvider(providers)
if !ok || got.Name != "chatgpt" {
t.Fatalf("want keyless local proxy fallback, got %q ok=%v", got.Name, ok)
}
}
// A profile whose catalog entry no longer resolves and that carries no explicit
// BaseURL has no endpoint, so it must be skipped rather than selected as a
// fallback that fails at first use. A stale CatalogID with a BaseURL still works.
func TestFirstUsableProviderSkipsUnresolvableCatalogWithoutBaseURL(t *testing.T) {
withAuthStore(t)
providers := []config.ProviderProfile{
{Name: "ghost", CatalogID: "no-such-catalog-entry", APIKey: "k"}, // unusable: no endpoint
{Name: "custom", CatalogID: "no-such-catalog-entry", BaseURL: "https://api.custom.test/v1", APIKey: "k"},
}
got, ok := firstUsableProvider(providers)
if !ok || got.Name != "custom" {
t.Fatalf("want custom-endpoint fallback, got %q ok=%v", got.Name, ok)
}
}
// An OAuth-only provider (no inline key, no env var) must be selectable as a
// fallback, matching setupRequired/usableSavedProviders — otherwise a fully
// authenticated user gets forced back into onboarding when activeProvider
// goes stale.
func TestFirstUsableProviderRecognizesOAuthLogin(t *testing.T) {
withAuthStore(t)
store, err := oauth.NewStore(oauth.StoreOptions{})
if err != nil {
t.Fatalf("NewStore: %v", err)
}
if err := store.Save(oauth.ProviderKey("xai"), oauth.Token{AccessToken: "tok", ExpiresAt: time.Now().Add(time.Hour)}); err != nil {
t.Fatalf("seed token: %v", err)
}
providers := []config.ProviderProfile{
{Name: "xai", CatalogID: "xai", APIKeyEnv: "XAI_API_KEY"}, // no inline key/env, but logged in via OAuth
}
got, ok := firstUsableProvider(providers)
if !ok || got.Name != "xai" {
t.Fatalf("want OAuth-logged-in provider (xai), got %q ok=%v", got.Name, ok)
}
}
func TestProviderProfileIsLocal(t *testing.T) {
cases := []struct {
name string
baseURL string
want bool
}{
{"loopback name", "http://localhost:11434/v1", true},
{"loopback v4", "http://127.0.0.1:8080", true},
{"loopback v6", "http://[::1]:10531/v1", true},
{"remote", "https://api.moonshot.ai/v1", false},
{"contains-localhost-substring", "https://notlocalhost.com/v1", false},
{"host-with-127-substring", "https://api127.0.0.1.example.com", false},
{"empty", "", false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := providerProfileIsLocal(config.ProviderProfile{BaseURL: tc.baseURL})
if got != tc.want {
t.Fatalf("providerProfileIsLocal(%q) = %v, want %v", tc.baseURL, got, tc.want)
}
})
}
}
// TestFirstUsableProviderAcceptsOAuthLoginProfile: a keyless catalog profile
// whose credential is a stored OAuth login (the shape the login flows persist)
// must be usable during active-pointer recovery — the same rule setupRequired
// and usableSavedProviders apply — instead of falling through to onboarding.
func TestFirstUsableProviderAcceptsOAuthLoginProfile(t *testing.T) {
withAuthStore(t)
providers := []config.ProviderProfile{
{Name: "chatgpt", CatalogID: "chatgpt", BaseURL: "https://chatgpt.com/backend-api/codex", Model: "gpt-5.5"},
}
// No login stored: still not usable.
if got, ok := firstUsableProvider(providers); ok {
t.Fatalf("keyless profile without a login must not be usable, got %q", got.Name)
}
store, err := oauth.NewStore(oauth.StoreOptions{})
if err != nil {
t.Fatalf("oauth store: %v", err)
}
if err := store.Save(oauth.ProviderKey("chatgpt"), oauth.Token{AccessToken: "bearer-123"}); err != nil {
t.Fatalf("save token: %v", err)
}
got, ok := firstUsableProvider(providers)
if !ok || got.Name != "chatgpt" {
t.Fatalf("want OAuth-login profile to be usable, got %q ok=%v", got.Name, ok)
}
}