mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix: fetch PR checks by head sha (#312)
This commit is contained in:
@@ -12,7 +12,7 @@ vi.mock('util', async () => {
|
||||
}
|
||||
})
|
||||
|
||||
import { getPRForBranch, _resetOwnerRepoCache } from './client'
|
||||
import { getPRForBranch, getPRChecks, _resetOwnerRepoCache } from './client'
|
||||
|
||||
describe('getPRForBranch', () => {
|
||||
beforeEach(() => {
|
||||
@@ -241,3 +241,71 @@ describe('getPRForBranch', () => {
|
||||
expect(pr).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('getPRChecks', () => {
|
||||
beforeEach(() => {
|
||||
execFileAsyncMock.mockReset()
|
||||
_resetOwnerRepoCache()
|
||||
})
|
||||
|
||||
it('queries check-runs by PR head SHA when GitHub remote metadata is available', async () => {
|
||||
execFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' })
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
check_runs: [
|
||||
{
|
||||
name: 'build',
|
||||
status: 'completed',
|
||||
conclusion: 'success',
|
||||
html_url: 'https://github.com/acme/widgets/actions/runs/1',
|
||||
details_url: null
|
||||
}
|
||||
]
|
||||
})
|
||||
})
|
||||
|
||||
const checks = await getPRChecks('/repo-root', 42, 'head-oid')
|
||||
|
||||
expect(execFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'gh',
|
||||
['api', '--cache', '60s', 'repos/acme/widgets/commits/head-oid/check-runs?per_page=100'],
|
||||
{ cwd: '/repo-root', encoding: 'utf-8' }
|
||||
)
|
||||
expect(checks).toEqual([
|
||||
{
|
||||
name: 'build',
|
||||
status: 'completed',
|
||||
conclusion: 'success',
|
||||
url: 'https://github.com/acme/widgets/actions/runs/1'
|
||||
}
|
||||
])
|
||||
})
|
||||
|
||||
it('falls back to gh pr checks when the cached head SHA no longer resolves', async () => {
|
||||
execFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' })
|
||||
.mockRejectedValueOnce(new Error('gh: No commit found for SHA: stale-head (HTTP 422)'))
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ name: 'lint', state: 'PASS', link: 'https://example.com/lint' }])
|
||||
})
|
||||
|
||||
const checks = await getPRChecks('/repo-root', 42, 'stale-head')
|
||||
|
||||
expect(execFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
'gh',
|
||||
['pr', 'checks', '42', '--json', 'name,state,link'],
|
||||
{ cwd: '/repo-root', encoding: 'utf-8' }
|
||||
)
|
||||
expect(checks).toEqual([
|
||||
{
|
||||
name: 'lint',
|
||||
status: 'completed',
|
||||
conclusion: 'success',
|
||||
url: 'https://example.com/lint'
|
||||
}
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
+34
-26
@@ -103,6 +103,7 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
|
||||
checksStatus: deriveCheckStatus(data.statusCheckRollup),
|
||||
updatedAt: data.updatedAt,
|
||||
mergeable: (data.mergeable as PRMergeableState) ?? 'UNKNOWN',
|
||||
headSha: data.headRefOid,
|
||||
conflictSummary
|
||||
}
|
||||
} catch {
|
||||
@@ -120,40 +121,47 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
|
||||
export async function getPRChecks(
|
||||
repoPath: string,
|
||||
prNumber: number,
|
||||
branch?: string,
|
||||
headSha?: string,
|
||||
options?: { noCache?: boolean }
|
||||
): Promise<PRCheckDetail[]> {
|
||||
const ownerRepo = branch ? await getOwnerRepo(repoPath) : null
|
||||
const ownerRepo = headSha ? await getOwnerRepo(repoPath) : null
|
||||
await acquire()
|
||||
try {
|
||||
if (ownerRepo && branch) {
|
||||
if (ownerRepo && headSha) {
|
||||
// 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 execFileAsync(
|
||||
'gh',
|
||||
[
|
||||
'api',
|
||||
...cacheArgs,
|
||||
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodeURIComponent(branch)}/check-runs?per_page=100`
|
||||
],
|
||||
{ cwd: repoPath, encoding: 'utf-8' }
|
||||
)
|
||||
const data = JSON.parse(stdout) as {
|
||||
check_runs: {
|
||||
name: string
|
||||
status: string
|
||||
conclusion: string | null
|
||||
html_url: string
|
||||
details_url: string | null
|
||||
}[]
|
||||
try {
|
||||
const { stdout } = await execFileAsync(
|
||||
'gh',
|
||||
[
|
||||
'api',
|
||||
...cacheArgs,
|
||||
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodeURIComponent(headSha)}/check-runs?per_page=100`
|
||||
],
|
||||
{ cwd: repoPath, encoding: 'utf-8' }
|
||||
)
|
||||
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)
|
||||
}
|
||||
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
|
||||
}))
|
||||
}
|
||||
// Fallback: no branch provided or non-GitHub remote
|
||||
const { stdout } = await execFileAsync(
|
||||
|
||||
+10
-2
@@ -37,9 +37,17 @@ export function registerGitHubHandlers(store: Store): void {
|
||||
|
||||
ipcMain.handle(
|
||||
'gh:prChecks',
|
||||
(_event, args: { repoPath: string; prNumber: number; branch?: string; noCache?: boolean }) => {
|
||||
(
|
||||
_event,
|
||||
args: {
|
||||
repoPath: string
|
||||
prNumber: number
|
||||
headSha?: string
|
||||
noCache?: boolean
|
||||
}
|
||||
) => {
|
||||
const repoPath = assertRegisteredRepoPath(args.repoPath, store)
|
||||
return getPRChecks(repoPath, args.prNumber, args.branch, {
|
||||
return getPRChecks(repoPath, args.prNumber, args.headSha, {
|
||||
noCache: args.noCache
|
||||
})
|
||||
}
|
||||
|
||||
Vendored
+1
-1
@@ -70,7 +70,7 @@ type GhApi = {
|
||||
prChecks: (args: {
|
||||
repoPath: string
|
||||
prNumber: number
|
||||
branch?: string
|
||||
headSha?: string
|
||||
noCache?: boolean
|
||||
}) => Promise<PRCheckDetail[]>
|
||||
updatePRTitle: (args: { repoPath: string; prNumber: number; title: string }) => Promise<boolean>
|
||||
|
||||
@@ -189,7 +189,7 @@ const api = {
|
||||
prChecks: (args: {
|
||||
repoPath: string
|
||||
prNumber: number
|
||||
branch?: string
|
||||
headSha?: string
|
||||
noCache?: boolean
|
||||
}): Promise<unknown[]> => ipcRenderer.invoke('gh:prChecks', args),
|
||||
|
||||
|
||||
@@ -96,7 +96,9 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
}
|
||||
setChecksLoading(true)
|
||||
try {
|
||||
const result = await fetchPRChecks(repo.path, targetPRNumber, branch, { force })
|
||||
const result = await fetchPRChecks(repo.path, targetPRNumber, branch, pr?.headSha, {
|
||||
force
|
||||
})
|
||||
setChecks(result)
|
||||
|
||||
// Exponential backoff: if checks haven't changed, double the interval (cap 120s).
|
||||
@@ -114,7 +116,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
setChecksLoading(false)
|
||||
}
|
||||
},
|
||||
[repo, prNumber, branch, fetchPRChecks]
|
||||
[repo, prNumber, branch, pr?.headSha, fetchPRChecks]
|
||||
)
|
||||
|
||||
// Fetch checks on mount + poll with exponential backoff
|
||||
|
||||
@@ -37,6 +37,7 @@ function makePR(overrides: Partial<PRInfo> = {}): PRInfo {
|
||||
checksStatus: 'pending',
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
mergeable: 'UNKNOWN',
|
||||
headSha: 'head-oid',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
@@ -71,7 +72,7 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
||||
{ name: 'lint', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, { force: true })
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { force: true })
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('success')
|
||||
})
|
||||
@@ -96,7 +97,7 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
||||
{ name: 'integration', status: 'completed', conclusion: 'failure', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, { force: true })
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { force: true })
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('failure')
|
||||
})
|
||||
@@ -120,7 +121,9 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
||||
{ name: 'build', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, `refs/heads/${branch}`, { force: true })
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(repoPath, 12, `refs/heads/${branch}`, undefined, { force: true })
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('success')
|
||||
})
|
||||
@@ -146,7 +149,7 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
||||
{ name: 'build', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, { force: true })
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { force: true })
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
|
||||
expect(mockApi.cache.setGitHub).toHaveBeenCalledWith({
|
||||
@@ -193,6 +196,31 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('passes the cached PR head SHA to the checks IPC request', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const branch = 'feature/test'
|
||||
const prCacheKey = `${repoPath}::${branch}`
|
||||
|
||||
store.setState({
|
||||
prCache: {
|
||||
[prCacheKey]: {
|
||||
data: makePR({ headSha: 'abc123head' }),
|
||||
fetchedAt: 1
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, 'abc123head', { force: true })
|
||||
|
||||
expect(mockApi.gh.prChecks).toHaveBeenCalledWith({
|
||||
repoPath,
|
||||
prNumber: 12,
|
||||
headSha: 'abc123head',
|
||||
noCache: true
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('createGitHubSlice.fetchPRForBranch', () => {
|
||||
|
||||
@@ -58,6 +58,7 @@ export type GitHubSlice = {
|
||||
repoPath: string,
|
||||
prNumber: number,
|
||||
branch?: string,
|
||||
headSha?: string,
|
||||
options?: FetchOptions
|
||||
) => Promise<PRCheckDetail[]>
|
||||
initGitHubCache: () => Promise<void>
|
||||
@@ -170,7 +171,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
return request
|
||||
},
|
||||
|
||||
fetchPRChecks: async (repoPath, prNumber, branch, options): Promise<PRCheckDetail[]> => {
|
||||
fetchPRChecks: async (repoPath, prNumber, branch, headSha, options): Promise<PRCheckDetail[]> => {
|
||||
const cacheKey = `${repoPath}::pr-checks::${prNumber}`
|
||||
const cached = get().checksCache[cacheKey]
|
||||
if (!options?.force && isFresh(cached, CHECKS_CACHE_TTL)) {
|
||||
@@ -193,7 +194,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
const checks = (await window.api.gh.prChecks({
|
||||
repoPath,
|
||||
prNumber,
|
||||
branch,
|
||||
headSha,
|
||||
noCache: options?.force
|
||||
})) as PRCheckDetail[]
|
||||
set((s) => {
|
||||
|
||||
@@ -120,6 +120,10 @@ export type PRInfo = {
|
||||
checksStatus: CheckStatus
|
||||
updatedAt: string
|
||||
mergeable: PRMergeableState
|
||||
// Why: check-runs are keyed by the PR head commit, not the mutable branch name.
|
||||
// Keeping the head SHA in cached PR metadata lets the checks panel poll the
|
||||
// correct commit without re-querying GitHub or guessing from local branch refs.
|
||||
headSha?: string
|
||||
conflictSummary?: PRConflictSummary
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user