starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-09-27 00:02:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Accept merged fallback PRs during branch lookup (#5908)

Browse Source 
Ensure that when a visible fallback PR has been merged (e.g., outside
Orca with a deleted head branch), it is still accepted and refreshed by
branch lookup instead of being discarded as an implicit merged PR.

* Add `acceptMergedFallbackPR` option to GitHub branch lookups
* Enable this option during manual and background refreshes of fallback PRs
* Plumb the new option through preload APIs, IPC handlers, and RPC protocols
This commit is contained in:
Jinjing
2026-06-20 03:30:18 -07:00
committed by GitHub
parent 1419193bea
commit 97dc6d63e3
16 changed files with 253 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/github/client.test.ts
+102
View File
@@ -823,6 +823,76 @@ describe('getPRForBranch', () => {
expect(pr).toMatchObject({ number: 42, title: 'Open fallback PR' })
})
it('returns a merged PR when branch lookup and fallback point at the same PR', async () => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock
.mockResolvedValueOnce({
stdout: JSON.stringify([
{
number: 5511,
title: 'Merged current PR',
state: 'closed',
merged_at: '2026-06-16T17:15:33Z',
html_url: 'https://github.com/acme/widgets/pull/5511',
updated_at: '2026-06-16T17:15:33Z',
draft: false,
mergeable_state: 'clean',
head: { ref: 'feature/test', sha: 'merged-head-oid' },
base: { ref: 'main', sha: 'base-oid' }
}
])
})
.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 5511,
title: 'Merged current PR',
state: 'MERGED',
url: 'https://github.com/acme/widgets/pull/5511',
statusCheckRollup: [],
updatedAt: '2026-06-16T17:15:33Z',
isDraft: false,
mergeable: 'MERGEABLE',
baseRefName: 'main',
headRefName: 'feature/test',
baseRefOid: 'base-oid',
headRefOid: 'merged-head-oid'
})
})
.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 5511,
title: 'Merged current PR',
state: 'MERGED',
url: 'https://github.com/acme/widgets/pull/5511',
statusCheckRollup: [],
updatedAt: '2026-06-16T17:15:33Z',
isDraft: false,
mergeable: 'MERGEABLE',
baseRefName: 'main',
headRefName: 'feature/test',
baseRefOid: 'base-oid',
headRefOid: 'merged-head-oid'
})
})
const pr = await getPRForBranch('/repo-root', 'feature/test', null, null, 5511)
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
3,
[
'pr',
'view',
'5511',
'--repo',
'acme/widgets',
'--json',
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
],
{ cwd: '/repo-root' }
)
expect(pr).toMatchObject({ number: 5511, state: 'merged', title: 'Merged current PR' })
})
it('does not carry a merged upstream branch head repo into a fallback PR number', async () => {
resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({
candidates: [{ owner: 'stablyai', repo: 'orca' }],
@@ -925,6 +995,38 @@ describe('getPRForBranch', () => {
expect(pr).toBeNull()
})
it('returns a merged fallback PR when visible fallback lifecycle is accepted', async () => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock
.mockResolvedValueOnce({ stdout: JSON.stringify([]) })
.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 5511,
title: 'Merged visible fallback PR',
state: 'MERGED',
url: 'https://github.com/acme/widgets/pull/5511',
statusCheckRollup: [],
updatedAt: '2026-06-16T17:15:33Z',
isDraft: false,
mergeable: 'MERGEABLE',
baseRefName: 'main',
headRefName: 'deleted-head',
baseRefOid: 'base-oid',
headRefOid: 'head-oid'
})
})
const pr = await getPRForBranch('/repo-root', 'deleted-head', null, null, 5511, {
acceptMergedFallbackPR: true
})
expect(pr).toMatchObject({
number: 5511,
state: 'merged',
title: 'Merged visible fallback PR'
})
})
it('falls back to the tracked upstream branch when the local branch name differs', async () => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock
src/main/github/client.ts
+20 -3
View File
@@ -1989,6 +1989,10 @@ const PR_LOOKUP_JSON_FIELDS =
const PR_BRANCH_LIST_JSON_FIELDS =
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
export type GitHubPRBranchLookupOptions = HostedReviewExecutionOptions & {
acceptMergedFallbackPR?: boolean
}
function mapRestPRMergeable(pr: RestPullRequest): PRMergeableState {
const mergeableState = pr.mergeable_state?.toLowerCase()
if (mergeableState === 'dirty') {
@@ -2455,7 +2459,7 @@ export async function getPRForBranch(
linkedPRNumber?: number | null,
connectionId?: string | null,
fallbackPRNumber?: number | null,
options: HostedReviewExecutionOptions = {}
options: GitHubPRBranchLookupOptions = {}
): Promise<PRInfo | null> {
const outcome = await getPRForBranchOutcome(
repoPath,
@@ -2474,7 +2478,7 @@ export async function getPRForBranchOutcome(
linkedPRNumber?: number | null,
connectionId?: string | null,
fallbackPRNumber?: number | null,
options: HostedReviewExecutionOptions = {}
options: GitHubPRBranchLookupOptions = {}
): Promise<PRRefreshOutcome> {
// Strip refs/heads/ prefix if present
const branchName = branch.replace(/^refs\/heads\//, '')
@@ -2549,7 +2553,9 @@ export async function getPRForBranchOutcome(
}
}
}
let mergedBranchLookupNumber: number | null = null
if (data && isMergedImplicitPR(data, linkedPRNumber)) {
mergedBranchLookupNumber = data.number
data = null
dataRepo = null
dataHeadRepo = headRepo
@@ -2566,7 +2572,18 @@ export async function getPRForBranchOutcome(
if (!data) {
return { kind: 'no-pr', fetchedAt: Date.now() }
}
if (isMergedImplicitPR(data, linkedPRNumber)) {
const fallbackConfirmedMergedBranch =
typeof fallbackPRNumber === 'number' &&
mergedBranchLookupNumber === fallbackPRNumber &&
data.number === fallbackPRNumber
// Why: a currently visible PR can be merged outside Orca; when the caller
// marks the fallback as visible review state, keep its lifecycle fresh even
// if GitHub no longer reports it by branch (for example deleted heads).
if (
isMergedImplicitPR(data, linkedPRNumber) &&
!fallbackConfirmedMergedBranch &&
options.acceptMergedFallbackPR !== true
) {
return { kind: 'no-pr', fetchedAt: Date.now() }
}
src/main/github/pr-refresh-coordinator.test.ts
+25
View File
@@ -309,6 +309,31 @@ describe('pr-refresh-coordinator', () => {
expect(outcome?.sequence).toBe(inFlight?.sequence)
})
it('accepts merged fallback PRs for visible fallback refreshes', async () => {
const { refreshPRNow } = await import('./pr-refresh-coordinator')
getPRForBranchOutcomeMock.mockResolvedValueOnce({
kind: 'found',
pr: makePR({ state: 'merged' }),
fetchedAt: Date.now()
})
await refreshPRNow(
makeCandidate({
fallbackPRNumber: 12,
fallbackPRSource: 'pr-cache'
})
)
expect(getPRForBranchOutcomeMock).toHaveBeenCalledWith(
'/repo',
'feature/test',
null,
null,
12,
{ acceptMergedFallbackPR: true }
)
})
it('does not coalesce local and SSH refreshes for the same branch', async () => {
const { enqueuePRRefresh } = await import('./pr-refresh-coordinator')
getPRForBranchOutcomeMock
src/main/github/pr-refresh-coordinator.ts
+26 -9
View File
@@ -10,8 +10,7 @@ import type {
GitHubPRRefreshSkippedReason,
PRRefreshOutcome
} from '../../shared/types'
import type { HostedReviewExecutionOptions } from '../source-control/hosted-review-git-options'
import { getPRForBranchOutcome } from './client'
import { getPRForBranchOutcome, type GitHubPRBranchLookupOptions } from './client'
import { getRateLimit, noteRateLimitSpend, rateLimitGuard } from './rate-limit'
type QueueEntry = {
@@ -30,12 +29,30 @@ type PRRefreshOutcomeObserver = (
outcome: PRRefreshOutcome
) => void
type PRBranchLookupCandidate = Pick<
GitHubPRRefreshCandidate,
'localGitOptions' | 'linkedPRNumber' | 'fallbackPRNumber' | 'fallbackPRSource'
>
function shouldAcceptMergedFallbackPR(candidate: PRBranchLookupCandidate): boolean {
return (
candidate.linkedPRNumber == null &&
candidate.fallbackPRNumber != null &&
candidate.fallbackPRSource != null
)
}
function hostedReviewOptionArgs(
localGitOptions?: GitHubPRRefreshCandidate['localGitOptions']
): [] | [HostedReviewExecutionOptions] {
return localGitOptions?.wslDistro
? [{ localGitExecOptions: { wslDistro: localGitOptions.wslDistro } }]
: []
candidate: PRBranchLookupCandidate
): [] | [GitHubPRBranchLookupOptions] {
const options: GitHubPRBranchLookupOptions = {}
if (candidate.localGitOptions?.wslDistro) {
options.localGitExecOptions = { wslDistro: candidate.localGitOptions.wslDistro }
}
if (shouldAcceptMergedFallbackPR(candidate)) {
options.acceptMergedFallbackPR = true
}
return Object.keys(options).length > 0 ? [options] : []
}
const MIN_BACKGROUND_REFRESH_AGE_MS = 60_000
@@ -532,7 +549,7 @@ async function drainQueue(): Promise<void> {
next.candidate.linkedPRNumber ?? null,
next.candidate.connectionId ?? null,
next.candidate.linkedPRNumber == null ? (next.candidate.fallbackPRNumber ?? null) : null,
...hostedReviewOptionArgs(next.candidate.localGitOptions)
...hostedReviewOptionArgs(next.candidate)
)
outcomeObserver?.(next.candidate, outcome)
broadcast({ aliases, reason: next.reason, outcome, requestStartedAt }, requestSequence)
@@ -659,7 +676,7 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
candidate.linkedPRNumber ?? null,
candidate.connectionId ?? null,
candidate.linkedPRNumber == null ? (candidate.fallbackPRNumber ?? null) : null,
...hostedReviewOptionArgs(candidate.localGitOptions)
...hostedReviewOptionArgs(candidate)
)
outcomeObserver?.(candidate, outcome)
broadcast({ aliases, reason: 'manual', outcome, requestStartedAt }, requestSequence)