diff --git a/go.sum b/go.sum index d2e323cf6..0ff7b51fa 100644 --- a/go.sum +++ b/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= diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index f82fa0553..15b1efc10 100644 --- a/internal/ghmcp/server.go +++ b/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. diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go index 8fb9be08d..368290811 100644 --- a/pkg/lockdown/lockdown.go +++ b/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, diff --git a/pkg/lockdown/lockdown_test.go b/pkg/lockdown/lockdown_test.go index 8e4ac548c..145d3d629 100644 --- a/pkg/lockdown/lockdown_test.go +++ b/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) +}