From 8fe86dd8ab6d73c1974972457707918a9c75864e Mon Sep 17 00:00:00 2001 From: Claude Code Date: Fri, 20 Feb 2026 16:47:54 +0200 Subject: [PATCH] Fix OAuth reconnect loop caused by DCR-only placeholder token (#305) After logout/restart/login, DCR saves client credentials to storage before token exchange completes. GetToken() returned a non-nil token for these DCR-only records (empty AccessToken, zero ExpiresAt), causing scanForNewTokens() to trigger reconnect loops every 10 seconds. Now GetToken() returns ErrNoToken when AccessToken is empty, consistent with HasPersistedToken() which already checks this correctly. Co-Authored-By: Claude Opus 4.6 --- internal/oauth/persistent_token_store.go | 11 +++++++ internal/oauth/persistent_token_store_test.go | 32 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/internal/oauth/persistent_token_store.go b/internal/oauth/persistent_token_store.go index f5874f49..714d39e8 100644 --- a/internal/oauth/persistent_token_store.go +++ b/internal/oauth/persistent_token_store.go @@ -87,6 +87,17 @@ func (p *PersistentTokenStore) GetToken(ctx context.Context) (*client.Token, err return nil, transport.ErrNoToken } + // DCR (Dynamic Client Registration) creates a minimal record with only + // client credentials but no access token. Treat these as "no token" to + // prevent scanForNewTokens() from triggering reconnect loops (issue #305). + if record.AccessToken == "" { + p.logger.Debug("⏳ OAuth record exists but has no access token (DCR-only), treating as no token", + zap.String("server_name", p.serverName), + zap.String("server_key", p.serverKey), + zap.Bool("has_client_id", record.ClientID != "")) + return nil, transport.ErrNoToken + } + now := time.Now() timeUntilExpiry := record.ExpiresAt.Sub(now) isExpired := now.After(record.ExpiresAt) diff --git a/internal/oauth/persistent_token_store_test.go b/internal/oauth/persistent_token_store_test.go index 2af1f8da..569018ea 100644 --- a/internal/oauth/persistent_token_store_test.go +++ b/internal/oauth/persistent_token_store_test.go @@ -480,3 +480,35 @@ func TestPersistentTokenStoreSameNameDifferentURL(t *testing.T) { t.Errorf("Server2 token should still exist: got %s, want token-for-server2-url", retrievedToken2Again.AccessToken) } } + +// TestGetToken_DCROnlyRecord_ReturnsError reproduces GitHub issue #305: +// After DCR saves client credentials (but no access token), GetToken() should +// return ErrNoToken — not a non-nil token with empty AccessToken. Returning a +// non-nil token causes scanForNewTokens() to trigger reconnect loops. +func TestGetToken_DCROnlyRecord_ReturnsError(t *testing.T) { + tmpDir := t.TempDir() + logger := zap.NewNop().Sugar() + db, err := storage.NewBoltDB(tmpDir, logger) + if err != nil { + t.Fatalf("Failed to create BoltDB: %v", err) + } + defer db.Close() + + serverName := "oauth-server" + serverURL := "https://oauth.example.com/mcp" + serverKey := GenerateServerKey(serverName, serverURL) + + // Simulate DCR saving only client credentials (no access token yet) + err = db.UpdateOAuthClientCredentials(serverKey, "dcr-client-id", "dcr-secret", 12345) + if err != nil { + t.Fatalf("Failed to save DCR credentials: %v", err) + } + + // GetToken should return an error for DCR-only records (no access token) + tokenStore := NewPersistentTokenStore(serverName, serverURL, db) + tok, err := tokenStore.GetToken(context.Background()) + if err == nil { + t.Errorf("GetToken() should return error for DCR-only record (no access token), got token with AccessToken=%q ExpiresAt=%v", + tok.AccessToken, tok.ExpiresAt) + } +}