fix(oauth): treat zero ExpiresAt as never-expires in HasPersistedToken (#453)

Authorization servers that omit `expires_in` from the /token response
(e.g. Atlassian's MCP endpoint at https://mcp.atlassian.com/v1/mcp)
land in BBolt with ExpiresAt = time.Time{}. HasValidToken and mcp-go's
own Token.IsExpired() already treat that as "never expires", but
HasPersistedToken was computing `time.Now().After(time.Time{})` which
is always true — so the persisted token was reported as expired on
every reconnect, skipBrowserFlow stayed false, and the upstream
looped forever asking for re-auth even though a valid access token
sat in storage.

Fix HasPersistedToken to short-circuit on IsZero(), matching
HasValidToken. Also clean up PersistentTokenStore.GetToken so it no
longer logs a spurious "token has expired" warning for those records.

Add three unit tests covering the zero / future / past ExpiresAt
cases against HasPersistedToken so the IsZero branch can't regress.

Author: electrolobzik

internal/oauth/config.go

@@ -100,6 +100,13 @@ func (m *TokenStoreManager) HasTokenStore(serverName string) bool {
 
 // HasPersistedToken checks if a token exists in persistent storage (BBolt) for the server.
 // This is the preferred method to check for existing tokens as it reflects actual token availability.
+//
+// A zero ExpiresAt (time.Time{}) is treated as "no expiration" — some authorization servers
+// (e.g. Atlassian's MCP endpoint) issue access tokens without an `expires_in` field, which
+// the oauth2 library deserializes as the Go zero time. Returning isExpired=true for those
+// records would make the upstream connection loop forever asking for re-auth even though a
+// valid token is on disk. This mirrors HasValidToken (see below), which already treats
+// IsZero() as "never expires".
 func HasPersistedToken(serverName, serverURL string, boltStorage *storage.BoltDB) (hasToken bool, hasRefreshToken bool, isExpired bool) {
 	if boltStorage == nil {
 		return false, false, false
@@ -113,7 +120,11 @@ func HasPersistedToken(serverName, serverURL string, boltStorage *storage.BoltDB
 
 	hasToken = record.AccessToken != ""
 	hasRefreshToken = record.RefreshToken != ""
-	isExpired = time.Now().After(record.ExpiresAt)
+	if record.ExpiresAt.IsZero() {
+		isExpired = false
+	} else {
+		isExpired = time.Now().After(record.ExpiresAt)
+	}
 	return
 }
 

internal/oauth/config_test.go

@@ -271,6 +271,78 @@ func TestTokenStoreManager_HasValidToken_PersistentStore_NoExpiration(t *testing
 	assert.True(t, result, "Expected true for token with no expiration (zero time)")
 }
 
+// TestHasPersistedToken_ZeroExpiresAt validates that a persisted token with no expiration
+// (time.Time zero value) is reported as NOT expired. Some authorization servers (e.g. the
+// Atlassian MCP endpoint) omit `expires_in` from /token responses, which the oauth2
+// library deserializes as ExpiresAt = time.Time{}. Treating that as "expired" would loop
+// the upstream connection forever asking for re-auth even though a valid access token is
+// on disk. This must match HasValidToken's IsZero() == valid semantics.
+func TestHasPersistedToken_ZeroExpiresAt(t *testing.T) {
+	db := setupTestStorage(t)
+
+	serverName := "Atlassian"
+	serverURL := "https://mcp.atlassian.com/v1/mcp"
+	serverKey := GenerateServerKey(serverName, serverURL)
+
+	record := &storage.OAuthTokenRecord{
+		ServerName:  serverKey,
+		AccessToken: "opaque-access-token-without-expiry",
+		TokenType:   "Bearer",
+		ExpiresAt:   time.Time{}, // No expires_in returned by the auth server
+	}
+	require.NoError(t, db.SaveOAuthToken(record))
+
+	hasToken, hasRefresh, isExpired := HasPersistedToken(serverName, serverURL, db)
+
+	assert.True(t, hasToken, "Expected hasToken=true for a record with a non-empty access token")
+	assert.False(t, hasRefresh, "Expected hasRefresh=false for a record without a refresh token")
+	assert.False(t, isExpired, "Expected isExpired=false for a record with zero ExpiresAt (no expiration)")
+}
+
+// TestHasPersistedToken_FutureExpiresAt sanity-checks that a normal future expiry is
+// reported as not expired (regression guard alongside the zero-expiry test above).
+func TestHasPersistedToken_FutureExpiresAt(t *testing.T) {
+	db := setupTestStorage(t)
+
+	serverName := "future-server"
+	serverURL := "https://example.com/mcp"
+	serverKey := GenerateServerKey(serverName, serverURL)
+
+	require.NoError(t, db.SaveOAuthToken(&storage.OAuthTokenRecord{
+		ServerName:   serverKey,
+		AccessToken:  "valid-token",
+		RefreshToken: "valid-refresh",
+		TokenType:    "Bearer",
+		ExpiresAt:    time.Now().Add(1 * time.Hour),
+	}))
+
+	hasToken, hasRefresh, isExpired := HasPersistedToken(serverName, serverURL, db)
+	assert.True(t, hasToken)
+	assert.True(t, hasRefresh)
+	assert.False(t, isExpired)
+}
+
+// TestHasPersistedToken_PastExpiresAt sanity-checks that an actually-expired token is
+// still reported as expired (we only want IsZero() to be the special case).
+func TestHasPersistedToken_PastExpiresAt(t *testing.T) {
+	db := setupTestStorage(t)
+
+	serverName := "past-server"
+	serverURL := "https://example.com/mcp"
+	serverKey := GenerateServerKey(serverName, serverURL)
+
+	require.NoError(t, db.SaveOAuthToken(&storage.OAuthTokenRecord{
+		ServerName:  serverKey,
+		AccessToken: "expired-token",
+		TokenType:   "Bearer",
+		ExpiresAt:   time.Now().Add(-1 * time.Hour),
+	}))
+
+	hasToken, _, isExpired := HasPersistedToken(serverName, serverURL, db)
+	assert.True(t, hasToken)
+	assert.True(t, isExpired, "Expected isExpired=true for a record with a past ExpiresAt")
+}
+
 // TestTokenStoreManager_HasValidToken_NilStorage validates graceful handling of nil storage
 func TestTokenStoreManager_HasValidToken_NilStorage(t *testing.T) {
 	manager := &TokenStoreManager{

internal/oauth/persistent_token_store.go

@@ -99,25 +99,39 @@ func (p *PersistentTokenStore) GetToken(ctx context.Context) (*client.Token, err
 	}
 
 	now := time.Now()
-	timeUntilExpiry := record.ExpiresAt.Sub(now)
-	isExpired := now.After(record.ExpiresAt)
-	needsRefresh := timeUntilExpiry < TokenRefreshGracePeriod
+	// A zero ExpiresAt means the authorization server did not return an `expires_in`
+	// claim. Treat such tokens as never-expiring (matches Go's oauth2 convention and
+	// HasValidToken/HasPersistedToken) instead of perpetually-expired.
+	hasExpiry := !record.ExpiresAt.IsZero()
+	timeUntilExpiry := time.Duration(0)
+	isExpired := false
+	needsRefresh := false
+	if hasExpiry {
+		timeUntilExpiry = record.ExpiresAt.Sub(now)
+		isExpired = now.After(record.ExpiresAt)
+		needsRefresh = timeUntilExpiry < TokenRefreshGracePeriod
+	}
 
 	// Log token status for debugging
-	if isExpired {
+	switch {
+	case !hasExpiry:
+		p.logger.Debug("✅ OAuth token has no expiration, treating as long-lived",
+			zap.String("server_key", p.serverKey),
+			zap.Bool("has_refresh_token", record.RefreshToken != ""))
+	case isExpired:
 		p.logger.Warn("⚠️ OAuth token has expired and needs refresh",
 			zap.String("server_key", p.serverKey),
 			zap.Time("expires_at", record.ExpiresAt),
 			zap.Duration("expired_since", -timeUntilExpiry),
 			zap.Bool("has_refresh_token", record.RefreshToken != ""))
-	} else if needsRefresh {
+	case needsRefresh:
 		p.logger.Info("⏰ OAuth token will expire soon, proactive refresh recommended",
 			zap.String("server_key", p.serverKey),
 			zap.Time("expires_at", record.ExpiresAt),
 			zap.Duration("time_until_expiry", timeUntilExpiry),
 			zap.Duration("grace_period", TokenRefreshGracePeriod),
 			zap.Bool("has_refresh_token", record.RefreshToken != ""))
-	} else {
+	default:
 		p.logger.Debug("✅ OAuth token is valid and not expiring soon",
 			zap.String("server_key", p.serverKey),
 			zap.Time("expires_at", record.ExpiresAt),