fix(store): bound prRefreshSequences with an LRU cap (#5792)

* fix(store): bound prRefreshSequences with an LRU cap

prRefreshSequences (Record<cacheKey, number>) is keyed by PR cache key
(execution-host/repo/branch). applyGitHubPRRefreshEvent only ever wrote entries
and never removed them, so the map grew monotonically with the number of distinct
(host, repo, branch) tuples observed over a session — branches are ephemeral and
unbounded. The sibling prRefreshStates is pruned on settle, but prRefreshSequences
was not (its retained entries back the out-of-order sequence guard, so delete-on-
settle is unsafe).

Cap it to MAX_CACHE_ENTRIES, evicting the oldest-touched keys; the writer moves
each touched key to the most-recent position so active branches aren't evicted.
An evicted long-idle branch simply restarts sequence comparison from 0.

Regression test fails before the fix (map grows past the cap) and passes after.

Co-authored-by: Orca <help@stably.ai>

* test(store): cover the move-to-end behavior of the prRefreshSequences cap

Review follow-up: the prior tests passed even if the delete-then-set move-to-end
were removed. Add a test that refreshes the oldest key and asserts it survives
capping (evicting the next-oldest instead) — fails without the move-to-end.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/store/slices/github-pr-refresh-sequences-leak.test.ts

@@ -0,0 +1,117 @@
+/**
+ * Memory-leak regression: prRefreshSequences must stay bounded.
+ *
+ * `prRefreshSequences` is a Record keyed by PR cache key (repo/branch/execution
+ * host). `applyGitHubPRRefreshEvent` only ever wrote entries and never removed
+ * them, so the map grew monotonically with the number of distinct (host, repo,
+ * branch) tuples observed — branches are ephemeral and unbounded over a long
+ * session. The fix caps it to MAX_CACHE_ENTRIES, evicting the oldest-touched
+ * keys (the writer moves each touched key to the most-recent position).
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { create } from 'zustand'
+import { createGitHubSlice } from './github'
+import { createHostedReviewSlice } from './hosted-review'
+import type { AppState } from '../types'
+
+// MAX_CACHE_ENTRIES is module-private; mirror its value here.
+const MAX_CACHE_ENTRIES = 500
+
+const mockApi = {
+  gh: {
+    prForBranch: vi.fn().mockResolvedValue(null),
+    refreshPRNow: vi.fn(),
+    enqueuePRRefresh: vi.fn().mockResolvedValue(undefined),
+    issue: vi.fn().mockResolvedValue(null),
+    prChecks: vi.fn().mockResolvedValue([])
+  },
+  hostedReview: { forBranch: vi.fn().mockResolvedValue(null) },
+  runtimeEnvironments: { call: vi.fn() },
+  cache: {
+    getGitHub: vi.fn().mockResolvedValue(null),
+    setGitHub: vi.fn().mockResolvedValue(undefined)
+  }
+}
+
+// @ts-expect-error -- minimal window.api stub for the slice under test
+globalThis.window = { api: mockApi }
+
+function createTestStore() {
+  return create<AppState>()(
+    (...a) =>
+      ({
+        ...createGitHubSlice(...a),
+        ...createHostedReviewSlice(...a)
+      }) as AppState
+  )
+}
+
+describe('prRefreshSequences stays bounded (leak regression)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('caps prRefreshSequences and keeps the most recently touched key', () => {
+    const store = createTestStore()
+
+    // Seed more sequence entries than the cap allows.
+    const seeded: Record<string, number> = {}
+    const seedCount = MAX_CACHE_ENTRIES + 100
+    for (let i = 0; i < seedCount; i++) {
+      seeded[`seed-${i}`] = 1
+    }
+    store.setState({ prRefreshSequences: seeded })
+
+    // One more refresh event for a brand-new PR cache key pushes over the cap.
+    store.getState().applyGitHubPRRefreshEvent({
+      sequence: 1,
+      reason: 'visible',
+      status: 'in-flight',
+      aliases: [{ cacheKey: 'key-new', repoPath: '/repo/new', branch: 'branch-new' }]
+    })
+
+    const sequences = store.getState().prRefreshSequences
+    // Bounded — not seedCount + 1.
+    expect(Object.keys(sequences)).toHaveLength(MAX_CACHE_ENTRIES)
+    // The just-touched key survives; the oldest seeded key is evicted.
+    expect(sequences['key-new']).toBe(1)
+    expect(sequences['seed-0']).toBeUndefined()
+  })
+
+  it('does not evict anything while under the cap', () => {
+    const store = createTestStore()
+    store.getState().applyGitHubPRRefreshEvent({
+      sequence: 3,
+      reason: 'visible',
+      status: 'in-flight',
+      aliases: [{ cacheKey: 'only-key', repoPath: '/repo', branch: 'b' }]
+    })
+    expect(store.getState().prRefreshSequences['only-key']).toBe(3)
+  })
+
+  it('keeps a refreshed older key by moving it to most-recent before capping', () => {
+    const store = createTestStore()
+    const seeded: Record<string, number> = {}
+    const seedCount = MAX_CACHE_ENTRIES + 100
+    for (let i = 0; i < seedCount; i++) {
+      seeded[`seed-${i}`] = 1
+    }
+    store.setState({ prRefreshSequences: seeded })
+
+    // Refresh the OLDEST key. The writer moves it to most-recent (delete+set),
+    // so capping must evict the next-oldest keys, not this freshly-touched one.
+    store.getState().applyGitHubPRRefreshEvent({
+      sequence: 9,
+      reason: 'visible',
+      status: 'in-flight',
+      aliases: [{ cacheKey: 'seed-0', repoPath: '/repo/0', branch: 'branch-0' }]
+    })
+
+    const sequences = store.getState().prRefreshSequences
+    expect(Object.keys(sequences)).toHaveLength(MAX_CACHE_ENTRIES)
+    // Survives with its updated sequence; without move-to-end it would be evicted.
+    expect(sequences['seed-0']).toBe(9)
+    // The next-oldest key is the one evicted instead.
+    expect(sequences['seed-1']).toBeUndefined()
+  })
+})

src/renderer/src/store/slices/github.ts

@@ -1365,6 +1365,27 @@ function withBoundedCacheEntry<T extends { fetchedAt: number }>(
   return evictStaleEntries({ ...cache, [key]: entry })
 }
 
+// Why: prRefreshSequences only ever grows — one entry per PR cache key
+// (repo/branch/execution-host) ever observed, and branches are ephemeral and
+// unbounded over a long session. It has no `fetchedAt` to sort by, so bound it
+// by insertion order (oldest-touched keys evicted first; the writer moves each
+// touched key to the end). An evicted long-idle branch simply restarts sequence
+// comparison from 0, which is acceptable.
+function capPrRefreshSequences(
+  sequences: Record<string, number>,
+  maxEntries = MAX_CACHE_ENTRIES
+): Record<string, number> {
+  const keys = Object.keys(sequences)
+  if (keys.length <= maxEntries) {
+    return sequences
+  }
+  const capped: Record<string, number> = {}
+  for (const key of keys.slice(keys.length - maxEntries)) {
+    capped[key] = sequences[key]
+  }
+  return capped
+}
+
 function shouldRefreshIssueDecorations(state: AppState): boolean {
   return (state.worktreeCardProperties ?? []).includes('issue')
 }
@@ -3361,6 +3382,9 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
           }
           continue
         }
+        // Why: delete-then-set moves this key to the end of insertion order so
+        // capPrRefreshSequences evicts genuinely idle keys, not active ones.
+        delete nextSequences[alias.cacheKey]
         nextSequences[alias.cacheKey] = event.sequence
         changed = true
 
@@ -3510,7 +3534,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
 
       return changed
         ? {
-            prRefreshSequences: nextSequences,
+            prRefreshSequences: capPrRefreshSequences(nextSequences),
             prRefreshStates: nextStates,
             prCache: nextPRCache,
             hostedReviewCache: nextHostedReviewCache