Make RepoAccessCache a singleton (#1426)

* Initial plan

* Implement RepoAccessCache as a singleton pattern

Co-authored-by: JoannaaKL <[email protected]>

* Complete singleton implementation and verification

Co-authored-by: JoannaaKL <[email protected]>

* Remove cacheIDCounter as requested

Co-authored-by: JoannaaKL <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: JoannaaKL <[email protected]>

Author: Copilot

go.sum

@@ -1,4 +1,3 @@
-cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k=
 github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
 github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
 github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPnH1Wvgk=
@@ -95,7 +94,6 @@ github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3A
 github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU=
 github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
@@ -110,23 +108,18 @@ github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82 h1:BHyfKlQyqbsFN5p3Ifn
 github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82/go.mod h1:lgjkn3NuSvDfVJdfcVVdX+jpBxNmX4rDAzaS45IcYoM=
 go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc=
 go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
-golang.org/x/crypto v0.36.0/go.mod h1:Y4J0ReaxCR1IMaabaSMugxJES1EpwhBHhv2bDHklZvc=
 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8=
 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY=
-golang.org/x/mod v0.26.0/go.mod h1:/j6NAhSk8iQ723BGAUyoAcn7SlD7s15Dp9Nd/SfeaFQ=
 golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8=
 golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8=
 golang.org/x/oauth2 v0.29.0 h1:WdYw2tdTK1S8olAzWHdgeqfy+Mtm9XNhv/xJsY65d98=
 golang.org/x/oauth2 v0.29.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8=
-golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
 golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik=
 golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
-golang.org/x/term v0.30.0/go.mod h1:NYYFdzHoI5wRh/h5tDMdMqCqPJZEuNqVR5xJLd/n67g=
 golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng=
 golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU=
 golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
 golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
-golang.org/x/tools v0.35.0/go.mod h1:NKdj5HkL/73byiZSJjqJgKn3ep7KjFkBOkR/Hps3VPw=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

internal/ghmcp/server.go

@@ -90,7 +90,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
 	}
 	var repoAccessCache *lockdown.RepoAccessCache
 	if cfg.LockdownMode {
-		repoAccessCache = lockdown.NewRepoAccessCache(gqlClient, repoAccessOpts...)
+		repoAccessCache = lockdown.GetInstance(gqlClient, repoAccessOpts...)
 	}
 
 	// When a client send an initialize request, update the user agent to include the client info.

pkg/lockdown/lockdown.go

@@ -29,6 +29,12 @@ type repoAccessCacheEntry struct {
 
 const defaultRepoAccessTTL = 5 * time.Minute
 
+var (
+	instance     *RepoAccessCache
+	instanceOnce sync.Once
+	instanceMu   sync.RWMutex
+)
+
 // RepoAccessOption configures RepoAccessCache at construction time.
 type RepoAccessOption func(*RepoAccessCache)
 
@@ -47,10 +53,43 @@ func WithLogger(logger *slog.Logger) RepoAccessOption {
 	}
 }
 
-// NewRepoAccessCache returns a cache bound to the provided GitHub GraphQL
-// client. The cache is safe for concurrent use.
+// GetInstance returns the singleton instance of RepoAccessCache.
+// It initializes the instance on first call with the provided client and options.
+// Subsequent calls ignore the client and options parameters and return the existing instance.
+// This is the preferred way to access the cache in production code.
+func GetInstance(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessCache {
+	instanceOnce.Do(func() {
+		instance = newRepoAccessCache(client, opts...)
+	})
+	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 {
-	// Use a unique cache name for each instance to avoid sharing state between tests
+	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 {
 	cacheName := "repo-access-cache"
 	c := &RepoAccessCache{
 		client: client,

pkg/lockdown/lockdown_test.go

@@ -145,3 +145,58 @@ func TestRepoAccessCacheSetTTLReschedulesExistingEntry(t *testing.T) {
 	requireAccess(ctx, t, cache)
 	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)
+}