mirror of
https://github.com/stablyai/orca.git
synced 2026-10-29 00:03:37 +00:00
Handle PR refresh fallback errors
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
+43
-35
@@ -1158,43 +1158,51 @@ export async function getPRChecks(
|
||||
): Promise<PRCheckDetail[]> {
|
||||
const ownerRepo = headSha ? await getOwnerRepo(repoPath) : null
|
||||
if (ownerRepo && headSha) {
|
||||
await assertRateLimitBudget('core')
|
||||
await acquire()
|
||||
let canUseRestChecks = true
|
||||
try {
|
||||
// Why: --cache 60s saves rate-limit budget during polling, but when the
|
||||
// user explicitly clicks refresh we must skip it so gh fetches fresh data.
|
||||
const cacheArgs = options?.noCache ? [] : ['--cache', '60s']
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
[
|
||||
'api',
|
||||
...cacheArgs,
|
||||
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodeURIComponent(headSha)}/check-runs?per_page=100`
|
||||
],
|
||||
{ cwd: repoPath }
|
||||
)
|
||||
noteRateLimitSpend('core')
|
||||
const data = JSON.parse(stdout) as {
|
||||
check_runs: {
|
||||
name: string
|
||||
status: string
|
||||
conclusion: string | null
|
||||
html_url: string
|
||||
details_url: string | null
|
||||
}[]
|
||||
}
|
||||
return data.check_runs.map((d) => ({
|
||||
name: d.name,
|
||||
status: mapCheckRunRESTStatus(d.status),
|
||||
conclusion: mapCheckRunRESTConclusion(d.status, d.conclusion),
|
||||
url: d.details_url || d.html_url || null
|
||||
}))
|
||||
await assertRateLimitBudget('core')
|
||||
} catch (err) {
|
||||
// Why: a PR can outlive the cached head SHA after force-pushes or remote
|
||||
// rewrites. Falling back to `gh pr checks` keeps the panel populated
|
||||
// instead of rendering a false "no checks" state from a stale commit.
|
||||
console.warn('getPRChecks via head SHA failed, falling back to gh pr checks:', err)
|
||||
} finally {
|
||||
release()
|
||||
canUseRestChecks = false
|
||||
console.warn('getPRChecks skipped REST check-runs, falling back to gh pr checks:', err)
|
||||
}
|
||||
if (canUseRestChecks) {
|
||||
await acquire()
|
||||
try {
|
||||
// Why: --cache 60s saves rate-limit budget during polling, but when the
|
||||
// user explicitly clicks refresh we must skip it so gh fetches fresh data.
|
||||
const cacheArgs = options?.noCache ? [] : ['--cache', '60s']
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
[
|
||||
'api',
|
||||
...cacheArgs,
|
||||
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodeURIComponent(headSha)}/check-runs?per_page=100`
|
||||
],
|
||||
{ cwd: repoPath }
|
||||
)
|
||||
noteRateLimitSpend('core')
|
||||
const data = JSON.parse(stdout) as {
|
||||
check_runs: {
|
||||
name: string
|
||||
status: string
|
||||
conclusion: string | null
|
||||
html_url: string
|
||||
details_url: string | null
|
||||
}[]
|
||||
}
|
||||
return data.check_runs.map((d) => ({
|
||||
name: d.name,
|
||||
status: mapCheckRunRESTStatus(d.status),
|
||||
conclusion: mapCheckRunRESTConclusion(d.status, d.conclusion),
|
||||
url: d.details_url || d.html_url || null
|
||||
}))
|
||||
} catch (err) {
|
||||
// Why: a PR can outlive the cached head SHA after force-pushes or remote
|
||||
// rewrites. Falling back to `gh pr checks` keeps the panel populated
|
||||
// instead of rendering a false "no checks" state from a stale commit.
|
||||
console.warn('getPRChecks via head SHA failed, falling back to gh pr checks:', err)
|
||||
} finally {
|
||||
release()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -544,6 +544,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
const isQueuedPRRefresh = prRefreshState?.status === 'queued'
|
||||
const isInFlightPRRefresh = prRefreshState?.status === 'in-flight'
|
||||
const isPausedPRRefresh = prRefreshState?.status === 'paused'
|
||||
const isErroredPRRefresh = prRefreshState?.status === 'error'
|
||||
|
||||
return (
|
||||
<div className="px-4 py-6">
|
||||
@@ -552,24 +553,28 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
? `${operationLabel} in progress`
|
||||
: isRemoteRepo
|
||||
? 'Checks unavailable'
|
||||
: isQueuedPRRefresh
|
||||
? 'Checking for pull request'
|
||||
: isInFlightPRRefresh
|
||||
: isErroredPRRefresh
|
||||
? 'Could not refresh pull request'
|
||||
: isQueuedPRRefresh
|
||||
? 'Checking for pull request'
|
||||
: 'No pull request found'}
|
||||
: isInFlightPRRefresh
|
||||
? 'Checking for pull request'
|
||||
: 'No pull request found'}
|
||||
</div>
|
||||
<div className="mt-1 text-xs text-muted-foreground">
|
||||
{operationInProgress
|
||||
? 'PR checks will be available after the operation completes'
|
||||
: isRemoteRepo
|
||||
? 'PR refresh is not supported for remote worktrees yet'
|
||||
: isQueuedPRRefresh
|
||||
? 'Waiting to refresh GitHub status for this branch'
|
||||
: isInFlightPRRefresh
|
||||
? 'Refreshing GitHub status for this branch'
|
||||
: isPausedPRRefresh
|
||||
? 'GitHub refresh is paused by the current rate-limit budget'
|
||||
: 'Push your branch and open a PR to see checks here'}
|
||||
: isErroredPRRefresh
|
||||
? 'GitHub status could not be refreshed. Existing cached data was preserved.'
|
||||
: isQueuedPRRefresh
|
||||
? 'Waiting to refresh GitHub status for this branch'
|
||||
: isInFlightPRRefresh
|
||||
? 'Refreshing GitHub status for this branch'
|
||||
: isPausedPRRefresh
|
||||
? 'GitHub refresh is paused by the current rate-limit budget'
|
||||
: 'Push your branch and open a PR to see checks here'}
|
||||
</div>
|
||||
{!operationInProgress && !isRemoteRepo && (
|
||||
<button
|
||||
|
||||
@@ -10,6 +10,7 @@ import type { PRInfo } from '../../../../shared/types'
|
||||
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([]),
|
||||
@@ -232,6 +233,8 @@ describe('createGitHubSlice.fetchPRForBranch', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockApi.gh.prForBranch.mockResolvedValue(null)
|
||||
mockApi.gh.refreshPRNow.mockReset()
|
||||
mockApi.gh.refreshPRNow.mockResolvedValue({ kind: 'no-pr', fetchedAt: Date.now() })
|
||||
})
|
||||
|
||||
it('lets a forced refresh bypass a non-forced inflight request and keeps the newer result', async () => {
|
||||
@@ -239,6 +242,8 @@ describe('createGitHubSlice.fetchPRForBranch', () => {
|
||||
const repoPath = '/repo'
|
||||
const branch = 'feature/test'
|
||||
const prCacheKey = `${repoPath}::${branch}`
|
||||
const refreshPRNow = mockApi.gh.refreshPRNow
|
||||
;(mockApi.gh as unknown as { refreshPRNow?: typeof refreshPRNow }).refreshPRNow = undefined
|
||||
|
||||
let resolveInitial: ((value: null) => void) | undefined
|
||||
const initialRequest = new Promise<null>((resolve) => {
|
||||
@@ -249,17 +254,21 @@ describe('createGitHubSlice.fetchPRForBranch', () => {
|
||||
.mockReturnValueOnce(initialRequest)
|
||||
.mockResolvedValueOnce(makePR({ number: 99, title: 'Forced refresh PR' }))
|
||||
|
||||
const initialFetch = store.getState().fetchPRForBranch(repoPath, branch)
|
||||
const forcedFetch = store.getState().fetchPRForBranch(repoPath, branch, { force: true })
|
||||
try {
|
||||
const initialFetch = store.getState().fetchPRForBranch(repoPath, branch)
|
||||
const forcedFetch = store.getState().fetchPRForBranch(repoPath, branch, { force: true })
|
||||
|
||||
await expect(forcedFetch).resolves.toMatchObject({ number: 99, title: 'Forced refresh PR' })
|
||||
expect(mockApi.gh.prForBranch).toHaveBeenCalledTimes(2)
|
||||
expect(store.getState().prCache[prCacheKey]?.data).toMatchObject({ number: 99 })
|
||||
await expect(forcedFetch).resolves.toMatchObject({ number: 99, title: 'Forced refresh PR' })
|
||||
expect(mockApi.gh.prForBranch).toHaveBeenCalledTimes(2)
|
||||
expect(store.getState().prCache[prCacheKey]?.data).toMatchObject({ number: 99 })
|
||||
|
||||
resolveInitial?.(null)
|
||||
await expect(initialFetch).resolves.toBeNull()
|
||||
resolveInitial?.(null)
|
||||
await expect(initialFetch).resolves.toBeNull()
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data).toMatchObject({ number: 99 })
|
||||
expect(store.getState().prCache[prCacheKey]?.data).toMatchObject({ number: 99 })
|
||||
} finally {
|
||||
mockApi.gh.refreshPRNow = refreshPRNow
|
||||
}
|
||||
})
|
||||
|
||||
it('does not call GitHub refresh IPC for SSH-backed repos', async () => {
|
||||
@@ -291,6 +300,70 @@ describe('createGitHubSlice.fetchPRForBranch', () => {
|
||||
expect(mockApi.gh.prForBranch).not.toHaveBeenCalled()
|
||||
expect(mockApi.gh.enqueuePRRefresh).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('preserves cached PR data when a forced coordinator refresh errors', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const branch = 'feature/test'
|
||||
const cachedPR = makePR({ number: 12 })
|
||||
|
||||
store.setState({
|
||||
repos: [{ id: 'repo-1', path: repoPath, name: 'repo', kind: 'git' }],
|
||||
prCache: {
|
||||
[`${repoPath}::${branch}`]: {
|
||||
data: cachedPR,
|
||||
fetchedAt: 1
|
||||
}
|
||||
}
|
||||
} as unknown as Partial<AppState>)
|
||||
mockApi.gh.refreshPRNow.mockResolvedValueOnce({
|
||||
kind: 'upstream-error',
|
||||
errorType: 'network',
|
||||
message: 'network unavailable',
|
||||
fetchedAt: Date.now()
|
||||
})
|
||||
|
||||
await expect(
|
||||
store.getState().fetchPRForBranch(repoPath, branch, { force: true })
|
||||
).resolves.toEqual(cachedPR)
|
||||
expect(store.getState().prCache[`${repoPath}::${branch}`]?.data).toEqual(cachedPR)
|
||||
})
|
||||
|
||||
it('records PR refresh errors without clearing cached PR data', () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const branch = 'feature/test'
|
||||
const cacheKey = `${repoPath}::${branch}`
|
||||
const cachedPR = makePR({ number: 12 })
|
||||
|
||||
store.setState({
|
||||
prCache: {
|
||||
[cacheKey]: {
|
||||
data: cachedPR,
|
||||
fetchedAt: 1
|
||||
}
|
||||
}
|
||||
} as unknown as Partial<AppState>)
|
||||
|
||||
store.getState().applyGitHubPRRefreshEvent({
|
||||
sequence: 1,
|
||||
aliases: [{ cacheKey, repoPath, branch }],
|
||||
reason: 'manual',
|
||||
outcome: {
|
||||
kind: 'upstream-error',
|
||||
errorType: 'network',
|
||||
message: 'network unavailable',
|
||||
fetchedAt: Date.now()
|
||||
}
|
||||
})
|
||||
|
||||
expect(store.getState().prCache[cacheKey]?.data).toEqual(cachedPR)
|
||||
expect(store.getState().prRefreshStates[cacheKey]).toMatchObject({
|
||||
status: 'error',
|
||||
reason: 'manual',
|
||||
message: 'network unavailable'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('createGitHubSlice.refreshGitHubForWorktreeIfStale', () => {
|
||||
|
||||
@@ -293,10 +293,11 @@ type FetchOptions = {
|
||||
}
|
||||
|
||||
type PRRefreshState = {
|
||||
status: 'queued' | 'in-flight' | 'paused' | 'skipped'
|
||||
status: 'queued' | 'in-flight' | 'paused' | 'skipped' | 'error'
|
||||
reason: GitHubPRRefreshReason
|
||||
updatedAt: number
|
||||
pausedUntil?: number
|
||||
message?: string
|
||||
}
|
||||
|
||||
const CACHE_TTL = 300_000 // 5 minutes (stale data shown instantly, then refreshed)
|
||||
@@ -1308,13 +1309,14 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
)
|
||||
const pr: PRInfo | null =
|
||||
outcome.kind === 'found' ? outcome.pr : outcome.kind === 'no-pr' ? null : null
|
||||
if (outcome.kind === 'upstream-error') {
|
||||
return cached?.data ?? null
|
||||
}
|
||||
if (!usesRefreshCoordinator && prRequestGenerations.get(cacheKey) === generation) {
|
||||
if (outcome.kind !== 'upstream-error') {
|
||||
set((s) => ({
|
||||
prCache: { ...s.prCache, [cacheKey]: { data: pr, fetchedAt: outcome.fetchedAt } }
|
||||
}))
|
||||
debouncedSaveCache(get())
|
||||
}
|
||||
set((s) => ({
|
||||
prCache: { ...s.prCache, [cacheKey]: { data: pr, fetchedAt: outcome.fetchedAt } }
|
||||
}))
|
||||
debouncedSaveCache(get())
|
||||
}
|
||||
return pr ?? null
|
||||
} catch (err) {
|
||||
@@ -1562,6 +1564,12 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
if (event.outcome) {
|
||||
delete nextStates[alias.cacheKey]
|
||||
if (event.outcome.kind === 'upstream-error') {
|
||||
nextStates[alias.cacheKey] = {
|
||||
status: 'error',
|
||||
reason: event.reason,
|
||||
updatedAt: Date.now(),
|
||||
message: event.outcome.message
|
||||
}
|
||||
continue
|
||||
}
|
||||
const data =
|
||||
|
||||
Reference in New Issue
Block a user