starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-11-06 03:57:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix GitHub PR auto-merge eligibility (#32075)

Browse Source 
* Fix GitHub PR auto-merge eligibility handling

* Preserve GraphQL auto-merge state over REST fallback
This commit is contained in:
Jinjing
2026-10-22 21:23:08 -07:00
committed by GitHub
parent 26f677ee94
commit 075ed138bd
17 changed files with 535 additions and 231 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/github/client-merge-queue-auto-merge.test.ts
+77
View File
@@ -270,6 +270,83 @@ describe('GitHub GraphQL rate-limit guard', () => {
})
})
it('preserves a provider rejection when eligibility changes before enabling auto-merge', async () => {
const providerError =
'gh: Pull request Pull request is in unstable status (enablePullRequestAutoMerge)'
ghExecFileAsyncMock
.mockResolvedValueOnce({ stdout: JSON.stringify({ stack: null }) })
.mockResolvedValueOnce({
stdout: JSON.stringify({ id: 'PR_kwDO123', headRefOid: 'head-oid' })
})
.mockRejectedValueOnce(new Error(providerError))
await expect(
setPRAutoMerge('/repo-root', 7, true, 'squash', undefined, {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({
ok: false,
error: `Failed to enable auto-merge for pull request: ${providerError}`
})
expect(
ghExecFileAsyncMock.mock.calls.some(([args]) => args[0] === 'pr' && args[1] === 'merge')
).toBe(false)
})
it('names a failed disable-auto-merge operation', async () => {
ghExecFileAsyncMock.mockRejectedValueOnce(new Error('gh: pull request is already closed'))
await expect(
setPRAutoMerge('/repo-root', 7, false, 'squash', undefined, {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({
ok: false,
error: 'Failed to disable auto-merge for pull request: gh: pull request is already closed'
})
})
it('preserves REST merge eligibility and an existing auto-merge request in PR details', async () => {
getOwnerRepoMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' })
ghExecFileAsyncMock
.mockRejectedValueOnce(new Error('GraphQL unavailable'))
.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 7,
title: 'PR',
state: 'open',
html_url: 'https://github.com/stablyai/orca/pull/7',
mergeable: true,
mergeable_state: 'blocked',
auto_merge: { enabled_by: { login: 'octocat' }, merge_method: 'squash' },
head: { ref: 'feature', sha: 'head-oid' },
base: { ref: 'main' }
})
})
.mockResolvedValueOnce({ stdout: JSON.stringify({ reviewRequests: [], latestReviews: [] }) })
await expect(
getWorkItemByOwnerRepo(
'/repo-root',
{
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
},
7,
'pr'
)
).resolves.toMatchObject({
state: 'open',
mergeable: 'MERGEABLE',
mergeStateStatus: 'BLOCKED',
autoMergeEnabled: true
})
})
it('uses the queue-aware gh merge path when the base branch has a merge queue', async () => {
ghExecFileAsyncMock
.mockResolvedValueOnce({ stdout: JSON.stringify({ stack: null }) })
src/main/github/client-pr-branch-discovery.test.ts
+146
View File
@@ -27,6 +27,7 @@ vi.mock('./github-api-repository', async (importOriginal) =>
import { getPRForBranch } from './client/lookup/get-pr-for-branch'
import { resetPRForBranchMocks } from './client-test-harness'
import { canShowGitHubPRAutoMergeControl } from '../../shared/github/pull-request-auto-merge-availability'
const {
execFileAsyncMock,
@@ -40,6 +41,151 @@ describe('getPRForBranch', () => {
resetPRForBranchMocks(clientMocks)
})
it.each([
{ restEnabled: false, graphqlEnabled: true },
{ restEnabled: true, graphqlEnabled: false }
])(
'uses exact GraphQL auto-merge state ($graphqlEnabled) over REST discovery ($restEnabled)',
async ({ restEnabled, graphqlEnabled }) => {
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock
.mockResolvedValueOnce({
stdout: JSON.stringify([
{
number: 42,
title: 'Fix PR discovery',
state: 'open',
html_url: 'https://github.com/acme/widgets/pull/42',
updated_at: '2026-10-21T16:31:00Z',
draft: false,
mergeable: true,
mergeable_state: 'blocked',
auto_merge: restEnabled ? { merge_method: 'squash' } : null,
base: { ref: 'main', sha: 'base-oid' },
head: { ref: 'feature/test', sha: 'head-oid' }
}
])
})
.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 42,
title: 'Fix PR discovery',
state: 'OPEN',
url: 'https://github.com/acme/widgets/pull/42',
updatedAt: '2026-10-21T16:32:00Z',
statusCheckRollup: [],
isDraft: false,
mergeable: 'MERGEABLE',
mergeStateStatus: 'BLOCKED',
autoMergeRequest: graphqlEnabled ? { enabledAt: '2026-10-21T16:32:00Z' } : null,
baseRefName: 'main',
baseRefOid: 'base-oid',
headRefName: 'feature/test',
headRefOid: 'head-oid'
})
})
.mockResolvedValue({
stdout: JSON.stringify({
data: {
repository: {
autoMergeAllowed: true,
mergeQueue: null,
ref: { rules: { nodes: [{ type: 'PULL_REQUEST' }] } }
}
}
})
})
const pr = await getPRForBranch('/repo-root', 'feature/test')
expect(pr).toMatchObject({ number: 42, autoMergeEnabled: graphqlEnabled })
}
)
it.each([
['unstable', true, false],
['unstable', null, false],
['blocked', true, true],
['behind', true, true],
['dirty', false, false],
['unknown', null, false]
] as const)(
'preserves REST %s eligibility when exact GraphQL lookup fails (mergeable=%s)',
async (mergeable_state, mergeable, autoMergeAvailable) => {
getOwnerRepoMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' })
ghExecFileAsyncMock
.mockRejectedValueOnce(new Error('network unavailable'))
.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 31259,
title: 'fix(i18n): restore localized editor safety copy',
state: 'open',
html_url: 'https://github.com/stablyai/orca/pull/31259',
updated_at: '2026-10-21T16:31:00Z',
draft: false,
mergeable,
mergeable_state,
auto_merge: null,
base: { ref: 'main', sha: 'base-oid' },
head: {
ref: 'fix/editor-safety-copy',
sha: 'e4353ead012043474fc723e50aea198a76d32979'
}
})
})
.mockResolvedValue({
stdout: JSON.stringify({
data: {
repository: {
autoMergeAllowed: true,
mergeQueue: null,
ref: { rules: { nodes: [{ type: 'PULL_REQUEST' }] } }
}
}
})
})
const pr = await getPRForBranch('/repo-root', 'fix/editor-safety-copy', 31259)
expect(pr).toMatchObject({
mergeStateStatus: mergeable_state.toUpperCase(),
autoMergeEnabled: false
})
if (!pr) {
throw new Error('Expected a discovered PR')
}
expect(canShowGitHubPRAutoMergeControl(pr)).toBe(autoMergeAvailable)
}
)
it('does not infer auto-merge eligibility from a branch list result when exact lookup fails', async () => {
getOwnerRepoMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' })
ghExecFileAsyncMock
.mockResolvedValueOnce({
stdout: JSON.stringify([
{
number: 31259,
title: 'fix(i18n): restore localized editor safety copy',
state: 'open',
html_url: 'https://github.com/stablyai/orca/pull/31259',
updated_at: '2026-10-21T16:31:00Z',
draft: false,
auto_merge: null,
base: { ref: 'main', sha: 'base-oid' },
head: { ref: 'fix/editor-safety-copy', sha: 'e4353ead012043474fc723e50aea198a76d32979' }
}
])
})
.mockRejectedValueOnce(new Error('network unavailable'))
const pr = await getPRForBranch('/repo-root', 'fix/editor-safety-copy')
expect(pr).toMatchObject({ number: 31259, mergeable: 'UNKNOWN', autoMergeEnabled: false })
expect(pr?.mergeStateStatus).toBeUndefined()
if (!pr) {
throw new Error('Expected a discovered PR')
}
expect(canShowGitHubPRAutoMergeControl(pr)).toBe(false)
})
it('queries GitHub by head branch when the remote is on github.com', async () => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock.mockResolvedValueOnce({
src/main/github/client-pr-conflict-summary.test.ts
+5
View File
@@ -330,6 +330,11 @@ describe('getPRForBranch', () => {
ghExecFileAsyncMock
.mockResolvedValueOnce({ stdout: JSON.stringify([branchLookup]) })
.mockResolvedValueOnce({ stdout: JSON.stringify(exactLookup) })
.mockResolvedValueOnce({
stdout: JSON.stringify({
data: { repository: { autoMergeAllowed: true, mergeQueue: null } }
})
})
.mockResolvedValueOnce({ stdout: JSON.stringify([pushedBranchLookup]) })
.mockResolvedValueOnce({ stdout: JSON.stringify(pushedExactLookup) })
gitExecFileAsyncMock
src/main/github/client/lookup/pull-request-lookup-data.ts
+5
View File
@@ -61,6 +61,7 @@ export type RestPullRequest = {
merged_at?: string | null
mergeable?: boolean | null
mergeable_state?: string | null
auto_merge?: unknown
base?: { ref?: string; sha?: string }
head?: {
ref?: string
@@ -130,6 +131,10 @@ export function mapRestPullRequest(pr: RestPullRequest): PullRequestLookupData {
updatedAt: pr.updated_at ?? '',
isDraft: pr.draft,
mergeable: mapRestPRMergeable(pr),
...(pr.mergeable_state !== undefined
? { mergeStateStatus: pr.mergeable_state?.toUpperCase() ?? null }
: {}),
...('auto_merge' in pr ? { autoMergeRequest: pr.auto_merge } : {}),
baseRefName: pr.base?.ref,
headRefName: pr.head?.ref,
baseRefOid: pr.base?.sha,
src/main/github/client/lookup/pull-request-lookup-hydration.ts
+10 -8
View File
@@ -1,6 +1,7 @@
import type { OwnerRepo } from '../../gh-utils'
import type { GhExecOptions } from './../github-exec-scope'
import { detectRepositoryMergeMetadata } from './../detect/repository-merge-metadata'
import { mapPRState } from '../../mappers'
import {
normalizePullRequestLookupData,
type PullRequestLookupData
@@ -14,14 +15,15 @@ export async function hydratePullRequestLookupData(
const normalized = normalizePullRequestLookupData(data)
const hasRichMergeFields =
'reviewDecision' in data || 'mergeStateStatus' in data || 'autoMergeRequest' in data
const mergeMetadata = hasRichMergeFields
? await detectRepositoryMergeMetadata(
ownerRepo,
normalized.stack?.baseRefName ?? normalized.baseRefName,
ghOptions,
executionScope
)
: undefined
const mergeMetadata =
hasRichMergeFields && mapPRState(data.state, data.isDraft) === 'open'
? await detectRepositoryMergeMetadata(
ownerRepo,
normalized.stack?.baseRefName ?? normalized.baseRefName,
ghOptions,
executionScope
)
: undefined
return {
...normalized,
...(mergeMetadata ? { mergeQueueRequired: mergeMetadata.mergeQueueRequired } : {}),
src/main/github/client/map/work-item.ts
+9 -4
View File
@@ -122,12 +122,17 @@ export function mapPullRequestWorkItem(
? { checksSummary: deriveWorkItemCheckSummary(item.statusCheckRollup) }
: {}),
...(mergeable ? { mergeable } : {}),
...('autoMergeRequest' in item
? { autoMergeEnabled: isAutoMergeEnabled(item.autoMergeRequest) }
...('autoMergeRequest' in item || 'auto_merge' in item
? { autoMergeEnabled: isAutoMergeEnabled(item.autoMergeRequest ?? item.auto_merge) }
: {}),
...('mergeStateStatus' in item
...('mergeStateStatus' in item || 'mergeable_state' in item
? {
mergeStateStatus: typeof item.mergeStateStatus === 'string' ? item.mergeStateStatus : null
mergeStateStatus:
typeof item.mergeStateStatus === 'string'
? item.mergeStateStatus
: typeof item.mergeable_state === 'string'
? item.mergeable_state.toUpperCase()
: null
}
: {}),
...(typeof item.maintainerCanModify === 'boolean'
src/main/github/client/merge/pr-auto-merge.ts
+9 -11
View File
@@ -1,12 +1,7 @@
import { invalidateReviewLookupsAfterPRMutation } from '../../pr-mutation-review-invalidation'
import type { GitHubPRMergeMethod } from '../../../../shared/github/pull-request-types'
import {
ghExecFileAsync,
acquire,
release,
classifyGhError,
type LocalGitExecOptions
} from '../../gh-utils'
import { ghExecFileAsync, acquire, release, type LocalGitExecOptions } from '../../gh-utils'
import { classifyPullRequestUpdateError } from '../../gh-error-classification'
import { resolveGitHubRepoExecution, type GitHubApiRepository } from '../../github-api-repository'
import { githubPRStackExecutionScope, type GhExecOptions } from './../github-exec-scope'
import { detectRepositoryMergeMetadata } from './../detect/repository-merge-metadata'
@@ -21,11 +16,14 @@ export const GITHUB_AUTO_MERGE_METHODS: Record<GitHubPRMergeMethod, 'MERGE' | 'S
}
// Why: GitHub rejects auto-merge on an already-mergeable PR ("clean status"); surface an actionable message instead of the raw error.
export function classifySetAutoMergeError(message: string): string {
if (/in clean status/i.test(message)) {
export function classifySetAutoMergeError(message: string, enabled = true): string {
if (enabled && /in clean status/i.test(message)) {
return 'This pull request can already be merged. Use Merge instead of auto-merge.'
}
return classifyGhError(message).message
return classifyPullRequestUpdateError(
message,
`${enabled ? 'enable' : 'disable'} auto-merge for pull request`
).message
}
export type PRAutoMergeIdentity = {
@@ -191,7 +189,7 @@ export async function setPRAutoMerge(
} catch (err) {
const message =
err instanceof Error ? err.message : typeof err === 'string' ? err : 'Unknown error'
return { ok: false, error: classifySetAutoMergeError(message) }
return { ok: false, error: classifySetAutoMergeError(message, enabled) }
} finally {
release()
}
src/main/github/gh-error-classification.ts
+5 -2
View File
@@ -58,7 +58,10 @@ export function classifyListIssuesError(stderr: string): ClassifiedError {
// Why: classifyGhError's copy names an "issue"; PR mutations reuse the same
// classification but must not tell the user their pull request is an issue.
export function classifyPullRequestUpdateError(stderr: string): ClassifiedError {
export function classifyPullRequestUpdateError(
stderr: string,
action = 'update pull request'
): ClassifiedError {
const c = classifyGhError(stderr)
const trimmed = stderr.trim()
const pullRequestMessages: Record<ClassifiedError['type'], string> = {
@@ -69,7 +72,7 @@ export function classifyPullRequestUpdateError(stderr: string): ClassifiedError
validation_error: `Invalid update — ${trimmed}`,
rate_limited: c.message,
network_error: c.message,
unknown: `Failed to update pull request: ${trimmed}`
unknown: `Failed to ${action}: ${trimmed}`
}
return { type: c.type, message: pullRequestMessages[c.type] }
}