git.iko.soy/crush
on main help sign in
fix(config): check config file for newer token before OAuth refresh
99bc5ceAndrey Nering
internal/config/store.go+62−7
compare with main
unifiedsplit
@@ -3,6 +3,7 @@ package config
3import (
3import (
4 "cmp"
4 "cmp"
5 "context"
5 "context"
6+ "encoding/json"
6 "fmt"
7 "fmt"
7 "log/slog"
8 "log/slog"
8 "os"
9 "os"
@@ -288,6 +289,9 @@ func (s *ConfigStore) SetProviderAPIKey(scope Scope, providerID string, apiKey a
288}
289}
289
290
290// RefreshOAuthToken refreshes the OAuth token for the given provider.
291// RefreshOAuthToken refreshes the OAuth token for the given provider.
292+// Before making an external refresh request, it checks the config file on
293+// disk to see if another Crush session has already refreshed the token. If
294+// a newer token is found, it is used instead of refreshing.
291func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, providerID string) error {
295func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, providerID string) error {
292 providerConfig, exists := s.config.Providers.Get(providerID)
296 providerConfig, exists := s.config.Providers.Get(providerID)
293 if !exists {
297 if !exists {
@@ -298,13 +302,29 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
298 return fmt.Errorf("provider %s does not have an OAuth token", providerID)
302 return fmt.Errorf("provider %s does not have an OAuth token", providerID)
299 }
303 }
300
304
305+ // Check if another session refreshed the token recently by reading
306+ // the current token from the config file on disk.
307+ newToken, err := s.loadTokenFromDisk(scope, providerID)
308+ if err != nil {
309+ slog.Warn("Failed to read token from config file, proceeding with refresh", "provider", providerID, "error", err)
310+ } else if newToken != nil && newToken.AccessToken != providerConfig.OAuthToken.AccessToken {
311+ slog.Info("Using token refreshed by another session", "provider", providerID)
312+ providerConfig.OAuthToken = newToken
313+ providerConfig.APIKey = newToken.AccessToken
314+ if providerID == string(catwalk.InferenceProviderCopilot) {
315+ providerConfig.SetupGitHubCopilot()
316+ }
317+ s.config.Providers.Set(providerID, providerConfig)
318+ return nil
319+ }
320+
301− var newToken *oauth.Token
321+ var refreshedToken *oauth.Token
302 var refreshErr error
322 var refreshErr error
303 switch providerID {
323 switch providerID {
304 case string(catwalk.InferenceProviderCopilot):
324 case string(catwalk.InferenceProviderCopilot):
305− newToken, refreshErr = copilot.RefreshToken(ctx, providerConfig.OAuthToken.RefreshToken)
325+ refreshedToken, refreshErr = copilot.RefreshToken(ctx, providerConfig.OAuthToken.RefreshToken)
306 case hyperp.Name:
326 case hyperp.Name:
307− newToken, refreshErr = hyper.ExchangeToken(ctx, providerConfig.OAuthToken.RefreshToken)
327+ refreshedToken, refreshErr = hyper.ExchangeToken(ctx, providerConfig.OAuthToken.RefreshToken)
308 default:
328 default:
309 return fmt.Errorf("OAuth refresh not supported for provider %s", providerID)
329 return fmt.Errorf("OAuth refresh not supported for provider %s", providerID)
310 }
330 }
@@ -313,8 +333,8 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
313 }
333 }
314
334
315 slog.Info("Successfully refreshed OAuth token", "provider", providerID)
335 slog.Info("Successfully refreshed OAuth token", "provider", providerID)
316− providerConfig.OAuthToken = newToken
336+ providerConfig.OAuthToken = refreshedToken
317− providerConfig.APIKey = newToken.AccessToken
337+ providerConfig.APIKey = refreshedToken.AccessToken
318
338
319 switch providerID {
339 switch providerID {
320 case string(catwalk.InferenceProviderCopilot):
340 case string(catwalk.InferenceProviderCopilot):
@@ -324,8 +344,8 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
324 s.config.Providers.Set(providerID, providerConfig)
344 s.config.Providers.Set(providerID, providerConfig)
325
345
326 if err := cmp.Or(
346 if err := cmp.Or(
327− s.SetConfigField(scope, fmt.Sprintf("providers.%s.api_key", providerID), newToken.AccessToken),
347+ s.SetConfigField(scope, fmt.Sprintf("providers.%s.api_key", providerID), refreshedToken.AccessToken),
328− s.SetConfigField(scope, fmt.Sprintf("providers.%s.oauth", providerID), newToken),
348+ s.SetConfigField(scope, fmt.Sprintf("providers.%s.oauth", providerID), refreshedToken),
329 ); err != nil {
349 ); err != nil {
330 return fmt.Errorf("failed to persist refreshed token: %w", err)
350 return fmt.Errorf("failed to persist refreshed token: %w", err)
331 }
351 }
@@ -333,6 +353,41 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
333 return nil
353 return nil
334}
354}
335
355
356+// loadTokenFromDisk reads the OAuth token for the given provider from the
357+// config file on disk. Returns nil if the token is not found or matches the
358+// current in-memory token.
359+func (s *ConfigStore) loadTokenFromDisk(scope Scope, providerID string) (*oauth.Token, error) {
360+ path, err := s.configPath(scope)
361+ if err != nil {
362+ return nil, err
363+ }
364+
365+ data, err := os.ReadFile(path)
366+ if err != nil {
367+ if os.IsNotExist(err) {
368+ return nil, nil
369+ }
370+ return nil, err
371+ }
372+
373+ oauthKey := fmt.Sprintf("providers.%s.oauth", providerID)
374+ oauthResult := gjson.Get(string(data), oauthKey)
375+ if !oauthResult.Exists() {
376+ return nil, nil
377+ }
378+
379+ var token oauth.Token
380+ if err := json.Unmarshal([]byte(oauthResult.Raw), &token); err != nil {
381+ return nil, err
382+ }
383+
384+ if token.AccessToken == "" {
385+ return nil, nil
386+ }
387+
388+ return &token, nil
389+}
390+
336// recordRecentModel records a model in the recent models list.
391// recordRecentModel records a model in the recent models list.
337func (s *ConfigStore) recordRecentModel(scope Scope, modelType SelectedModelType, model SelectedModel) error {
392func (s *ConfigStore) recordRecentModel(scope Scope, modelType SelectedModelType, model SelectedModel) error {
338 if model.Provider == "" || model.Model == "" {
393 if model.Provider == "" || model.Model == "" {
internal/config/store_test.go+182
@@ -8,6 +8,8 @@ import (
8 "testing"
8 "testing"
9 "time"
9 "time"
10
10
11+ "github.com/charmbracelet/crush/internal/csync"
12+ "github.com/charmbracelet/crush/internal/oauth"
11 "github.com/stretchr/testify/require"
13 "github.com/stretchr/testify/require"
12)
14)
13
15
@@ -510,3 +512,183 @@ func TestAutoReloadDisabledDuringReload(t *testing.T) {
510 // Verify reload completed successfully
512 // Verify reload completed successfully
511 require.False(t, store.autoReloadDisabled, "autoReloadDisabled should be false after ReloadFromDisk")
513 require.False(t, store.autoReloadDisabled, "autoReloadDisabled should be false after ReloadFromDisk")
512}
514}
515+
516+func TestLoadTokenFromDisk_ReturnsNewerToken(t *testing.T) {
517+ t.Parallel()
518+
519+ dir := t.TempDir()
520+ configPath := filepath.Join(dir, "crush.json")
521+
522+ // Create config file with a newer token on disk
523+ configContent := `{
524+ "providers": {
525+ "hyper": {
526+ "oauth": {
527+ "access_token": "newer-token-from-disk",
528+ "refresh_token": "refresh-abc",
529+ "expires_in": 3600,
530+ "expires_at": 9999999999
531+ }
532+ }
533+ }
534+ }`
535+ require.NoError(t, os.WriteFile(configPath, []byte(configContent), 0o600))
536+
537+ store := &ConfigStore{
538+ config: &Config{},
539+ globalDataPath: configPath,
540+ }
541+
542+ token, err := store.loadTokenFromDisk(ScopeGlobal, "hyper")
543+ require.NoError(t, err)
544+ require.NotNil(t, token)
545+ require.Equal(t, "newer-token-from-disk", token.AccessToken)
546+ require.Equal(t, "refresh-abc", token.RefreshToken)
547+ require.Equal(t, 3600, token.ExpiresIn)
548+ require.Equal(t, int64(9999999999), token.ExpiresAt)
549+}
550+
551+func TestLoadTokenFromDisk_ReturnsNilWhenSameToken(t *testing.T) {
552+ t.Parallel()
553+
554+ dir := t.TempDir()
555+ configPath := filepath.Join(dir, "crush.json")
556+
557+ // Create config file with the same token
558+ configContent := `{
559+ "providers": {
560+ "hyper": {
561+ "oauth": {
562+ "access_token": "same-token",
563+ "refresh_token": "refresh-abc",
564+ "expires_in": 3600,
565+ "expires_at": 9999999999
566+ }
567+ }
568+ }
569+ }`
570+ require.NoError(t, os.WriteFile(configPath, []byte(configContent), 0o600))
571+
572+ store := &ConfigStore{
573+ config: &Config{},
574+ globalDataPath: configPath,
575+ }
576+
577+ token, err := store.loadTokenFromDisk(ScopeGlobal, "hyper")
578+ require.NoError(t, err)
579+ require.NotNil(t, token)
580+ require.Equal(t, "same-token", token.AccessToken)
581+}
582+
583+func TestLoadTokenFromDisk_ReturnsNilWhenFileMissing(t *testing.T) {
584+ t.Parallel()
585+
586+ dir := t.TempDir()
587+ configPath := filepath.Join(dir, "nonexistent.json")
588+
589+ store := &ConfigStore{
590+ config: &Config{},
591+ globalDataPath: configPath,
592+ }
593+
594+ token, err := store.loadTokenFromDisk(ScopeGlobal, "hyper")
595+ require.NoError(t, err)
596+ require.Nil(t, token)
597+}
598+
599+func TestLoadTokenFromDisk_ReturnsNilWhenProviderMissing(t *testing.T) {
600+ t.Parallel()
601+
602+ dir := t.TempDir()
603+ configPath := filepath.Join(dir, "crush.json")
604+
605+ // Create config file without the hyper provider
606+ configContent := `{"providers": {"openai": {"api_key": "test-key"}}}`
607+ require.NoError(t, os.WriteFile(configPath, []byte(configContent), 0o600))
608+
609+ store := &ConfigStore{
610+ config: &Config{},
611+ globalDataPath: configPath,
612+ }
613+
614+ token, err := store.loadTokenFromDisk(ScopeGlobal, "hyper")
615+ require.NoError(t, err)
616+ require.Nil(t, token)
617+}
618+
619+func TestLoadTokenFromDisk_ReturnsNilWhenOAuthMissing(t *testing.T) {
620+ t.Parallel()
621+
622+ dir := t.TempDir()
623+ configPath := filepath.Join(dir, "crush.json")
624+
625+ // Create config file with provider but no OAuth token
626+ configContent := `{"providers": {"hyper": {"api_key": "test-key"}}}`
627+ require.NoError(t, os.WriteFile(configPath, []byte(configContent), 0o600))
628+
629+ store := &ConfigStore{
630+ config: &Config{},
631+ globalDataPath: configPath,
632+ }
633+
634+ token, err := store.loadTokenFromDisk(ScopeGlobal, "hyper")
635+ require.NoError(t, err)
636+ require.Nil(t, token)
637+}
638+
639+func TestRefreshOAuthToken_UsesDiskTokenWhenDifferent(t *testing.T) {
640+ t.Parallel()
641+
642+ dir := t.TempDir()
643+ configPath := filepath.Join(dir, "crush.json")
644+
645+ // Create config file with a newer token on disk
646+ configContent := `{
647+ "providers": {
648+ "hyper": {
649+ "api_key": "newer-access-token",
650+ "oauth": {
651+ "access_token": "newer-access-token",
652+ "refresh_token": "refresh-abc",
653+ "expires_in": 3600,
654+ "expires_at": 9999999999
655+ }
656+ }
657+ }
658+ }`
659+ require.NoError(t, os.WriteFile(configPath, []byte(configContent), 0o600))
660+
661+ // Set up store with an older in-memory token
662+ oldToken := &oauth.Token{
663+ AccessToken: "older-access-token",
664+ RefreshToken: "refresh-abc",
665+ ExpiresIn: 3600,
666+ ExpiresAt: time.Now().Add(-time.Hour).Unix(), // Expired
667+ }
668+
669+ providers := csync.NewMap[string, ProviderConfig]()
670+ providers.Set("hyper", ProviderConfig{
671+ ID: "hyper",
672+ Name: "Hyper",
673+ APIKey: oldToken.AccessToken,
674+ OAuthToken: oldToken,
675+ })
676+
677+ store := &ConfigStore{
678+ config: &Config{
679+ Providers: providers,
680+ },
681+ globalDataPath: configPath,
682+ }
683+
684+ // Refresh should use the disk token without making an external call
685+ err := store.RefreshOAuthToken(context.Background(), ScopeGlobal, "hyper")
686+ require.NoError(t, err)
687+
688+ // Verify the in-memory token was updated to the disk token
689+ updatedConfig, ok := store.config.Providers.Get("hyper")
690+ require.True(t, ok)
691+ require.Equal(t, "newer-access-token", updatedConfig.APIKey)
692+ require.Equal(t, "newer-access-token", updatedConfig.OAuthToken.AccessToken)
693+ require.Equal(t, "refresh-abc", updatedConfig.OAuthToken.RefreshToken)
694+}