fix: clear stale github pr refresh snapshots (#4187)

This commit is contained in:
Neil
2026-05-31 07:28:48 -07:00
committed by GitHub
parent b0a2a96a53
commit 91d91ef48b
2 changed files with 137 additions and 1 deletions
@@ -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<AppState>)
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'
+62 -1
View File
@@ -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<AppState, [], [], GitHubSlice> = (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<AppState, [], [], GitHubSlice> = (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<AppState, [], [], GitHubSlice> = (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<AppState, [], [], GitHubSlice> = (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<AppState, [], [], GitHubSlice> = (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,