diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index 4a47fefe1a1..19cafff266c 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -4,6 +4,8 @@ GitHub slice's cross-cutting invariants verifiable in one place. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { create } from 'zustand' import { + _clearGitHubPRRefreshStartedEntriesForTest, + _getGitHubPRRefreshStartedEntryCountForTest, _getGitHubPRRequestGenerationCountForTest, createGitHubSlice, mergePRCommentIntoList, @@ -1030,6 +1032,11 @@ describe('createGitHubSlice.fetchPRForBranch', () => { mockApi.gh.refreshPRNow.mockReset() mockApi.gh.refreshPRNow.mockResolvedValue({ kind: 'no-pr', fetchedAt: Date.now() }) mockApi.hostedReview.forBranch.mockResolvedValue(null) + _clearGitHubPRRefreshStartedEntriesForTest() + }) + + afterEach(() => { + _clearGitHubPRRefreshStartedEntriesForTest() }) it('lets a forced refresh bypass a non-forced inflight request and keeps the newer result', async () => { @@ -1945,6 +1952,74 @@ describe('createGitHubSlice.fetchPRForBranch', () => { } }) + it('drops request-start hosted-review snapshots when refreshes pause before outcomes', () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/rate-limit-pause' + const cacheKey = `${repoId}::${branch}` + const hostedReviewCacheKey = getHostedReviewCacheKey(repoPath, branch, null, repoId) + + store.setState({ + hostedReviewCache: { + [hostedReviewCacheKey]: { + data: { + provider: 'github', + number: 12, + title: 'Existing PR', + state: 'open', + url: 'https://github.com/acme/orca/pull/12', + status: 'pending', + updatedAt: '2026-03-28T00:00:00Z', + mergeable: 'UNKNOWN' + }, + fetchedAt: 100, + linkedReviewHintKey: 'github:12' + } + } + } as unknown as Partial) + + for (let i = 0; i < 40; i += 1) { + const inFlightSequence = i * 2 + 1 + store.getState().applyGitHubPRRefreshEvent({ + sequence: inFlightSequence, + aliases: [{ cacheKey, repoId, repoPath, branch }], + reason: 'visible', + requestStartedAt: Date.now(), + status: 'in-flight' + }) + expect(_getGitHubPRRefreshStartedEntryCountForTest()).toBe(1) + + store.getState().applyGitHubPRRefreshEvent({ + sequence: inFlightSequence + 1, + aliases: [{ cacheKey, repoId, repoPath, branch }], + reason: 'visible', + status: 'paused', + pausedUntil: Date.now() + 60_000, + skippedReason: 'rate-limit' + }) + expect(_getGitHubPRRefreshStartedEntryCountForTest()).toBe(0) + } + }) + + it('does not retain empty request-start entries for PR refreshes without a hosted-review cache entry', () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/no-hosted-review' + const cacheKey = `${repoId}::${branch}` + + store.getState().applyGitHubPRRefreshEvent({ + sequence: 1, + aliases: [{ cacheKey, repoId, repoPath, branch }], + reason: 'visible', + requestStartedAt: Date.now(), + status: 'in-flight' + }) + + expect(_getGitHubPRRefreshStartedEntryCountForTest()).toBe(0) + }) + it('does not overwrite a non-GitHub hosted review from GitHub PR refresh events', () => { const store = createTestStore() const repoPath = '/repo' diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index b87bff32f69..c5a7eeb0ea1 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -364,12 +364,23 @@ const prRefreshStartedHostedReviewEntries = new Map< string, AppState['hostedReviewCache'][string] | undefined >() +const PR_REFRESH_STARTED_HOSTED_REVIEW_ENTRY_MAX = 128 /** @internal - exposed for leak-regression tests only */ export function _getGitHubPRRequestGenerationCountForTest(): number { return prRequestGenerations.size } +/** @internal - exposed for leak-regression tests only */ +export function _getGitHubPRRefreshStartedEntryCountForTest(): number { + return prRefreshStartedHostedReviewEntries.size +} + +/** @internal - exposed for leak-regression tests only */ +export function _clearGitHubPRRefreshStartedEntriesForTest(): void { + prRefreshStartedHostedReviewEntries.clear() +} + // Why: cap in-flight cross-repo fan-out and hover-prefetches at the renderer // boundary — the main-side gate is behind the IPC queue, so it can't see a // stampede until the calls are already mid-flight. 8 balances responsiveness @@ -830,6 +841,41 @@ function prRefreshStartedEntryKey(sequence: number, cacheKey: string): string { return `${sequence}::${cacheKey}` } +function deletePRRefreshStartedEntry(sequence: number | undefined, cacheKey: string): void { + if (sequence !== undefined && sequence > 0) { + prRefreshStartedHostedReviewEntries.delete(prRefreshStartedEntryKey(sequence, cacheKey)) + } +} + +function setPRRefreshStartedHostedReviewEntry( + key: string, + entry: AppState['hostedReviewCache'][string] | undefined +): void { + if (entry === undefined) { + prRefreshStartedHostedReviewEntries.delete(key) + return + } + prRefreshStartedHostedReviewEntries.delete(key) + prRefreshStartedHostedReviewEntries.set(key, entry) + while (prRefreshStartedHostedReviewEntries.size > PR_REFRESH_STARTED_HOSTED_REVIEW_ENTRY_MAX) { + const oldest = prRefreshStartedHostedReviewEntries.keys().next() + if (oldest.done) { + return + } + prRefreshStartedHostedReviewEntries.delete(oldest.value) + } +} + +function deletePRRefreshStartedEntriesForEvent( + event: GitHubPRRefreshEvent, + sequences: AppState['prRefreshSequences'] +): void { + for (const alias of event.aliases) { + deletePRRefreshStartedEntry(event.sequence, alias.cacheKey) + deletePRRefreshStartedEntry(sequences[alias.cacheKey], alias.cacheKey) + } +} + function setGitHubPRResultCaches( state: AppState, args: { @@ -2605,6 +2651,7 @@ export const createGitHubSlice: StateCreator = (s // Why: local main-process refresh events are keyed only by repo/branch; // applying them while a runtime is active can leak local PR state into SSH. if (getActiveRuntimeTarget(s.settings).kind === 'environment') { + deletePRRefreshStartedEntriesForEvent(event, s.prRefreshSequences) return {} } const nextSequences = { ...s.prRefreshSequences } @@ -2618,6 +2665,9 @@ export const createGitHubSlice: StateCreator = (s if ( event.outcome ? event.sequence < previousSequence : event.sequence <= previousSequence ) { + if (event.outcome || event.status !== 'in-flight') { + deletePRRefreshStartedEntry(event.sequence, alias.cacheKey) + } continue } nextSequences[alias.cacheKey] = event.sequence @@ -2627,6 +2677,9 @@ export const createGitHubSlice: StateCreator = (s const startedEntryKey = prRefreshStartedEntryKey(event.sequence, alias.cacheKey) const requestStartedEntry = prRefreshStartedHostedReviewEntries.get(startedEntryKey) prRefreshStartedHostedReviewEntries.delete(startedEntryKey) + if (previousSequence !== event.sequence) { + deletePRRefreshStartedEntry(previousSequence, alias.cacheKey) + } delete nextStates[alias.cacheKey] if (event.outcome.kind === 'upstream-error') { nextStates[alias.cacheKey] = { @@ -2700,6 +2753,9 @@ export const createGitHubSlice: StateCreator = (s } if (event.status) { + if (previousSequence !== event.sequence) { + deletePRRefreshStartedEntry(previousSequence, alias.cacheKey) + } if (event.status === 'in-flight' && event.requestStartedAt !== undefined) { const hostedReviewCacheKey = getHostedReviewCacheKey( alias.repoPath, @@ -2708,10 +2764,15 @@ export const createGitHubSlice: StateCreator = (s alias.repoId, alias.connectionId ) - prRefreshStartedHostedReviewEntries.set( + setPRRefreshStartedHostedReviewEntry( prRefreshStartedEntryKey(event.sequence, alias.cacheKey), s.hostedReviewCache[hostedReviewCacheKey] ) + } else { + // Why: rate-limit pauses/skips can follow an in-flight broadcast + // without an outcome; the cached request-start snapshot is no + // longer live and would otherwise accumulate per refresh sequence. + deletePRRefreshStartedEntry(event.sequence, alias.cacheKey) } nextStates[alias.cacheKey] = { status: event.status,