diff --git a/mobile/src/components/pr-sidebar/PRActionsSection.tsx b/mobile/src/components/pr-sidebar/PRActionsSection.tsx index 8f331ef11ba..f98d6eab6a5 100644 --- a/mobile/src/components/pr-sidebar/PRActionsSection.tsx +++ b/mobile/src/components/pr-sidebar/PRActionsSection.tsx @@ -7,7 +7,7 @@ import type { RpcClient } from '../../transport/rpc-client' import type { MobilePrActions } from '../../session/use-mobile-pr-actions' import { unlinkMobilePr } from '../../source-control/mobile-pr-link' import { ConfirmModal } from '../ConfirmModal' -import { canShowMobilePRAutoMergeControl } from './pr-auto-merge-availability' +import { canShowGitHubPRAutoMergeControl } from '../../../../src/shared/github/pull-request-auto-merge-availability' import { resolveMobilePrMergeMethod, resolvePrActionAvailability } from './pr-actions-state' import { prActionsStyles as styles } from './pr-actions-styles' @@ -44,7 +44,7 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked } const unlinkBusy = unlinking || mergeBusy || autoMergeBusy || stateBusy const showAutoMerge = avail.canAutoMerge && - canShowMobilePRAutoMergeControl({ + canShowGitHubPRAutoMergeControl({ ...pr, autoMergeEnabled: autoMerge || pr.autoMergeEnabled === true }) diff --git a/mobile/src/components/pr-sidebar/pr-auto-merge-availability.ts b/mobile/src/components/pr-sidebar/pr-auto-merge-availability.ts deleted file mode 100644 index 166e6e0c189..00000000000 --- a/mobile/src/components/pr-sidebar/pr-auto-merge-availability.ts +++ /dev/null @@ -1,51 +0,0 @@ -import type { PRInfo } from '../../../../src/shared/github/pull-request-types' - -type MobilePRAutoMergeAvailabilityInput = Pick< - PRInfo, - | 'state' - | 'mergeable' - | 'mergeStateStatus' - | 'reviewDecision' - | 'autoMergeEnabled' - | 'autoMergeAllowed' - | 'mergeQueueRequired' -> - -function isConflicting(item: MobilePRAutoMergeAvailabilityInput): boolean { - return item.mergeable === 'CONFLICTING' || item.mergeStateStatus === 'DIRTY' -} - -function isUnstable(item: MobilePRAutoMergeAvailabilityInput): boolean { - return item.mergeStateStatus === 'UNSTABLE' -} - -function hasReviewRequirement(item: MobilePRAutoMergeAvailabilityInput): boolean { - return item.reviewDecision === 'REVIEW_REQUIRED' || item.reviewDecision === 'CHANGES_REQUESTED' -} - -function canMergeImmediately(item: MobilePRAutoMergeAvailabilityInput): boolean { - if (item.mergeStateStatus === 'BLOCKED' || item.mergeStateStatus === 'BEHIND') { - return false - } - return item.mergeable === 'MERGEABLE' || item.mergeStateStatus === 'CLEAN' -} - -function canRequestWhenReady(item: MobilePRAutoMergeAvailabilityInput): boolean { - // Why: GitHub auto-merge waits on unmet requirements; UNSTABLE is rejected - // by the mutation rather than becoming a waitable auto-merge request. - if (item.state !== 'open' || isConflicting(item) || isUnstable(item)) { - return false - } - if (item.mergeQueueRequired === true) { - return true - } - return ( - item.autoMergeAllowed !== false && (hasReviewRequirement(item) || !canMergeImmediately(item)) - ) -} - -export function canShowMobilePRAutoMergeControl(item: MobilePRAutoMergeAvailabilityInput): boolean { - // Why: Metro/Expo cannot reliably transform runtime imports from root - // src/shared, so this mirrors github-pr-auto-merge-availability.ts. - return item.state === 'open' && (item.autoMergeEnabled === true || canRequestWhenReady(item)) -} diff --git a/src/main/github/client-merge-queue-auto-merge.test.ts b/src/main/github/client-merge-queue-auto-merge.test.ts index 3cd7723838d..fbaf81e686c 100644 --- a/src/main/github/client-merge-queue-auto-merge.test.ts +++ b/src/main/github/client-merge-queue-auto-merge.test.ts @@ -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 }) }) diff --git a/src/main/github/client-pr-branch-discovery.test.ts b/src/main/github/client-pr-branch-discovery.test.ts index ca41a3361f3..916a6320844 100644 --- a/src/main/github/client-pr-branch-discovery.test.ts +++ b/src/main/github/client-pr-branch-discovery.test.ts @@ -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({ diff --git a/src/main/github/client-pr-conflict-summary.test.ts b/src/main/github/client-pr-conflict-summary.test.ts index a245414228d..ed15023968f 100644 --- a/src/main/github/client-pr-conflict-summary.test.ts +++ b/src/main/github/client-pr-conflict-summary.test.ts @@ -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 diff --git a/src/main/github/client/lookup/pull-request-lookup-data.ts b/src/main/github/client/lookup/pull-request-lookup-data.ts index 71b534446d9..2f69e404a2e 100644 --- a/src/main/github/client/lookup/pull-request-lookup-data.ts +++ b/src/main/github/client/lookup/pull-request-lookup-data.ts @@ -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, diff --git a/src/main/github/client/lookup/pull-request-lookup-hydration.ts b/src/main/github/client/lookup/pull-request-lookup-hydration.ts index bf2993a0f6d..5ce00692b57 100644 --- a/src/main/github/client/lookup/pull-request-lookup-hydration.ts +++ b/src/main/github/client/lookup/pull-request-lookup-hydration.ts @@ -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 } : {}), diff --git a/src/main/github/client/map/work-item.ts b/src/main/github/client/map/work-item.ts index d127c1a6e67..5916119f330 100644 --- a/src/main/github/client/map/work-item.ts +++ b/src/main/github/client/map/work-item.ts @@ -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' diff --git a/src/main/github/client/merge/pr-auto-merge.ts b/src/main/github/client/merge/pr-auto-merge.ts index 45c970662ec..eb314cc963c 100644 --- a/src/main/github/client/merge/pr-auto-merge.ts +++ b/src/main/github/client/merge/pr-auto-merge.ts @@ -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 = { @@ -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] } } diff --git a/src/renderer/src/components/github-pr-merge-state.test.ts b/src/renderer/src/components/github-pr-merge-state.test.ts index 036d34c4c0d..01bb9aba770 100644 --- a/src/renderer/src/components/github-pr-merge-state.test.ts +++ b/src/renderer/src/components/github-pr-merge-state.test.ts @@ -32,6 +32,18 @@ describe('presentGitHubPRMergeState', () => { }) }) + it('blocks conflicts on merge-queue branches', () => { + expect( + presentGitHubPRMergeState( + pr({ mergeQueueRequired: true, mergeable: 'UNKNOWN', mergeStateStatus: 'DIRTY' }) + ) + ).toMatchObject({ + label: 'Conflicts', + directMergeAvailable: false, + autoMergeAction: null + }) + }) + it('offers enable auto-merge for open PRs that are waiting on requirements', () => { expect(presentGitHubPRMergeState(pr({ mergeQueueRequired: null })).autoMergeAction).toBeNull() // Approval-required and required-check-blocked PRs are exactly when auto-merge helps. @@ -78,24 +90,31 @@ describe('presentGitHubPRMergeState', () => { }) }) - it('does not offer enable auto-merge for GitHub unstable PRs', () => { - expect( - presentGitHubPRMergeState( - pr({ - mergeable: 'UNKNOWN', - mergeStateStatus: 'UNSTABLE', - checksSummary: { - state: 'failure', - total: 1, - passed: 0, - failed: 1, - pending: 0, - neutral: 0 - } - }) - ).autoMergeAction - ).toBeNull() - }) + it.each(['UNSTABLE', 'HAS_HOOKS'])( + 'allows a direct merge for %s even before conflict detection finishes', + (mergeStateStatus) => { + expect( + presentGitHubPRMergeState( + pr({ + mergeable: 'UNKNOWN', + mergeStateStatus, + checksSummary: { + state: 'failure', + total: 1, + passed: 0, + failed: 1, + pending: 0, + neutral: 0 + } + }) + ) + ).toMatchObject({ + label: 'Checks failed', + directMergeAvailable: true, + autoMergeAction: null + }) + } + ) it('does not offer enable auto-merge on conflicting PRs (GitHub would reject it)', () => { expect( diff --git a/src/renderer/src/components/github-pr-merge-state.ts b/src/renderer/src/components/github-pr-merge-state.ts index f26edd5896b..b91bca323d5 100644 --- a/src/renderer/src/components/github-pr-merge-state.ts +++ b/src/renderer/src/components/github-pr-merge-state.ts @@ -1,23 +1,13 @@ -import type { - CheckStatus, - PRMergeableState, - PRReviewDecision, - PRState, - ProviderCheckSummary -} from '../../../shared/github/pull-request-types' -import { canEnableGitHubPRAutoMerge } from '../../../shared/github/pull-request-auto-merge-availability' +import type { CheckStatus, ProviderCheckSummary } from '../../../shared/github/pull-request-types' +import { + canShowGitHubPRAutoMergeControl, + type GitHubPRAutoMergeAvailabilityInput +} from '../../../shared/github/pull-request-auto-merge-availability' import { translate } from '@/i18n/i18n' -export type GitHubPRMergeStateInput = { - state: PRState | 'open' | 'closed' | 'merged' | 'draft' - mergeable?: PRMergeableState - mergeStateStatus?: string | null - reviewDecision?: PRReviewDecision | null +export type GitHubPRMergeStateInput = GitHubPRAutoMergeAvailabilityInput & { checksStatus?: CheckStatus checksSummary?: ProviderCheckSummary - autoMergeEnabled?: boolean - autoMergeAllowed?: boolean | null - mergeQueueRequired?: boolean | null } export type GitHubPRAutoMergeAction = { @@ -55,21 +45,38 @@ function hasFullMergeMetadata(item: GitHubPRMergeStateInput): boolean { return item.mergeable !== undefined || item.mergeStateStatus !== undefined } -// Why: GitHub rejects enabling auto-merge on a conflicting PR, so offering it -// there only yields an error toast. Repos can also disable auto-merge entirely, -// so suppress the action when GitHub explicitly reports that setting is off. -function canEnableAutoMerge(item: GitHubPRMergeStateInput): boolean { - return canEnableGitHubPRAutoMerge(item) -} - -// Why: when GitHub already allows a direct merge, offering "Enable auto-merge" -// only yields a "clean status" rejection. Keep the Disable action so users can -// still turn off an existing auto-merge request; merge-queue "Merge when ready" -// paths set directMergeAvailable=false and never reach here. -function autoMergeActionWhenDirectMergeAvailable( - autoMergeAction: GitHubPRAutoMergeAction | null -): GitHubPRAutoMergeAction | null { - return autoMergeAction?.kind === 'disable' ? autoMergeAction : null +function presentAutoMergeAction(item: GitHubPRMergeStateInput): GitHubPRAutoMergeAction | null { + if (!canShowGitHubPRAutoMergeControl(item)) { + return null + } + if (item.autoMergeEnabled === true) { + return { + kind: 'disable', + label: translate('auto.components.github.pr.merge.state.48d75ae118', 'Disable auto-merge'), + tooltip: translate( + 'auto.components.github.pr.merge.state.62703b1dc4', + 'GitHub auto-merge is enabled for this pull request' + ) + } + } + if (item.mergeQueueRequired === true) { + return { + kind: 'enable', + label: translate('auto.components.github.pr.merge.state.b169f943e1', 'Merge when ready'), + tooltip: translate( + 'auto.components.github.pr.merge.state.331ebe1170', + 'Add this pull request to the GitHub merge queue' + ) + } + } + return { + kind: 'enable', + label: translate('auto.components.github.pr.merge.state.4ab19a62ef', 'Enable auto-merge'), + tooltip: translate( + 'auto.components.github.pr.merge.state.8f6cb3772f', + 'Merge this pull request automatically once requirements are met' + ) + } } function passedChecksMergePresentation( @@ -83,53 +90,14 @@ function passedChecksMergePresentation( 'Checks passed. Merge eligibility will be checked again before merging.' ), directMergeAvailable: true, - autoMergeAction: autoMergeActionWhenDirectMergeAvailable(autoMergeAction) + autoMergeAction } } export function presentGitHubPRMergeState( item: GitHubPRMergeStateInput ): GitHubPRMergeStatePresentation { - const autoMergeAction = - item.state !== 'open' - ? null - : item.autoMergeEnabled === true - ? { - kind: 'disable' as const, - label: translate( - 'auto.components.github.pr.merge.state.48d75ae118', - 'Disable auto-merge' - ), - tooltip: translate( - 'auto.components.github.pr.merge.state.62703b1dc4', - 'GitHub auto-merge is enabled for this pull request' - ) - } - : item.mergeQueueRequired === true - ? { - kind: 'enable' as const, - label: translate( - 'auto.components.github.pr.merge.state.b169f943e1', - 'Merge when ready' - ), - tooltip: translate( - 'auto.components.github.pr.merge.state.331ebe1170', - 'Add this pull request to the GitHub merge queue' - ) - } - : canEnableAutoMerge(item) - ? { - kind: 'enable' as const, - label: translate( - 'auto.components.github.pr.merge.state.4ab19a62ef', - 'Enable auto-merge' - ), - tooltip: translate( - 'auto.components.github.pr.merge.state.8f6cb3772f', - 'Merge this pull request automatically once requirements are met' - ) - } - : null + const autoMergeAction = presentAutoMergeAction(item) if (item.state === 'merged') { return { @@ -191,6 +159,18 @@ export function presentGitHubPRMergeState( autoMergeAction } } + if (item.mergeable === 'CONFLICTING' || item.mergeStateStatus === 'DIRTY') { + return { + label: translate('auto.components.github.pr.merge.state.7e8bbe3cd7', 'Conflicts'), + tone: DANGER_TONE, + tooltip: translate( + 'auto.components.github.pr.merge.state.b37d45bca9', + 'GitHub reports merge conflicts' + ), + directMergeAvailable: false, + autoMergeAction + } + } if (item.mergeQueueRequired === true) { return { label: item.autoMergeEnabled ? 'Auto-merge on' : 'Merge when ready', @@ -220,18 +200,6 @@ export function presentGitHubPRMergeState( autoMergeAction } } - if (item.mergeable === 'CONFLICTING' || item.mergeStateStatus === 'DIRTY') { - return { - label: translate('auto.components.github.pr.merge.state.7e8bbe3cd7', 'Conflicts'), - tone: DANGER_TONE, - tooltip: translate( - 'auto.components.github.pr.merge.state.b37d45bca9', - 'GitHub reports merge conflicts' - ), - directMergeAvailable: false, - autoMergeAction - } - } if (item.mergeStateStatus === 'BEHIND') { return { label: translate('auto.components.github.pr.merge.state.039c072f94', 'Behind'), @@ -256,7 +224,12 @@ export function presentGitHubPRMergeState( autoMergeAction } } - if (item.mergeable === 'MERGEABLE' || item.mergeStateStatus === 'CLEAN') { + if ( + item.mergeable === 'MERGEABLE' || + item.mergeStateStatus === 'CLEAN' || + item.mergeStateStatus === 'UNSTABLE' || + item.mergeStateStatus === 'HAS_HOOKS' + ) { const checkState = checksState(item) const checkStatus = checkState === 'failure' @@ -290,7 +263,7 @@ export function presentGitHubPRMergeState( ? 'GitHub says this PR can merge and checks passed' : 'GitHub says this PR can merge'), directMergeAvailable: true, - autoMergeAction: autoMergeActionWhenDirectMergeAvailable(autoMergeAction) + autoMergeAction } } // Why: GitHub may still report intermediate mergeability while checks are diff --git a/src/renderer/src/components/right-sidebar/HostedReviewActions.draft.test.tsx b/src/renderer/src/components/right-sidebar/HostedReviewActions.draft.test.tsx index fcf501216ee..2d0c05c596d 100644 --- a/src/renderer/src/components/right-sidebar/HostedReviewActions.draft.test.tsx +++ b/src/renderer/src/components/right-sidebar/HostedReviewActions.draft.test.tsx @@ -5,6 +5,8 @@ import type { Repo } from '../../../../shared/repo-types' import type { Worktree } from '../../../../shared/worktree/types' import HostedReviewActions from './HostedReviewActions' import type { HostedReviewActionInfo } from './use-hosted-review-actions' +import type { PRInfo } from '../../../../shared/github/pull-request-types' +import { gitHubPRToChecksPanelReview } from './checks-panel-review' const actionMocks = vi.hoisted(() => ({ handleMarkReadyForReview: vi.fn(), @@ -73,3 +75,97 @@ describe('HostedReviewActions draft state', () => { } ) }) + +describe('HostedReviewActions GitHub merge selection', () => { + it.each([ + { + mergeable: 'MERGEABLE', + mergeStateStatus: 'UNSTABLE', + status: 'pending', + label: 'Squash and merge', + autoMerge: false + }, + { + mergeable: 'MERGEABLE', + mergeStateStatus: 'UNSTABLE', + status: 'failure', + label: 'Squash and merge', + autoMerge: false + }, + { + mergeable: 'UNKNOWN', + mergeStateStatus: 'UNKNOWN', + status: 'pending', + label: 'Checking', + autoMerge: false + }, + { mergeable: 'UNKNOWN', status: 'pending', label: 'Checking', autoMerge: false }, + { + mergeable: 'MERGEABLE', + mergeStateStatus: 'BLOCKED', + status: 'pending', + label: 'Enable auto-merge', + autoMerge: true + }, + { + mergeable: 'MERGEABLE', + mergeStateStatus: 'BLOCKED', + reviewDecision: 'REVIEW_REQUIRED', + status: 'success', + label: 'Enable auto-merge', + autoMerge: true + }, + { + mergeable: 'MERGEABLE', + mergeStateStatus: 'BLOCKED', + autoMergeEnabled: true, + status: 'pending', + label: 'Disable auto-merge', + autoMerge: true + }, + { + mergeable: 'MERGEABLE', + mergeStateStatus: 'UNSTABLE', + mergeQueueRequired: true, + status: 'pending', + label: 'Merge when ready', + autoMerge: false + }, + { + mergeable: 'CONFLICTING', + mergeStateStatus: 'DIRTY', + mergeQueueRequired: true, + status: 'pending', + label: 'Conflicts', + autoMerge: false + } + ] as const)('renders $label for $mergeStateStatus / $status', (scenario) => { + const pr: PRInfo = { + number: 31259, + title: 'fix(i18n): restore localized editor safety copy', + state: 'open', + url: 'https://github.com/stablyai/orca/pull/31259', + updatedAt: '2026-10-21T16:31:00Z', + headSha: 'e4353ead012043474fc723e50aea198a76d32979', + autoMergeAllowed: true, + checksStatus: scenario.status, + ...scenario + } + const markup = renderToStaticMarkup( + + ) + expect(markup).toContain(scenario.label) + expect(markup.includes('Enable auto-merge') || markup.includes('Disable auto-merge')).toBe( + scenario.autoMerge + ) + if (scenario.label === 'Checking' || scenario.label === 'Conflicts') { + expect(markup).toMatch(/]*disabled=""/) + } + }) +}) diff --git a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx index a660ec7a210..9cd0721ee49 100644 --- a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx +++ b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { PRInfo } from '../../../../shared/github/pull-request-types' import type { Repo } from '../../../../shared/repo-types' import { useHostedReviewActions, type HostedReviewActionInfo } from './use-hosted-review-actions' +import { presentGitHubPRMergeState } from '../github-pr-merge-state' const confirmationMocks = vi.hoisted(() => ({ confirm: vi.fn() @@ -65,7 +66,9 @@ function HookProbe(props: { shortLabel: 'PR', reviewLabel: 'pull request', defaultMergeMethod: 'squash', - autoMergeAction: null, + autoMergeAction: props.pullRequest + ? presentGitHubPRMergeState(props.pullRequest).autoMergeAction + : null, onRefreshReview: props.onRefreshReview }) return null @@ -118,6 +121,35 @@ describe('useHostedReviewActions', () => { document.body.replaceChildren() }) + it('refreshes stale eligibility after auto-merge fails while retaining the provider error', async () => { + const pr: PRInfo = { + number: review.number, + title: 'PR', + state: 'open', + url: 'https://github.com/stablyai/orca/pull/1015', + updatedAt: '2026-10-21T16:31:00Z', + mergeable: 'MERGEABLE', + mergeStateStatus: 'BLOCKED', + checksStatus: 'pending', + autoMergeAllowed: true, + prRepo + } + const error = + 'Failed to enable auto-merge for pull request: gh: Pull request is in unstable status' + vi.mocked(window.api.gh.setPRAutoMerge).mockResolvedValue({ ok: false, error }) + const onRefreshReview = vi.fn().mockRejectedValue(new Error('refresh unavailable')) + await renderHook(makeRepo(), onRefreshReview, pr) + + await act(async () => { + await latest?.handleAutoMerge() + }) + + expect(latest?.actionError).toBe(error) + expect(latest?.merging).toBe(false) + expect(onRefreshReview).toHaveBeenCalledOnce() + expect(window.api.gh.mergePR).not.toHaveBeenCalled() + }) + it('routes runtime-owned GitHub PR merges through the runtime host', async () => { const { onRefreshReview } = await renderHook(makeRepo({ executionHostId: 'runtime:env-1' })) diff --git a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts index 97d7c0bbcf8..b11e85e6a69 100644 --- a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts +++ b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts @@ -155,6 +155,8 @@ export function useHostedReviewActions({ }) if (!result.ok) { setActionError(result.error) + // Refresh stale eligibility without replacing the provider's mutation error. + await onRefreshReview().catch(() => {}) } else { await onRefreshReview() } diff --git a/src/shared/github/pull-request-auto-merge-availability.test.ts b/src/shared/github/pull-request-auto-merge-availability.test.ts index 8cfe706476e..9f1d3d9ada2 100644 --- a/src/shared/github/pull-request-auto-merge-availability.test.ts +++ b/src/shared/github/pull-request-auto-merge-availability.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from 'vitest' import { - canEnableGitHubPRAutoMerge, canShowGitHubPRAutoMergeControl, type GitHubPRAutoMergeAvailabilityInput } from './pull-request-auto-merge-availability' @@ -18,34 +17,55 @@ function pr( } describe('github PR auto-merge availability', () => { + it.each([undefined, null, 'UNKNOWN', 'FUTURE_STATUS'])( + 'does not infer an auto-merge requirement from unresolved status %s', + (mergeStateStatus) => { + const item = pr({ mergeable: 'UNKNOWN', mergeStateStatus }) + expect(canShowGitHubPRAutoMergeControl(item)).toBe(false) + } + ) + it('does not offer auto-merge for directly mergeable PRs with only optional checks pending', () => { - expect(canEnableGitHubPRAutoMerge(pr())).toBe(false) expect(canShowGitHubPRAutoMergeControl(pr())).toBe(false) }) it('offers auto-merge for requirement-blocked PRs', () => { expect( - canEnableGitHubPRAutoMerge( + canShowGitHubPRAutoMergeControl( pr({ mergeable: 'UNKNOWN', mergeStateStatus: 'BLOCKED', reviewDecision: 'REVIEW_REQUIRED' }) ) ).toBe(true) - expect(canEnableGitHubPRAutoMerge(pr({ mergeStateStatus: 'BLOCKED' }))).toBe(true) + expect(canShowGitHubPRAutoMergeControl(pr({ mergeStateStatus: 'BLOCKED' }))).toBe(true) + expect(canShowGitHubPRAutoMergeControl(pr({ mergeStateStatus: 'BEHIND' }))).toBe(true) expect( canShowGitHubPRAutoMergeControl(pr({ mergeable: 'UNKNOWN', mergeStateStatus: 'BLOCKED' })) ).toBe(true) }) it('keeps merge-queue branches available for merge-when-ready', () => { - expect(canEnableGitHubPRAutoMerge(pr({ mergeQueueRequired: true }))).toBe(false) expect(canShowGitHubPRAutoMergeControl(pr({ mergeQueueRequired: true }))).toBe(true) expect( canShowGitHubPRAutoMergeControl(pr({ autoMergeAllowed: false, mergeQueueRequired: true })) ).toBe(true) + expect( + canShowGitHubPRAutoMergeControl( + pr({ mergeStateStatus: 'UNSTABLE', mergeQueueRequired: true }) + ) + ).toBe(true) }) it('keeps enabled auto-merge visible so users can disable it', () => { - expect(canEnableGitHubPRAutoMerge(pr({ autoMergeEnabled: true }))).toBe(false) expect(canShowGitHubPRAutoMergeControl(pr({ autoMergeEnabled: true }))).toBe(true) + expect( + canShowGitHubPRAutoMergeControl( + pr({ + autoMergeEnabled: true, + autoMergeAllowed: false, + mergeable: 'CONFLICTING', + mergeStateStatus: 'DIRTY' + }) + ) + ).toBe(true) }) it('suppresses closed, draft, disallowed, conflicting, and unstable PRs', () => { @@ -63,9 +83,6 @@ describe('github PR auto-merge availability', () => { expect( canShowGitHubPRAutoMergeControl(pr({ mergeable: 'CONFLICTING', mergeStateStatus: 'DIRTY' })) ).toBe(false) - expect( - canEnableGitHubPRAutoMerge(pr({ mergeable: 'UNKNOWN', mergeStateStatus: 'UNSTABLE' })) - ).toBe(false) expect( canShowGitHubPRAutoMergeControl(pr({ mergeable: 'UNKNOWN', mergeStateStatus: 'UNSTABLE' })) ).toBe(false) diff --git a/src/shared/github/pull-request-auto-merge-availability.ts b/src/shared/github/pull-request-auto-merge-availability.ts index 867224fc0ee..533760baf76 100644 --- a/src/shared/github/pull-request-auto-merge-availability.ts +++ b/src/shared/github/pull-request-auto-merge-availability.ts @@ -1,7 +1,7 @@ import type { PRMergeableState, PRReviewDecision, PRState } from './pull-request-types' export type GitHubPRAutoMergeAvailabilityInput = { - state: PRState | 'open' | 'closed' | 'merged' | 'draft' + state: PRState mergeable?: PRMergeableState mergeStateStatus?: string | null reviewDecision?: PRReviewDecision | null @@ -10,51 +10,26 @@ export type GitHubPRAutoMergeAvailabilityInput = { mergeQueueRequired?: boolean | null } -function isOpenPR(item: GitHubPRAutoMergeAvailabilityInput): boolean { - return item.state === 'open' -} - -function isConflicting(item: GitHubPRAutoMergeAvailabilityInput): boolean { - return item.mergeable === 'CONFLICTING' || item.mergeStateStatus === 'DIRTY' -} - -function isUnstable(item: GitHubPRAutoMergeAvailabilityInput): boolean { - return item.mergeStateStatus === 'UNSTABLE' -} - -function hasReviewRequirement(item: GitHubPRAutoMergeAvailabilityInput): boolean { - return item.reviewDecision === 'REVIEW_REQUIRED' || item.reviewDecision === 'CHANGES_REQUESTED' -} - -function canMergeImmediately(item: GitHubPRAutoMergeAvailabilityInput): boolean { - if (item.mergeStateStatus === 'BLOCKED' || item.mergeStateStatus === 'BEHIND') { +export function canShowGitHubPRAutoMergeControl(item: GitHubPRAutoMergeAvailabilityInput): boolean { + if (item.state !== 'open') { return false } - return item.mergeable === 'MERGEABLE' || item.mergeStateStatus === 'CLEAN' -} - -function canRequestWhenReady(item: GitHubPRAutoMergeAvailabilityInput): boolean { - // Why: GitHub auto-merge waits on unmet requirements; UNSTABLE is rejected - // by the mutation rather than becoming a waitable auto-merge request. - if (!isOpenPR(item) || isConflicting(item) || isUnstable(item)) { + if (item.autoMergeEnabled === true) { + return true + } + if (item.mergeable === 'CONFLICTING' || item.mergeStateStatus === 'DIRTY') { return false } if (item.mergeQueueRequired === true) { return true } + // Missing merge metadata is not evidence of an unmet branch requirement. return ( - item.autoMergeAllowed !== false && (hasReviewRequirement(item) || !canMergeImmediately(item)) + item.mergeStateStatus !== 'UNSTABLE' && + item.autoMergeAllowed !== false && + (item.reviewDecision === 'REVIEW_REQUIRED' || + item.reviewDecision === 'CHANGES_REQUESTED' || + item.mergeStateStatus === 'BLOCKED' || + item.mergeStateStatus === 'BEHIND') ) } - -export function canEnableGitHubPRAutoMerge(item: GitHubPRAutoMergeAvailabilityInput): boolean { - return ( - item.autoMergeEnabled !== true && item.mergeQueueRequired !== true && canRequestWhenReady(item) - ) -} - -export function canShowGitHubPRAutoMergeControl(item: GitHubPRAutoMergeAvailabilityInput): boolean { - // Why: GitHub auto-merge waits for branch requirements, not arbitrary optional CI. - // Keep already-enabled PRs visible so users can disable the setting. - return isOpenPR(item) && (item.autoMergeEnabled === true || canRequestWhenReady(item)) -}