Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 75 additions & 2 deletions credentials/combined.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,25 @@ import (
"context"
"fmt"
"strings"
"sync"
"time"
)

// CombinedStore persists secrets in the OS secret store (macOS Keychain / Linux Secret Service).
type CombinedStore struct {
Keychain Store
mu sync.RWMutex
cache map[string]combinedCacheEntry
}

type combinedCacheEntry struct {
secret string
found bool
cachedAt time.Time
}

const combinedGetCacheTTL = 2 * time.Second

func NewCombinedStore() *CombinedStore {
return &CombinedStore{
Keychain: newPlatformKeyringStore(),
Expand All @@ -28,19 +40,80 @@ func (c *CombinedStore) Set(ctx context.Context, account, secret string) error {
if err := c.Keychain.Set(ctx, account, secret); err != nil {
return fmt.Errorf("%w: %s", err, KeyringUnavailableHelp())
}
c.invalidateAccount(account)
return nil
}

func (c *CombinedStore) Get(ctx context.Context, account string) (string, error) {
if c.Keychain == nil {
return "", ErrNotFound
}
return c.Keychain.Get(ctx, account)
if secret, ok := c.cachedGet(account); ok {
if secret == "" {
return "", ErrNotFound
}
return secret, nil
}
secret, err := c.Keychain.Get(ctx, account)
if err == nil {
c.storeCachedGet(account, secret, true)
return secret, nil
}
if err == ErrNotFound {
c.storeCachedGet(account, "", false)
}
return secret, err
}

func (c *CombinedStore) Delete(ctx context.Context, account string) error {
if c.Keychain == nil {
return nil
}
return c.Keychain.Delete(ctx, account)
err := c.Keychain.Delete(ctx, account)
if err == nil {
c.invalidateAccount(account)
}
return err
}

func (c *CombinedStore) cachedGet(account string) (string, bool) {
c.mu.RLock()
defer c.mu.RUnlock()
if c.cache == nil {
return "", false
}
entry, ok := c.cache[c.cacheKey(account)]
if !ok || time.Since(entry.cachedAt) >= combinedGetCacheTTL {
return "", false
}
if !entry.found {
return "", true
}
return entry.secret, true
}

func (c *CombinedStore) storeCachedGet(account, secret string, found bool) {
c.mu.Lock()
defer c.mu.Unlock()
if c.cache == nil {
c.cache = make(map[string]combinedCacheEntry)
}
c.cache[c.cacheKey(account)] = combinedCacheEntry{
secret: strings.TrimSpace(secret),
found: found,
cachedAt: time.Now(),
}
}

func (c *CombinedStore) invalidateAccount(account string) {
c.mu.Lock()
defer c.mu.Unlock()
if c.cache == nil {
return
}
delete(c.cache, c.cacheKey(account))
}

func (c *CombinedStore) cacheKey(account string) string {
return strings.ToLower(strings.TrimSpace(account))
}
108 changes: 108 additions & 0 deletions credentials/combined_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,54 @@ package credentials
import (
"context"
"errors"
"sync"
"testing"
)

type countingStore struct {
mu sync.Mutex
data map[string]string
gets int
deletes int
}

func (s *countingStore) Set(ctx context.Context, account, secret string) error {
_ = ctx
s.mu.Lock()
defer s.mu.Unlock()
if s.data == nil {
s.data = map[string]string{}
}
s.data[account] = secret
return nil
}

func (s *countingStore) Get(ctx context.Context, account string) (string, error) {
_ = ctx
s.mu.Lock()
defer s.mu.Unlock()
s.gets++
if s.data == nil {
return "", ErrNotFound
}
v, ok := s.data[account]
if !ok || v == "" {
return "", ErrNotFound
}
return v, nil
}

func (s *countingStore) Delete(ctx context.Context, account string) error {
_ = ctx
s.mu.Lock()
defer s.mu.Unlock()
s.deletes++
if s.data != nil {
delete(s.data, account)
}
return nil
}

// --- CombinedStore edge-case tests (nil keychain, empty secrets) ---

func TestCombinedStore_NilKeychain(t *testing.T) {
Expand Down Expand Up @@ -103,6 +148,69 @@ func TestCombinedStore_FullCycle(t *testing.T) {
}
}

func TestCombinedStore_GetUsesShortLivedCache(t *testing.T) {
t.Parallel()
backend := &countingStore{data: map[string]string{"account": "secret"}}
cs := &CombinedStore{Keychain: backend}
ctx := context.Background()

got1, err := cs.Get(ctx, "account")
if err != nil || got1 != "secret" {
t.Fatalf("first Get = (%q, %v), want (secret, nil)", got1, err)
}
got2, err := cs.Get(ctx, "account")
if err != nil || got2 != "secret" {
t.Fatalf("second Get = (%q, %v), want (secret, nil)", got2, err)
}
if backend.gets != 1 {
t.Fatalf("backend Get count = %d, want 1", backend.gets)
}
}

func TestCombinedStore_SetInvalidatesCache(t *testing.T) {
t.Parallel()
backend := &countingStore{data: map[string]string{"account": "old"}}
cs := &CombinedStore{Keychain: backend}
ctx := context.Background()

got, err := cs.Get(ctx, "account")
if err != nil || got != "old" {
t.Fatalf("initial Get = (%q, %v), want (old, nil)", got, err)
}
if err := cs.Set(ctx, "account", "new"); err != nil {
t.Fatalf("Set error: %v", err)
}
got, err = cs.Get(ctx, "account")
if err != nil || got != "new" {
t.Fatalf("post-Set Get = (%q, %v), want (new, nil)", got, err)
}
if backend.gets != 2 {
t.Fatalf("backend Get count after invalidation = %d, want 2", backend.gets)
}
}

func TestCombinedStore_DeleteInvalidatesCache(t *testing.T) {
t.Parallel()
backend := &countingStore{data: map[string]string{"account": "secret"}}
cs := &CombinedStore{Keychain: backend}
ctx := context.Background()

got, err := cs.Get(ctx, "account")
if err != nil || got != "secret" {
t.Fatalf("initial Get = (%q, %v), want (secret, nil)", got, err)
}
if err := cs.Delete(ctx, "account"); err != nil {
t.Fatalf("Delete error: %v", err)
}
got, err = cs.Get(ctx, "account")
if err != ErrNotFound || got != "" {
t.Fatalf("post-Delete Get = (%q, %v), want (\"\", ErrNotFound)", got, err)
}
if backend.gets != 2 {
t.Fatalf("backend Get count after delete invalidation = %d, want 2", backend.gets)
}
}

// --- DefaultStore / SetDefaultStore tests ---

func TestSetDefaultStore_Override(t *testing.T) {
Expand Down
37 changes: 20 additions & 17 deletions runtime/default_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,35 +49,37 @@ func PreferredProvider(ctx context.Context) string {
if ctx == nil {
ctx = context.Background()
}
if provider := normalizeRuntimeProviderID(ActiveProvider(ctx)); provider != "" && providerConfigured(ctx, provider) {
return preferredProviderWithState(ctx, newSelectionRuntimeState(ctx))
}

func preferredProviderWithState(ctx context.Context, state *selectionRuntimeState) string {
if provider := normalizeRuntimeProviderID(ActiveProvider(ctx)); provider != "" && providerConfiguredWithState(state, provider) {
return provider
}
if model := ActiveModel(ctx); model != "" {
if provider := inferProviderForModel(ctx, model); provider != "" && providerConfigured(ctx, provider) {
if provider := inferProviderForModel(ctx, model); provider != "" && providerConfiguredWithState(state, provider) {
return provider
}
}
if provider := preferredConfiguredProvider(ctx); provider != "" {
if provider := preferredConfiguredProviderWithState(state); provider != "" {
return provider
}
return preferredDetectedProvider()
return preferredDetectedProviderWithState(state)
}

func preferredConfiguredProvider(ctx context.Context) string {
rt, err := Load(ctx)
if err != nil || rt == nil {
func preferredConfiguredProviderWithState(state *selectionRuntimeState) string {
compiled := state.compiledCatalog()
if compiled == nil || compiled.Catalog == nil {
return ""
}
rows, err := rt.DeploymentRows()
if err != nil || len(rows) == 0 {
return ""
}
configured := make(map[string]struct{}, len(rows))
for _, row := range rows {
if !row.Configured {
env := state.env()
configured := make(map[string]struct{}, len(compiled.Catalog.Deployments))
for id, dep := range compiled.Catalog.Deployments {
dc := config.DeploymentConfigFromEnv(dep, env)
if !config.DeploymentConfigured(id, dep, dc) {
continue
}
if provider := catalog.CanonicalProviderID(row.ProviderID); provider != "" {
if provider := catalog.CanonicalProviderID(dep.ProviderID); provider != "" {
configured[provider] = struct{}{}
}
}
Expand All @@ -98,15 +100,16 @@ func preferredConfiguredProvider(ctx context.Context) string {
return ordered[0]
}

func preferredDetectedProvider() string {
func preferredDetectedProviderWithState(state *selectionRuntimeState) string {
env := state.env()
for _, provider := range registry.ChatProviderPreferenceOrder() {
profile, ok := runtimeProfileForProvider(provider)
if !ok {
continue
}
ready := true
for _, envKey := range profile.DetectionEnv {
if runtimeEnvValue(envKey) == "" {
if strings.TrimSpace(env[envKey]) == "" && runtimeEnvValue(envKey) == "" {
ready = false
break
}
Expand Down
Loading
Loading