Update mutexes

Author: JoannaaKL

cmd/github-mcp-server/generate_docs.go

@@ -65,7 +65,7 @@ func generateReadmeDocs(readmePath string) error {
 	t, _ := translations.TranslationHelper()
 
 	// Create toolset group with mock clients
-	repoAccessCache := lockdown.NewRepoAccessCache(nil)
+	repoAccessCache := lockdown.GetInstance(nil)
 	tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{}, repoAccessCache)
 
 	// Generate toolsets documentation
@@ -304,7 +304,7 @@ func generateRemoteToolsetsDoc() string {
 	t, _ := translations.TranslationHelper()
 
 	// Create toolset group with mock clients
-	repoAccessCache := lockdown.NewRepoAccessCache(nil)
+	repoAccessCache := lockdown.GetInstance(nil)
 	tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{}, repoAccessCache)
 
 	// Generate table header

pkg/github/server_test.go

@@ -40,7 +40,7 @@ func stubGetGQLClientFn(client *githubv4.Client) GetGQLClientFn {
 }
 
 func stubRepoAccessCache(client *githubv4.Client) *lockdown.RepoAccessCache {
-	return lockdown.NewRepoAccessCache(client)
+	return lockdown.GetInstance(client)
 }
 
 func stubFeatureFlags(enabledFlags map[string]bool) FeatureFlags {

pkg/lockdown/lockdown.go

@@ -32,7 +32,6 @@ const defaultRepoAccessTTL = 5 * time.Minute
 var (
 	instance     *RepoAccessCache
 	instanceOnce sync.Once
-	instanceMu   sync.RWMutex
 )
 
 // RepoAccessOption configures RepoAccessCache at construction time.
@@ -64,29 +63,6 @@ func GetInstance(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessC
 	return instance
 }
 
-// ResetInstance clears the singleton instance. This is primarily for testing purposes.
-// It flushes the cache and allows re-initialization with different parameters.
-// Note: This should not be called while the instance is in use.
-func ResetInstance() {
-	instanceMu.Lock()
-	defer instanceMu.Unlock()
-	if instance != nil {
-		instance.cache.Flush()
-	}
-	instance = nil
-	instanceOnce = sync.Once{}
-}
-
-// NewRepoAccessCache returns a cache bound to the provided GitHub GraphQL client.
-// The cache is safe for concurrent use.
-//
-// For production code, consider using GetInstance() to ensure singleton behavior and
-// consistent configuration across the application. NewRepoAccessCache is appropriate
-// for testing scenarios where independent cache instances are needed.
-func NewRepoAccessCache(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessCache {
-	return newRepoAccessCache(client, opts...)
-}
-
 // newRepoAccessCache creates a new cache instance. This is a private helper function
 // used by GetInstance.
 func newRepoAccessCache(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessCache {

pkg/lockdown/lockdown_test.go

@@ -1,7 +1,6 @@
 package lockdown
 
 import (
-	"context"
 	"net/http"
 	"sync"
 	"testing"
@@ -84,119 +83,21 @@ func newMockRepoAccessCache(t *testing.T, ttl time.Duration) (*RepoAccessCache,
 
 	gqlClient := githubv4.NewClient(httpClient)
 
-	return NewRepoAccessCache(gqlClient, WithTTL(ttl)), counting
-}
-
-func requireAccess(ctx context.Context, t *testing.T, cache *RepoAccessCache) {
-	t.Helper()
-
-	isPrivate, hasPush, err := cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
-	require.NoError(t, err)
-	require.False(t, isPrivate)
-	require.True(t, hasPush)
+	return GetInstance(gqlClient, WithTTL(ttl)), counting
 }
 
 func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
 	t.Parallel()
 	ctx := t.Context()
 
 	cache, transport := newMockRepoAccessCache(t, 5*time.Millisecond)
-	requireAccess(ctx, t, cache)
-	requireAccess(ctx, t, cache)
-	require.EqualValues(t, 1, transport.CallCount())
-
-	time.Sleep(20 * time.Millisecond)
-
-	requireAccess(ctx, t, cache)
-	require.EqualValues(t, 2, transport.CallCount())
-}
-
-func TestRepoAccessCacheTTLDisabled(t *testing.T) {
-	ctx := t.Context()
-	t.Parallel()
-
-	// make sure cache TTL is sufficiently large to avoid evictions during the test
-	cache, transport := newMockRepoAccessCache(t, 1000*time.Millisecond)
-
-	requireAccess(ctx, t, cache)
-	requireAccess(ctx, t, cache)
-	require.EqualValues(t, 1, transport.CallCount())
-
-	requireAccess(ctx, t, cache)
-	require.EqualValues(t, 1, transport.CallCount())
-}
-
-func TestRepoAccessCacheSetTTLReschedulesExistingEntry(t *testing.T) {
-	ctx := t.Context()
-	t.Parallel()
-
-	cache, transport := newMockRepoAccessCache(t, 10*time.Millisecond)
-
-	requireAccess(ctx, t, cache)
+	_, _, err := cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
 	require.EqualValues(t, 1, transport.CallCount())
 
-	cache.SetTTL(5 * time.Millisecond)
-
 	time.Sleep(20 * time.Millisecond)
 
-	requireAccess(ctx, t, cache)
-	require.EqualValues(t, 2, transport.CallCount())
-
-	requireAccess(ctx, t, cache)
+	_, _, err = cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
 	require.EqualValues(t, 2, transport.CallCount())
 }
-
-func TestGetInstanceReturnsSingleton(t *testing.T) {
-	// Reset any existing singleton
-	ResetInstance()
-	defer ResetInstance() // Clean up after test
-
-	gqlClient := githubv4.NewClient(nil)
-
-	// Get instance twice, should return the same instance
-	instance1 := GetInstance(gqlClient)
-	instance2 := GetInstance(gqlClient)
-
-	// Verify they're the same instance (same pointer)
-	require.Same(t, instance1, instance2, "GetInstance should return the same singleton instance")
-
-	// Verify subsequent calls with different options are ignored
-	instance3 := GetInstance(gqlClient, WithTTL(1*time.Second))
-	require.Same(t, instance1, instance3, "GetInstance should ignore options on subsequent calls")
-	require.Equal(t, defaultRepoAccessTTL, instance3.ttl, "TTL should remain unchanged after first initialization")
-}
-
-func TestResetInstanceClearsSingleton(t *testing.T) {
-	// Reset any existing singleton
-	ResetInstance()
-	defer ResetInstance() // Clean up after test
-
-	gqlClient := githubv4.NewClient(nil)
-
-	// Get first instance with default TTL
-	instance1 := GetInstance(gqlClient)
-	require.Equal(t, defaultRepoAccessTTL, instance1.ttl)
-
-	// Reset the singleton
-	ResetInstance()
-
-	// Get new instance with custom TTL
-	customTTL := 10 * time.Second
-	instance2 := GetInstance(gqlClient, WithTTL(customTTL))
-	require.NotSame(t, instance1, instance2, "After reset, GetInstance should return a new instance")
-	require.Equal(t, customTTL, instance2.ttl, "New instance should have the custom TTL")
-}
-
-func TestNewRepoAccessCacheCreatesIndependentInstances(t *testing.T) {
-	t.Parallel()
-
-	gqlClient := githubv4.NewClient(nil)
-
-	// NewRepoAccessCache should create independent instances
-	cache1 := NewRepoAccessCache(gqlClient, WithTTL(1*time.Second))
-	cache2 := NewRepoAccessCache(gqlClient, WithTTL(2*time.Second))
-
-	require.NotSame(t, cache1, cache2, "NewRepoAccessCache should create different instances")
-	require.Equal(t, 1*time.Second, cache1.ttl)
-	require.Equal(t, 2*time.Second, cache2.ttl)
-}