From 94205f215006f40ec92caf0912ccd1c754e8e123 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 15:26:32 +0000 Subject: [PATCH 1/6] Initial plan From 9dfed48b995b48e49f2fa789b72ad55ffb375512 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 15:48:29 +0000 Subject: [PATCH 2/6] refactor: replace filteredServerCache with generic syncutil.TTLCache Add TTLCache[K, V] to internal/syncutil with TTL eviction and LRU-cap, then replace the hand-rolled filteredServerCache in server/routed.go with the new type. Move truncateCacheKeyForLog to session_util.go and migrate cache-specific tests to syncutil/ttl_cache_test.go. --- internal/server/routed.go | 106 ++----------- internal/server/routed_test.go | 81 ---------- internal/server/session_util.go | 18 ++- internal/syncutil/ttl_cache.go | 98 ++++++++++++ internal/syncutil/ttl_cache_test.go | 223 ++++++++++++++++++++++++++++ 5 files changed, 349 insertions(+), 177 deletions(-) create mode 100644 internal/syncutil/ttl_cache.go create mode 100644 internal/syncutil/ttl_cache_test.go diff --git a/internal/server/routed.go b/internal/server/routed.go index d16df9e86..6e3ba8dc9 100644 --- a/internal/server/routed.go +++ b/internal/server/routed.go @@ -5,12 +5,11 @@ import ( "encoding/json" "fmt" "net/http" - "strings" - "sync" "time" "github.com/github/gh-aw-mcpg/internal/httputil" "github.com/github/gh-aw-mcpg/internal/logger" + "github.com/github/gh-aw-mcpg/internal/syncutil" sdk "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -30,86 +29,11 @@ func rejectIfShutdown(unifiedServer *UnifiedServer, next http.Handler, logNamesp }) } -// filteredServerCacheMaxSize is the maximum number of entries the filteredServerCache -// will hold. When the cache is full, the least-recently-used entry is evicted to make room. +// filteredServerCacheMaxSize is the maximum number of entries the filtered +// server cache will hold. When the cache is full, the least-recently-used entry +// is evicted to make room. const filteredServerCacheMaxSize = 1000 -// filteredServerCache caches filtered server instances per (backend, session) key. -// Entries are evicted after the configured TTL to prevent unbounded memory growth -// in long-running deployments with many sessions. A max-size cap provides an additional -// safety guard against an unbounded number of unique sessions. -type filteredServerCache struct { - servers map[string]*filteredServerEntry - ttl time.Duration - maxSize int - mu sync.RWMutex -} - -type filteredServerEntry struct { - server *sdk.Server - lastUsed time.Time -} - -// newFilteredServerCache creates a new server cache with the given entry TTL. -func newFilteredServerCache(ttl time.Duration) *filteredServerCache { - logRouted.Printf("[CACHE] Creating filtered server cache: ttl=%s, maxSize=%d", ttl, filteredServerCacheMaxSize) - return &filteredServerCache{ - servers: make(map[string]*filteredServerEntry), - ttl: ttl, - maxSize: filteredServerCacheMaxSize, - } -} - -// getOrCreate returns a cached server or creates a new one. -// Expired entries are lazily evicted on each call. When the cache has reached its -// maximum size, the least-recently-used entry is evicted to make room. -func (c *filteredServerCache) getOrCreate(backendID, sessionID string, creator func() *sdk.Server) *sdk.Server { - key := fmt.Sprintf("%s/%s", backendID, sessionID) - now := time.Now() - - c.mu.Lock() - defer c.mu.Unlock() - - // Lazy eviction of expired entries - for k, entry := range c.servers { - if now.Sub(entry.lastUsed) > c.ttl { - logRouted.Printf("[CACHE] Evicting expired server: key=%s (idle %s)", truncateCacheKeyForLog(k), now.Sub(entry.lastUsed).Round(time.Second)) - delete(c.servers, k) - } - } - - if entry, ok := c.servers[key]; ok { - entry.lastUsed = now - logRouted.Printf("[CACHE] Cache hit: key=%s", truncateCacheKeyForLog(key)) - return entry.server - } - - // When at capacity after TTL eviction, evict the least-recently-used entry - // to bound memory growth reliably. This may interrupt an active session for - // the evicted (backend, session) pair, but is preferable to unbounded growth. - if len(c.servers) >= c.maxSize { - lruKey := "" - var lruTime time.Time - first := true - for k, entry := range c.servers { - if first || entry.lastUsed.Before(lruTime) { - lruKey = k - lruTime = entry.lastUsed - first = false - } - } - if lruKey != "" { - logRouted.Printf("[CACHE] Max size reached (%d), evicting LRU entry: key=%s (idle %s)", c.maxSize, truncateCacheKeyForLog(lruKey), now.Sub(lruTime).Round(time.Second)) - delete(c.servers, lruKey) - } - } - - logRouted.Printf("[CACHE] Creating new filtered server: backend=%s, session=%s", backendID, truncateSessionID(sessionID)) - server := creator() - c.servers[key] = &filteredServerEntry{server: server, lastUsed: now} - return server -} - // CreateHTTPServerForRoutedMode creates an HTTP server for routed mode // In routed mode, each backend is accessible at /mcp/ // Multiple routes from the same Authorization header share a session @@ -129,7 +53,8 @@ func CreateHTTPServerForRoutedMode(addr string, unifiedServer *UnifiedServer, ap // TTL matches the SDK SessionTimeout so cache entries expire with sessions. // Long-running agentic workflows (e.g. >30 min GitHub Actions jobs) need this // to be at least as long as the workflow to avoid spurious "session not found" errors. - serverCache := newFilteredServerCache(sessionTimeout) + logRouted.Printf("[CACHE] Creating filtered server cache: ttl=%s, maxSize=%d", sessionTimeout, filteredServerCacheMaxSize) + serverCache := syncutil.NewTTLCache[string, *sdk.Server](sessionTimeout, filteredServerCacheMaxSize) // Create a proxy for each backend server for _, serverID := range allBackends { @@ -143,10 +68,12 @@ func CreateHTTPServerForRoutedMode(addr string, unifiedServer *UnifiedServer, ap return nil } - // Return a cached filtered proxy server for this backend and session - // This ensures the same server instance is reused for all requests in a session + // Return a cached filtered proxy server for this backend and session. + // This ensures the same server instance is reused for all requests in a session. sessionID := SessionIDFromContext(r.Context()) - return serverCache.getOrCreate(backendID, sessionID, func() *sdk.Server { + cacheKey := fmt.Sprintf("%s/%s", backendID, sessionID) + return serverCache.GetOrCreate(cacheKey, func() *sdk.Server { + logRouted.Printf("[CACHE] Creating new filtered server: backend=%s, session=%s", backendID, truncateSessionID(sessionID)) return createFilteredServer(unifiedServer, backendID) }) }, buildDefaultHandlerConfig(unifiedServer, sessionTimeout, defaultHandlerConfigOptions{ @@ -204,14 +131,3 @@ func createFilteredServer(unifiedServer *UnifiedServer, backendID string) *sdk.S return server } - -// truncateCacheKeyForLog returns a log-safe version of a cache key of the form -// "backendID/sessionID" by truncating the session ID portion. -func truncateCacheKeyForLog(key string) string { - backendID, sessionID, found := strings.Cut(key, "/") - if !found { - return key - } - - return fmt.Sprintf("%s/%s", backendID, truncateSessionID(sessionID)) -} diff --git a/internal/server/routed_test.go b/internal/server/routed_test.go index 582fd644b..c94b1734a 100644 --- a/internal/server/routed_test.go +++ b/internal/server/routed_test.go @@ -557,87 +557,6 @@ func TestCreateFilteredServer_EdgeCases(t *testing.T) { }) } -// TestFilteredServerCache_MaxSize verifies that when the cache is at capacity, the -// least-recently-used entry is evicted to make room for a new entry. -func TestFilteredServerCache_MaxSize(t *testing.T) { - assert := assert.New(t) - - ttl := time.Hour - cache := newFilteredServerCache(ttl) - cache.maxSize = 3 // Use a small max for the test - - callCount := 0 - creator := func() *sdk.Server { - callCount++ - return sdk.NewServer(&sdk.Implementation{Name: "test", Version: "1.0"}, &sdk.ServerOptions{}) - } - - // Fill the cache to max capacity - s1 := cache.getOrCreate("backend", "session1", creator) - s2 := cache.getOrCreate("backend", "session2", creator) - s3 := cache.getOrCreate("backend", "session3", creator) - assert.Equal(3, callCount, "Should have created 3 servers") - assert.NotNil(s1) - assert.NotNil(s2) - assert.NotNil(s3) - assert.Len(cache.servers, 3, "Cache should have 3 entries") - - // Manually set lastUsed to ensure deterministic LRU ordering: - // session1 is least recently used, session3 is most recently used. - now := time.Now() - cache.servers["backend/session1"].lastUsed = now.Add(-3 * time.Millisecond) - cache.servers["backend/session2"].lastUsed = now.Add(-2 * time.Millisecond) - cache.servers["backend/session3"].lastUsed = now.Add(-1 * time.Millisecond) - - // Adding a fourth entry should evict the LRU entry (session1) to stay within maxSize - s4 := cache.getOrCreate("backend", "session4", creator) - assert.Equal(4, callCount, "Should have created a 4th server") - assert.NotNil(s4) - assert.Len(cache.servers, 3, "Cache should maintain maxSize by evicting the LRU entry") - - // session1 (LRU) should have been evicted - _, session1Exists := cache.servers["backend/session1"] - assert.False(session1Exists, "session1 (LRU) should have been evicted to make room") - - // session2, session3, session4 should still be present - _, session2Exists := cache.servers["backend/session2"] - assert.True(session2Exists, "session2 should still be cached") - _, session3Exists := cache.servers["backend/session3"] - assert.True(session3Exists, "session3 should still be cached") - _, session4Exists := cache.servers["backend/session4"] - assert.True(session4Exists, "session4 should be cached") -} - -// TestFilteredServerCache_TTLEviction verifies that expired entries are evicted. -func TestFilteredServerCache_TTLEviction(t *testing.T) { - assert := assert.New(t) - - ttl := 100 * time.Millisecond - cache := newFilteredServerCache(ttl) - - callCount := 0 - creator := func() *sdk.Server { - callCount++ - return sdk.NewServer(&sdk.Implementation{Name: "test", Version: "1.0"}, &sdk.ServerOptions{}) - } - - // Add an entry - cache.getOrCreate("backend", "session1", creator) - assert.Equal(1, callCount) - assert.Len(cache.servers, 1) - - // Wait for TTL to expire (use generous margin to avoid CI flakiness) - time.Sleep(200 * time.Millisecond) - - // Next call should evict the expired entry and create a new one - cache.getOrCreate("backend", "session2", creator) - assert.Equal(2, callCount, "Should have created a new server after TTL eviction") - - // session1 should have been evicted during the lazy eviction scan - _, session1Exists := cache.servers["backend/session1"] - assert.False(session1Exists, "Expired session1 should have been evicted") -} - // TestRegisterToolWithoutValidation verifies that tools are registered on the server // and that the wrapped handler forwards calls correctly via in-memory transport. func TestRegisterToolWithoutValidation(t *testing.T) { diff --git a/internal/server/session_util.go b/internal/server/session_util.go index 42cf989a5..b1b4ce1e6 100644 --- a/internal/server/session_util.go +++ b/internal/server/session_util.go @@ -1,6 +1,11 @@ package server -import "github.com/github/gh-aw-mcpg/internal/strutil" +import ( + "fmt" + "strings" + + "github.com/github/gh-aw-mcpg/internal/strutil" +) // truncateSessionID returns a truncated session ID for safe logging (first 8 bytes). // Returns "(none)" for empty session IDs, and appends "..." for truncated values. @@ -10,3 +15,14 @@ func truncateSessionID(sessionID string) string { } return strutil.Truncate(sessionID, 8) } + +// truncateCacheKeyForLog returns a log-safe version of a cache key of the form +// "backendID/sessionID" by truncating the session ID portion. +func truncateCacheKeyForLog(key string) string { + backendID, sessionID, found := strings.Cut(key, "/") + if !found { + return key + } + + return fmt.Sprintf("%s/%s", backendID, truncateSessionID(sessionID)) +} diff --git a/internal/syncutil/ttl_cache.go b/internal/syncutil/ttl_cache.go new file mode 100644 index 000000000..38a1d1369 --- /dev/null +++ b/internal/syncutil/ttl_cache.go @@ -0,0 +1,98 @@ +package syncutil + +import ( + "sync" + "time" +) + +// ttlEntry holds a cached value together with its last-used timestamp for LRU +// tracking. +type ttlEntry[V any] struct { + value V + lastUsed time.Time +} + +// TTLCache is a thread-safe generic get-or-create cache that combines lazy TTL +// eviction with an LRU size cap. +// +// On every [TTLCache.GetOrCreate] call, all entries whose idle time exceeds the +// configured TTL are evicted first. If the cache is still at capacity after TTL +// eviction, the least-recently-used entry is evicted to make room. +type TTLCache[K comparable, V any] struct { + mu sync.Mutex + entries map[K]*ttlEntry[V] + ttl time.Duration + maxSize int + nowFn func() time.Time +} + +// NewTTLCache creates a new TTLCache with the given entry TTL and maximum size. +// Entries idle longer than ttl are evicted lazily; when the cache reaches +// maxSize the least-recently-used entry is evicted on the next GetOrCreate call. +func NewTTLCache[K comparable, V any](ttl time.Duration, maxSize int) *TTLCache[K, V] { + return newTTLCacheWithClock[K, V](ttl, maxSize, time.Now) +} + +// newTTLCacheWithClock creates a TTLCache with an injectable clock function. +// Intended for use in unit tests only. +func newTTLCacheWithClock[K comparable, V any](ttl time.Duration, maxSize int, nowFn func() time.Time) *TTLCache[K, V] { + return &TTLCache[K, V]{ + entries: make(map[K]*ttlEntry[V]), + ttl: ttl, + maxSize: maxSize, + nowFn: nowFn, + } +} + +// GetOrCreate returns the cached value for key. If the key is not present, or +// has been evicted, create is called to produce a new value which is then +// stored and returned. +// +// On each call, all expired entries are lazily evicted before the lookup. If +// the cache has reached its capacity after TTL eviction, the LRU entry is +// removed to make room. +func (c *TTLCache[K, V]) GetOrCreate(key K, create func() V) V { + now := c.nowFn() + c.mu.Lock() + defer c.mu.Unlock() + + // Lazy eviction of expired entries. + for k, entry := range c.entries { + if now.Sub(entry.lastUsed) > c.ttl { + delete(c.entries, k) + } + } + + if entry, ok := c.entries[key]; ok { + entry.lastUsed = now + return entry.value + } + + // LRU eviction when still at capacity after TTL sweep. + if len(c.entries) >= c.maxSize { + var lruKey K + var lruTime time.Time + first := true + for k, entry := range c.entries { + if first || entry.lastUsed.Before(lruTime) { + lruKey = k + lruTime = entry.lastUsed + first = false + } + } + if !first { + delete(c.entries, lruKey) + } + } + + v := create() + c.entries[key] = &ttlEntry[V]{value: v, lastUsed: now} + return v +} + +// Len returns the current number of entries held in the cache. +func (c *TTLCache[K, V]) Len() int { + c.mu.Lock() + defer c.mu.Unlock() + return len(c.entries) +} diff --git a/internal/syncutil/ttl_cache_test.go b/internal/syncutil/ttl_cache_test.go new file mode 100644 index 000000000..d724e7b63 --- /dev/null +++ b/internal/syncutil/ttl_cache_test.go @@ -0,0 +1,223 @@ +// Package syncutil tests – internal test file to allow access to +// newTTLCacheWithClock for deterministic time injection. +package syncutil + +import ( + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeClock is a monotonically-advancing fake clock for deterministic testing. +type fakeClock struct { + mu sync.Mutex + now time.Time +} + +func newFakeClock() *fakeClock { + return &fakeClock{now: time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)} +} + +func (f *fakeClock) Now() time.Time { + f.mu.Lock() + defer f.mu.Unlock() + return f.now +} + +func (f *fakeClock) Advance(d time.Duration) { + f.mu.Lock() + defer f.mu.Unlock() + f.now = f.now.Add(d) +} + +// TestTTLCache_GetOrCreate_ReturnsCachedValue verifies that a cached entry is +// returned without invoking create a second time. +func TestTTLCache_GetOrCreate_ReturnsCachedValue(t *testing.T) { + cache := NewTTLCache[string, int](time.Hour, 100) + + createCount := 0 + first := cache.GetOrCreate("key", func() int { createCount++; return 42 }) + second := cache.GetOrCreate("key", func() int { createCount++; return 99 }) + + assert.Equal(t, 42, first) + assert.Equal(t, 42, second, "cached value should be returned on second call") + assert.Equal(t, 1, createCount, "create should be called exactly once") +} + +// TestTTLCache_GetOrCreate_MissingKey verifies that create is called for a new +// key and the result is stored. +func TestTTLCache_GetOrCreate_MissingKey(t *testing.T) { + cache := NewTTLCache[string, int](time.Hour, 100) + + v := cache.GetOrCreate("key", func() int { return 7 }) + + assert.Equal(t, 7, v) + assert.Equal(t, 1, cache.Len()) +} + +// TestTTLCache_Len_MultipleKeys verifies that each unique key occupies a +// separate entry. +func TestTTLCache_Len_MultipleKeys(t *testing.T) { + cache := NewTTLCache[string, int](time.Hour, 100) + + for _, k := range []string{"a", "b", "c"} { + key := k + cache.GetOrCreate(key, func() int { return len(key) }) + } + + assert.Equal(t, 3, cache.Len()) +} + +// TestTTLCache_TTLEviction verifies that entries older than the TTL are lazily +// evicted on the next GetOrCreate call. +func TestTTLCache_TTLEviction(t *testing.T) { + clk := newFakeClock() + cache := newTTLCacheWithClock[string, int](100*time.Millisecond, 100, clk.Now) + + createCount := 0 + cache.GetOrCreate("key", func() int { createCount++; return 1 }) + require.Equal(t, 1, createCount) + + // Advance past TTL. + clk.Advance(200 * time.Millisecond) + + cache.GetOrCreate("other", func() int { return 99 }) // triggers lazy eviction + assert.Equal(t, 0, cache.Len()-1, "evicted entry should be removed; only 'other' remains") + + // Accessing the evicted key again must create a new entry. + cache.GetOrCreate("key", func() int { createCount++; return 2 }) + assert.Equal(t, 2, createCount, "create must be called again for the evicted key") +} + +// TestTTLCache_TTLEviction_RealTime mirrors the existing TTL eviction test that +// used time.Sleep to ensure the behaviour is preserved. +func TestTTLCache_TTLEviction_RealTime(t *testing.T) { + ttl := 100 * time.Millisecond + cache := NewTTLCache[string, int](ttl, 100) + + createCount := 0 + cache.GetOrCreate("session1", func() int { createCount++; return 1 }) + assert.Equal(t, 1, createCount) + assert.Equal(t, 1, cache.Len()) + + // Wait for TTL to expire (use generous margin to avoid CI flakiness). + time.Sleep(200 * time.Millisecond) + + // Accessing a different key should trigger eviction of session1. + cache.GetOrCreate("session2", func() int { createCount++; return 2 }) + assert.Equal(t, 2, createCount, "create should be called for the new key") + + // After eviction the only remaining entry is session2. + assert.Equal(t, 1, cache.Len(), "expired session1 should have been evicted") +} + +// TestTTLCache_LRUEviction verifies that the least-recently-used entry is +// evicted when the cache reaches its maximum size. +func TestTTLCache_LRUEviction(t *testing.T) { + clk := newFakeClock() + cache := newTTLCacheWithClock[string, int](time.Hour, 3, clk.Now) + + createCount := 0 + creator := func(v int) func() int { return func() int { createCount++; return v } } + + // Insert 3 entries with deterministic timestamps. + cache.GetOrCreate("key1", creator(1)) // lastUsed = T+0 + clk.Advance(time.Millisecond) + cache.GetOrCreate("key2", creator(2)) // lastUsed = T+1ms + clk.Advance(time.Millisecond) + cache.GetOrCreate("key3", creator(3)) // lastUsed = T+2ms + + require.Equal(t, 3, createCount) + require.Equal(t, 3, cache.Len()) + + // Adding a fourth entry must evict key1 (LRU). + clk.Advance(time.Millisecond) + v4 := cache.GetOrCreate("key4", creator(4)) + assert.Equal(t, 4, createCount) + assert.Equal(t, 4, v4) + assert.Equal(t, 3, cache.Len(), "cache should remain at maxSize after LRU eviction") + + // key2, key3, key4 should still be present – verify no extra creates. + cache.GetOrCreate("key2", creator(20)) + cache.GetOrCreate("key3", creator(30)) + cache.GetOrCreate("key4", creator(40)) + assert.Equal(t, 4, createCount, "key2, key3, key4 should still be cached") + + // key1 was evicted; fetching it again must call create. + clk.Advance(time.Millisecond) + cache.GetOrCreate("key1", creator(10)) + assert.Equal(t, 5, createCount, "key1 must be re-created after eviction") +} + +// TestTTLCache_LRU_UpdatesLastUsed verifies that a cache hit refreshes the +// entry's lastUsed timestamp, protecting it from LRU eviction. +func TestTTLCache_LRU_UpdatesLastUsed(t *testing.T) { + clk := newFakeClock() + cache := newTTLCacheWithClock[string, int](time.Hour, 2, clk.Now) + + creator := func(v int) func() int { return func() int { return v } } + + cache.GetOrCreate("key1", creator(1)) // T+0 + clk.Advance(time.Millisecond) + cache.GetOrCreate("key2", creator(2)) // T+1ms + + // Re-access key1 to refresh its lastUsed; now key2 is the LRU. + clk.Advance(time.Millisecond) + v1 := cache.GetOrCreate("key1", creator(99)) // hit – updates lastUsed of key1 + assert.Equal(t, 1, v1, "should return cached value, not 99") + + // Adding key3 should evict key2 (now the LRU). + clk.Advance(time.Millisecond) + createCount := 0 + cache.GetOrCreate("key3", func() int { createCount++; return 3 }) + assert.Equal(t, 1, createCount) + + // key1 must still be cached. + cache.GetOrCreate("key1", func() int { createCount++; return 99 }) + assert.Equal(t, 1, createCount, "key1 should still be cached after its lastUsed was refreshed") + + // key2 must have been evicted. + cache.GetOrCreate("key2", func() int { createCount++; return 99 }) + assert.Equal(t, 2, createCount, "key2 should have been evicted (was LRU)") +} + +// TestTTLCache_ConcurrentAccess verifies that concurrent GetOrCreate calls are +// race-free (run with -race). +func TestTTLCache_ConcurrentAccess(t *testing.T) { + cache := NewTTLCache[int, int](time.Hour, 50) + + var wg sync.WaitGroup + var createCount atomic.Int32 + const goroutines = 100 + + wg.Add(goroutines) + for i := 0; i < goroutines; i++ { + key := i % 10 // intentional key collisions + go func(k int) { + defer wg.Done() + cache.GetOrCreate(k, func() int { createCount.Add(1); return k * 10 }) + }(key) + } + wg.Wait() + + assert.LessOrEqual(t, int32(10), createCount.Load(), "at most 10 unique keys") +} + +// TestTTLCache_MaxSizeOne verifies edge behaviour when maxSize is 1. +func TestTTLCache_MaxSizeOne(t *testing.T) { + clk := newFakeClock() + cache := newTTLCacheWithClock[string, int](time.Hour, 1, clk.Now) + + createCount := 0 + cache.GetOrCreate("a", func() int { createCount++; return 1 }) + assert.Equal(t, 1, cache.Len()) + + clk.Advance(time.Millisecond) + cache.GetOrCreate("b", func() int { createCount++; return 2 }) + assert.Equal(t, 1, cache.Len(), "cache should still hold only 1 entry") + assert.Equal(t, 2, createCount) +} From 6637a52d6af48c7069c69f245b083dae237220e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 15:49:52 +0000 Subject: [PATCH 3/6] fix: correct assertion arguments in ttl_cache_test.go --- internal/syncutil/ttl_cache_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/syncutil/ttl_cache_test.go b/internal/syncutil/ttl_cache_test.go index d724e7b63..146d9754a 100644 --- a/internal/syncutil/ttl_cache_test.go +++ b/internal/syncutil/ttl_cache_test.go @@ -86,7 +86,7 @@ func TestTTLCache_TTLEviction(t *testing.T) { clk.Advance(200 * time.Millisecond) cache.GetOrCreate("other", func() int { return 99 }) // triggers lazy eviction - assert.Equal(t, 0, cache.Len()-1, "evicted entry should be removed; only 'other' remains") + assert.Equal(t, 1, cache.Len(), "evicted entry should be removed; only 'other' remains") // Accessing the evicted key again must create a new entry. cache.GetOrCreate("key", func() int { createCount++; return 2 }) @@ -204,7 +204,7 @@ func TestTTLCache_ConcurrentAccess(t *testing.T) { } wg.Wait() - assert.LessOrEqual(t, int32(10), createCount.Load(), "at most 10 unique keys") + assert.LessOrEqual(t, createCount.Load(), int32(10), "at most 10 unique keys") } // TestTTLCache_MaxSizeOne verifies edge behaviour when maxSize is 1. From 6997ec31651da706fc467509097f13dc4b7622b6 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 10 Jun 2026 08:21:57 -0700 Subject: [PATCH 4/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- internal/syncutil/ttl_cache.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/syncutil/ttl_cache.go b/internal/syncutil/ttl_cache.go index 38a1d1369..95183e506 100644 --- a/internal/syncutil/ttl_cache.go +++ b/internal/syncutil/ttl_cache.go @@ -57,9 +57,11 @@ func (c *TTLCache[K, V]) GetOrCreate(key K, create func() V) V { defer c.mu.Unlock() // Lazy eviction of expired entries. - for k, entry := range c.entries { - if now.Sub(entry.lastUsed) > c.ttl { - delete(c.entries, k) + if c.ttl > 0 { + for k, entry := range c.entries { + if now.Sub(entry.lastUsed) > c.ttl { + delete(c.entries, k) + } } } From c79c84348a6858a1c5d0891d0fa555bca3fea91c Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 10 Jun 2026 08:22:14 -0700 Subject: [PATCH 5/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- internal/syncutil/ttl_cache.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/syncutil/ttl_cache.go b/internal/syncutil/ttl_cache.go index 95183e506..69e2041e3 100644 --- a/internal/syncutil/ttl_cache.go +++ b/internal/syncutil/ttl_cache.go @@ -52,10 +52,12 @@ func newTTLCacheWithClock[K comparable, V any](ttl time.Duration, maxSize int, n // the cache has reached its capacity after TTL eviction, the LRU entry is // removed to make room. func (c *TTLCache[K, V]) GetOrCreate(key K, create func() V) V { + if c.maxSize <= 0 { + return create() + } now := c.nowFn() c.mu.Lock() defer c.mu.Unlock() - // Lazy eviction of expired entries. if c.ttl > 0 { for k, entry := range c.entries { From 3dae7c9dde0ee5f8ebb681a971cf916deb2278cf Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 10 Jun 2026 08:22:27 -0700 Subject: [PATCH 6/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- internal/syncutil/ttl_cache.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/syncutil/ttl_cache.go b/internal/syncutil/ttl_cache.go index 69e2041e3..c6eae157f 100644 --- a/internal/syncutil/ttl_cache.go +++ b/internal/syncutil/ttl_cache.go @@ -51,6 +51,8 @@ func newTTLCacheWithClock[K comparable, V any](ttl time.Duration, maxSize int, n // On each call, all expired entries are lazily evicted before the lookup. If // the cache has reached its capacity after TTL eviction, the LRU entry is // removed to make room. +// +// create is called while the cache lock is held. func (c *TTLCache[K, V]) GetOrCreate(key K, create func() V) V { if c.maxSize <= 0 { return create()