mirror of
https://github.com/stablyai/orca.git
synced 2026-10-12 16:03:03 +00:00
Fix stuck PR refresh queued indicator
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -0,0 +1,129 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitHubPRRefreshCandidate, PRInfo } from '../../shared/types'
|
||||
|
||||
const { sendMock, getAllWebContentsMock, getPRForBranchOutcomeMock, getRateLimitMock } = vi.hoisted(
|
||||
() => ({
|
||||
sendMock: vi.fn(),
|
||||
getAllWebContentsMock: vi.fn(),
|
||||
getPRForBranchOutcomeMock: vi.fn(),
|
||||
getRateLimitMock: vi.fn()
|
||||
})
|
||||
)
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
webContents: {
|
||||
getAllWebContents: getAllWebContentsMock
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('./client', () => ({
|
||||
getPRForBranchOutcome: getPRForBranchOutcomeMock
|
||||
}))
|
||||
|
||||
vi.mock('./rate-limit', () => ({
|
||||
getRateLimit: getRateLimitMock,
|
||||
noteRateLimitSpend: vi.fn(),
|
||||
rateLimitGuard: vi.fn(() => ({ blocked: false }))
|
||||
}))
|
||||
|
||||
function makeCandidate(
|
||||
overrides: Partial<GitHubPRRefreshCandidate> = {}
|
||||
): GitHubPRRefreshCandidate {
|
||||
return {
|
||||
cacheKey: '/repo::feature/test',
|
||||
repoPath: '/repo',
|
||||
branch: 'feature/test',
|
||||
repoKind: 'git',
|
||||
repoId: 'repo-1',
|
||||
worktreeId: 'wt-1',
|
||||
cachedFetchedAt: null,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
function makePR(overrides: Partial<PRInfo> = {}): PRInfo {
|
||||
return {
|
||||
number: 12,
|
||||
title: 'Test PR',
|
||||
state: 'open',
|
||||
url: 'https://github.com/acme/repo/pull/12',
|
||||
checksStatus: 'pending',
|
||||
updatedAt: '2026-05-12T00:00:00Z',
|
||||
mergeable: 'UNKNOWN',
|
||||
headSha: 'head-sha',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('pr-refresh-coordinator', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
vi.useFakeTimers()
|
||||
vi.setSystemTime(1_000)
|
||||
sendMock.mockReset()
|
||||
getAllWebContentsMock.mockReset()
|
||||
getPRForBranchOutcomeMock.mockReset()
|
||||
getRateLimitMock.mockReset()
|
||||
getAllWebContentsMock.mockReturnValue([
|
||||
{
|
||||
id: 1,
|
||||
isDestroyed: () => false,
|
||||
send: sendMock
|
||||
}
|
||||
])
|
||||
getRateLimitMock.mockResolvedValue({ ok: true })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('does not show a delayed visible follow-up as queued after a successful refresh', async () => {
|
||||
const { reportVisiblePRRefreshCandidates } = await import('./pr-refresh-coordinator')
|
||||
getPRForBranchOutcomeMock.mockResolvedValueOnce({
|
||||
kind: 'found',
|
||||
pr: makePR({ checksStatus: 'pending' }),
|
||||
fetchedAt: Date.now()
|
||||
})
|
||||
|
||||
reportVisiblePRRefreshCandidates([makeCandidate()], 1, 1)
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
|
||||
const queuedEvents = sendMock.mock.calls
|
||||
.map(([, event]) => event)
|
||||
.filter((event) => event.status === 'queued')
|
||||
|
||||
expect(queuedEvents).toHaveLength(1)
|
||||
expect(queuedEvents[0].reason).toBe('visible')
|
||||
expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('lets an active worktree refresh bypass a delayed visible follow-up', async () => {
|
||||
const { enqueuePRRefresh, reportVisiblePRRefreshCandidates } =
|
||||
await import('./pr-refresh-coordinator')
|
||||
getPRForBranchOutcomeMock
|
||||
.mockResolvedValueOnce({
|
||||
kind: 'found',
|
||||
pr: makePR({ checksStatus: 'pending' }),
|
||||
fetchedAt: Date.now()
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
kind: 'found',
|
||||
pr: makePR({ checksStatus: 'success' }),
|
||||
fetchedAt: Date.now()
|
||||
})
|
||||
|
||||
const candidate = makeCandidate()
|
||||
reportVisiblePRRefreshCandidates([candidate], 1, 1)
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
enqueuePRRefresh({ ...candidate, cachedFetchedAt: Date.now() }, 'active', 80, 1)
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
|
||||
const inFlightEvents = sendMock.mock.calls
|
||||
.map(([, event]) => event)
|
||||
.filter((event) => event.status === 'in-flight')
|
||||
|
||||
expect(inFlightEvents.map((event) => event.reason)).toEqual(['visible', 'active'])
|
||||
expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
@@ -84,10 +84,18 @@ function isManual(reason: GitHubPRRefreshReason): boolean {
|
||||
return reason === 'manual'
|
||||
}
|
||||
|
||||
function bypassesFreshnessDelay(reason: GitHubPRRefreshReason): boolean {
|
||||
return reason === 'manual' || reason === 'active' || reason === 'post-push'
|
||||
}
|
||||
|
||||
function isBackground(reason: GitHubPRRefreshReason): boolean {
|
||||
return reason !== 'manual'
|
||||
}
|
||||
|
||||
function isBudgetedBackground(reason: GitHubPRRefreshReason): boolean {
|
||||
return reason === 'visible' || reason === 'swr'
|
||||
}
|
||||
|
||||
function validateCandidate(
|
||||
candidate: GitHubPRRefreshCandidate
|
||||
): GitHubPRRefreshEvent['skippedReason'] | null {
|
||||
@@ -113,12 +121,16 @@ function shouldSkipFresh(
|
||||
candidate: GitHubPRRefreshCandidate,
|
||||
reason: GitHubPRRefreshReason
|
||||
): boolean {
|
||||
if (isManual(reason) || candidate.cachedFetchedAt == null) {
|
||||
if (bypassesFreshnessDelay(reason) || candidate.cachedFetchedAt == null) {
|
||||
return false
|
||||
}
|
||||
return Date.now() - candidate.cachedFetchedAt < refreshIntervalForCandidate(candidate)
|
||||
}
|
||||
|
||||
function shouldBroadcastQueued(dueAt: number): boolean {
|
||||
return dueAt - Date.now() <= 5_000
|
||||
}
|
||||
|
||||
function freshRetryAt(candidate: GitHubPRRefreshCandidate): number | null {
|
||||
return candidate.cachedFetchedAt == null
|
||||
? null
|
||||
@@ -166,7 +178,8 @@ function scheduleVisibleFollowUp(
|
||||
dueAt: retryAt,
|
||||
windowId
|
||||
})
|
||||
broadcast({ aliases, reason: 'visible', status: 'queued' })
|
||||
// Why: this is a delayed retry, not active work; showing it as a spinner
|
||||
// makes visible worktrees look stuck until the backoff expires.
|
||||
scheduleDrain(retryAt - Date.now())
|
||||
return
|
||||
}
|
||||
@@ -257,7 +270,7 @@ async function drainQueue(): Promise<void> {
|
||||
return
|
||||
}
|
||||
|
||||
const budgetDelay = isBackground(next.reason) ? nextBudgetDelay() : 0
|
||||
const budgetDelay = isBudgetedBackground(next.reason) ? nextBudgetDelay() : 0
|
||||
if (budgetDelay > 0) {
|
||||
scheduleDrain(budgetDelay)
|
||||
return
|
||||
@@ -312,7 +325,9 @@ async function drainQueue(): Promise<void> {
|
||||
scheduleDrain(Math.max(1_000, retryAt - Date.now()))
|
||||
continue
|
||||
}
|
||||
noteBackgroundStart()
|
||||
if (isBudgetedBackground(next.reason)) {
|
||||
noteBackgroundStart()
|
||||
}
|
||||
noteRateLimitSpend('graphql')
|
||||
noteRateLimitSpend('core')
|
||||
}
|
||||
@@ -384,7 +399,11 @@ export function enqueuePRRefresh(
|
||||
windowId
|
||||
})
|
||||
}
|
||||
broadcast({ aliases: [alias], reason, status: 'queued' })
|
||||
// Why: freshness follow-ups can be scheduled minutes out. Only immediate
|
||||
// queueing should surface in the card UI as pending work.
|
||||
if (shouldBroadcastQueued(dueAt)) {
|
||||
broadcast({ aliases: [alias], reason, status: 'queued' })
|
||||
}
|
||||
scheduleDrain()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user