Skip to content

Commit b725e7d

Browse files
authored
perf(runtime): cache credential-backed selection state (#57)
1 parent 8dfb25c commit b725e7d

5 files changed

Lines changed: 330 additions & 30 deletions

File tree

credentials/combined.go

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,25 @@ import (
44
"context"
55
"fmt"
66
"strings"
7+
"sync"
8+
"time"
79
)
810

911
// CombinedStore persists secrets in the OS secret store (macOS Keychain / Linux Secret Service).
1012
type CombinedStore struct {
1113
Keychain Store
14+
mu sync.RWMutex
15+
cache map[string]combinedCacheEntry
1216
}
1317

18+
type combinedCacheEntry struct {
19+
secret string
20+
found bool
21+
cachedAt time.Time
22+
}
23+
24+
const combinedGetCacheTTL = 2 * time.Second
25+
1426
func NewCombinedStore() *CombinedStore {
1527
return &CombinedStore{
1628
Keychain: newPlatformKeyringStore(),
@@ -28,19 +40,80 @@ func (c *CombinedStore) Set(ctx context.Context, account, secret string) error {
2840
if err := c.Keychain.Set(ctx, account, secret); err != nil {
2941
return fmt.Errorf("%w: %s", err, KeyringUnavailableHelp())
3042
}
43+
c.invalidateAccount(account)
3144
return nil
3245
}
3346

3447
func (c *CombinedStore) Get(ctx context.Context, account string) (string, error) {
3548
if c.Keychain == nil {
3649
return "", ErrNotFound
3750
}
38-
return c.Keychain.Get(ctx, account)
51+
if secret, ok := c.cachedGet(account); ok {
52+
if secret == "" {
53+
return "", ErrNotFound
54+
}
55+
return secret, nil
56+
}
57+
secret, err := c.Keychain.Get(ctx, account)
58+
if err == nil {
59+
c.storeCachedGet(account, secret, true)
60+
return secret, nil
61+
}
62+
if err == ErrNotFound {
63+
c.storeCachedGet(account, "", false)
64+
}
65+
return secret, err
3966
}
4067

4168
func (c *CombinedStore) Delete(ctx context.Context, account string) error {
4269
if c.Keychain == nil {
4370
return nil
4471
}
45-
return c.Keychain.Delete(ctx, account)
72+
err := c.Keychain.Delete(ctx, account)
73+
if err == nil {
74+
c.invalidateAccount(account)
75+
}
76+
return err
77+
}
78+
79+
func (c *CombinedStore) cachedGet(account string) (string, bool) {
80+
c.mu.RLock()
81+
defer c.mu.RUnlock()
82+
if c.cache == nil {
83+
return "", false
84+
}
85+
entry, ok := c.cache[c.cacheKey(account)]
86+
if !ok || time.Since(entry.cachedAt) >= combinedGetCacheTTL {
87+
return "", false
88+
}
89+
if !entry.found {
90+
return "", true
91+
}
92+
return entry.secret, true
93+
}
94+
95+
func (c *CombinedStore) storeCachedGet(account, secret string, found bool) {
96+
c.mu.Lock()
97+
defer c.mu.Unlock()
98+
if c.cache == nil {
99+
c.cache = make(map[string]combinedCacheEntry)
100+
}
101+
c.cache[c.cacheKey(account)] = combinedCacheEntry{
102+
secret: strings.TrimSpace(secret),
103+
found: found,
104+
cachedAt: time.Now(),
105+
}
106+
}
107+
108+
func (c *CombinedStore) invalidateAccount(account string) {
109+
c.mu.Lock()
110+
defer c.mu.Unlock()
111+
if c.cache == nil {
112+
return
113+
}
114+
delete(c.cache, c.cacheKey(account))
115+
}
116+
117+
func (c *CombinedStore) cacheKey(account string) string {
118+
return strings.ToLower(strings.TrimSpace(account))
46119
}

credentials/combined_store_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,54 @@ package credentials
33
import (
44
"context"
55
"errors"
6+
"sync"
67
"testing"
78
)
89

10+
type countingStore struct {
11+
mu sync.Mutex
12+
data map[string]string
13+
gets int
14+
deletes int
15+
}
16+
17+
func (s *countingStore) Set(ctx context.Context, account, secret string) error {
18+
_ = ctx
19+
s.mu.Lock()
20+
defer s.mu.Unlock()
21+
if s.data == nil {
22+
s.data = map[string]string{}
23+
}
24+
s.data[account] = secret
25+
return nil
26+
}
27+
28+
func (s *countingStore) Get(ctx context.Context, account string) (string, error) {
29+
_ = ctx
30+
s.mu.Lock()
31+
defer s.mu.Unlock()
32+
s.gets++
33+
if s.data == nil {
34+
return "", ErrNotFound
35+
}
36+
v, ok := s.data[account]
37+
if !ok || v == "" {
38+
return "", ErrNotFound
39+
}
40+
return v, nil
41+
}
42+
43+
func (s *countingStore) Delete(ctx context.Context, account string) error {
44+
_ = ctx
45+
s.mu.Lock()
46+
defer s.mu.Unlock()
47+
s.deletes++
48+
if s.data != nil {
49+
delete(s.data, account)
50+
}
51+
return nil
52+
}
53+
954
// --- CombinedStore edge-case tests (nil keychain, empty secrets) ---
1055

1156
func TestCombinedStore_NilKeychain(t *testing.T) {
@@ -103,6 +148,69 @@ func TestCombinedStore_FullCycle(t *testing.T) {
103148
}
104149
}
105150

151+
func TestCombinedStore_GetUsesShortLivedCache(t *testing.T) {
152+
t.Parallel()
153+
backend := &countingStore{data: map[string]string{"account": "secret"}}
154+
cs := &CombinedStore{Keychain: backend}
155+
ctx := context.Background()
156+
157+
got1, err := cs.Get(ctx, "account")
158+
if err != nil || got1 != "secret" {
159+
t.Fatalf("first Get = (%q, %v), want (secret, nil)", got1, err)
160+
}
161+
got2, err := cs.Get(ctx, "account")
162+
if err != nil || got2 != "secret" {
163+
t.Fatalf("second Get = (%q, %v), want (secret, nil)", got2, err)
164+
}
165+
if backend.gets != 1 {
166+
t.Fatalf("backend Get count = %d, want 1", backend.gets)
167+
}
168+
}
169+
170+
func TestCombinedStore_SetInvalidatesCache(t *testing.T) {
171+
t.Parallel()
172+
backend := &countingStore{data: map[string]string{"account": "old"}}
173+
cs := &CombinedStore{Keychain: backend}
174+
ctx := context.Background()
175+
176+
got, err := cs.Get(ctx, "account")
177+
if err != nil || got != "old" {
178+
t.Fatalf("initial Get = (%q, %v), want (old, nil)", got, err)
179+
}
180+
if err := cs.Set(ctx, "account", "new"); err != nil {
181+
t.Fatalf("Set error: %v", err)
182+
}
183+
got, err = cs.Get(ctx, "account")
184+
if err != nil || got != "new" {
185+
t.Fatalf("post-Set Get = (%q, %v), want (new, nil)", got, err)
186+
}
187+
if backend.gets != 2 {
188+
t.Fatalf("backend Get count after invalidation = %d, want 2", backend.gets)
189+
}
190+
}
191+
192+
func TestCombinedStore_DeleteInvalidatesCache(t *testing.T) {
193+
t.Parallel()
194+
backend := &countingStore{data: map[string]string{"account": "secret"}}
195+
cs := &CombinedStore{Keychain: backend}
196+
ctx := context.Background()
197+
198+
got, err := cs.Get(ctx, "account")
199+
if err != nil || got != "secret" {
200+
t.Fatalf("initial Get = (%q, %v), want (secret, nil)", got, err)
201+
}
202+
if err := cs.Delete(ctx, "account"); err != nil {
203+
t.Fatalf("Delete error: %v", err)
204+
}
205+
got, err = cs.Get(ctx, "account")
206+
if err != ErrNotFound || got != "" {
207+
t.Fatalf("post-Delete Get = (%q, %v), want (\"\", ErrNotFound)", got, err)
208+
}
209+
if backend.gets != 2 {
210+
t.Fatalf("backend Get count after delete invalidation = %d, want 2", backend.gets)
211+
}
212+
}
213+
106214
// --- DefaultStore / SetDefaultStore tests ---
107215

108216
func TestSetDefaultStore_Override(t *testing.T) {

runtime/default_provider.go

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,35 +49,37 @@ func PreferredProvider(ctx context.Context) string {
4949
if ctx == nil {
5050
ctx = context.Background()
5151
}
52-
if provider := normalizeRuntimeProviderID(ActiveProvider(ctx)); provider != "" && providerConfigured(ctx, provider) {
52+
return preferredProviderWithState(ctx, newSelectionRuntimeState(ctx))
53+
}
54+
55+
func preferredProviderWithState(ctx context.Context, state *selectionRuntimeState) string {
56+
if provider := normalizeRuntimeProviderID(ActiveProvider(ctx)); provider != "" && providerConfiguredWithState(state, provider) {
5357
return provider
5458
}
5559
if model := ActiveModel(ctx); model != "" {
56-
if provider := inferProviderForModel(ctx, model); provider != "" && providerConfigured(ctx, provider) {
60+
if provider := inferProviderForModel(ctx, model); provider != "" && providerConfiguredWithState(state, provider) {
5761
return provider
5862
}
5963
}
60-
if provider := preferredConfiguredProvider(ctx); provider != "" {
64+
if provider := preferredConfiguredProviderWithState(state); provider != "" {
6165
return provider
6266
}
63-
return preferredDetectedProvider()
67+
return preferredDetectedProviderWithState(state)
6468
}
6569

66-
func preferredConfiguredProvider(ctx context.Context) string {
67-
rt, err := Load(ctx)
68-
if err != nil || rt == nil {
70+
func preferredConfiguredProviderWithState(state *selectionRuntimeState) string {
71+
compiled := state.compiledCatalog()
72+
if compiled == nil || compiled.Catalog == nil {
6973
return ""
7074
}
71-
rows, err := rt.DeploymentRows()
72-
if err != nil || len(rows) == 0 {
73-
return ""
74-
}
75-
configured := make(map[string]struct{}, len(rows))
76-
for _, row := range rows {
77-
if !row.Configured {
75+
env := state.env()
76+
configured := make(map[string]struct{}, len(compiled.Catalog.Deployments))
77+
for id, dep := range compiled.Catalog.Deployments {
78+
dc := config.DeploymentConfigFromEnv(dep, env)
79+
if !config.DeploymentConfigured(id, dep, dc) {
7880
continue
7981
}
80-
if provider := catalog.CanonicalProviderID(row.ProviderID); provider != "" {
82+
if provider := catalog.CanonicalProviderID(dep.ProviderID); provider != "" {
8183
configured[provider] = struct{}{}
8284
}
8385
}
@@ -98,15 +100,16 @@ func preferredConfiguredProvider(ctx context.Context) string {
98100
return ordered[0]
99101
}
100102

101-
func preferredDetectedProvider() string {
103+
func preferredDetectedProviderWithState(state *selectionRuntimeState) string {
104+
env := state.env()
102105
for _, provider := range registry.ChatProviderPreferenceOrder() {
103106
profile, ok := runtimeProfileForProvider(provider)
104107
if !ok {
105108
continue
106109
}
107110
ready := true
108111
for _, envKey := range profile.DetectionEnv {
109-
if runtimeEnvValue(envKey) == "" {
112+
if strings.TrimSpace(env[envKey]) == "" && runtimeEnvValue(envKey) == "" {
110113
ready = false
111114
break
112115
}

0 commit comments

Comments
 (0)