mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-09 18:08:31 -10:00
* 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
154 lines
5.9 KiB
Go
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)
|
|
}
|
|
}
|