diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index af2677f4e..6b5e018bd 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -618,6 +618,31 @@ describe('getPRForBranch', () => { }) }) + it('omits conflict summaries for SSH-backed repos', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ 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-03-28T00:00:00Z', + draft: false, + mergeable_state: 'dirty', + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'feature/test', sha: 'head-oid' } + } + ]) + }) + + const pr = await getPRForBranch('/remote/repo-root', 'feature/test', undefined, 'ssh-1') + + expect(pr?.mergeable).toBe('CONFLICTING') + expect(pr?.conflictSummary).toBeUndefined() + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + it('keeps conflicted file paths when git merge-tree exits 1 with stdout', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock.mockResolvedValueOnce({ @@ -972,7 +997,23 @@ describe('GitHub GraphQL rate-limit guard', () => { }) it('uses explicit PR repo for merge and title mutations', async () => { - ghExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 7, + title: 'PR', + state: 'OPEN', + url: 'https://github.com/stablyai/orca/pull/7', + statusCheckRollup: [], + updatedAt: '2026-04-01T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + baseRefOid: 'base-oid', + headRefOid: 'head-oid' + }) + }) + .mockResolvedValue({ stdout: '', stderr: '' }) await expect( mergePR('/repo-root', 7, 'squash', undefined, { owner: 'stablyai', repo: 'orca' }) @@ -984,6 +1025,19 @@ describe('GitHub GraphQL rate-limit guard', () => { expect(getOwnerRepoMock).not.toHaveBeenCalled() expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( 1, + [ + 'pr', + 'view', + '7', + '--repo', + 'stablyai/orca', + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: '/repo-root' } + ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 2, ['pr', 'merge', '7', '--squash', '--repo', 'stablyai/orca'], expect.objectContaining({ cwd: '/repo-root', @@ -991,12 +1045,67 @@ describe('GitHub GraphQL rate-limit guard', () => { }) ) expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 2, + 3, ['pr', 'edit', '7', '--title', 'New title', '--repo', 'stablyai/orca'], { cwd: '/repo-root' } ) }) + it('returns conflicting file details instead of running gh merge when PR is dirty', async () => { + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 7, + title: 'PR', + state: 'OPEN', + url: 'https://github.com/stablyai/orca/pull/7', + statusCheckRollup: [], + updatedAt: '2026-04-01T00:00:00Z', + isDraft: false, + mergeable: 'CONFLICTING', + baseRefName: 'main', + baseRefOid: 'base-oid', + headRefOid: 'head-oid' + }) + }) + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: '' }) + .mockResolvedValueOnce({ stdout: 'latest-base-oid\n' }) + .mockResolvedValueOnce({ stdout: 'merge-base-oid\n' }) + .mockResolvedValueOnce({ stdout: '3\n' }) + .mockResolvedValueOnce({ stdout: 'result-tree-oid\u0000src/conflict.ts\u0000' }) + + await expect( + mergePR('/repo-root', 7, 'squash', undefined, { owner: 'stablyai', repo: 'orca' }) + ).resolves.toEqual({ + ok: false, + error: + 'This pull request has merge conflicts and cannot be merged yet.\n' + + '3 commits behind main (base commit: latest-).\n\n' + + 'Conflicting files:\n' + + '- src/conflict.ts' + }) + + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('does not run merge conflict preflight for SSH-backed repos', async () => { + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect( + mergePR('/remote/repo-root', 7, 'squash', 'ssh-1', { owner: 'stablyai', repo: 'orca' }) + ).resolves.toEqual({ ok: true }) + + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + ['pr', 'merge', '7', '--squash', '--repo', 'stablyai/orca'], + expect.objectContaining({ + env: expect.objectContaining({ GH_PROMPT_DISABLED: '1' }) + }) + ) + expect(ghExecFileAsyncMock.mock.calls[0]?.[1]).not.toHaveProperty('cwd') + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + it('blocks review-thread resolve mutations before spawning gh when GraphQL is low', async () => { rateLimitGuardMock.mockReturnValue({ blocked: true, diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 8428be0cf..e533b9204 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -6,6 +6,7 @@ import type { IssueSourcePreference, ListWorkItemsResult, PRInfo, + PRConflictSummary, PRRefreshOutcome, PRMergeableState, PRCheckDetail, @@ -74,6 +75,8 @@ import { type RateLimitBucketKind } from './rate-limit' +type GhExecOptions = ReturnType + const ORCA_REPO = 'stablyai/orca' async function assertRateLimitBudget(bucket: RateLimitBucketKind): Promise { @@ -2706,6 +2709,17 @@ export async function mergePR( const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId)) await acquire() try { + const mergeBlocker = await getPRMergeBlocker( + repoPath, + prNumber, + ownerRepo, + ghOptions, + connectionId + ) + if (mergeBlocker) { + return { ok: false, error: mergeBlocker } + } + // Don't use --delete-branch: it tries to delete the local branch which // fails when the user's worktree is checked out on it. Branch cleanup // is handled by worktree deletion (local) and GitHub's auto-delete setting (remote). @@ -2727,6 +2741,53 @@ export async function mergePR( } } +async function getPRMergeBlocker( + repoPath: string, + prNumber: number, + ownerRepo: OwnerRepo | null, + ghOptions: GhExecOptions, + connectionId?: string | null +): Promise { + // Why: conflict summaries shell out to local git; SSH repo paths are remote-only + // until that helper is routed through the SSH git provider. + if (!ownerRepo || connectionId) { + return null + } + + try { + const pr = await getPRByNumber(ownerRepo, prNumber, ghOptions) + if (pr?.mergeable !== 'CONFLICTING' || !pr.baseRefName || !pr.baseRefOid || !pr.headRefOid) { + return null + } + + const summary = await getPRConflictSummary( + repoPath, + pr.baseRefName, + pr.baseRefOid, + pr.headRefOid + ) + return formatMergeConflictBlocker(pr.baseRefName, summary) + } catch { + // Why: conflict preflight should improve stale UI diagnostics, not make + // merge impossible when the lookup endpoint has a transient failure. + return null + } +} + +function formatMergeConflictBlocker( + baseRefName: string, + summary: PRConflictSummary | undefined +): string { + const heading = 'This pull request has merge conflicts and cannot be merged yet.' + if (!summary || summary.files.length === 0) { + return `${heading}\nUpdate the branch with ${baseRefName} and resolve the conflicts before merging.` + } + + const files = summary.files.map((file) => `- ${file}`).join('\n') + const behind = `${summary.commitsBehind} commit${summary.commitsBehind === 1 ? '' : 's'} behind ${baseRefName}` + return `${heading}\n${behind} (base commit: ${summary.baseCommit}).\n\nConflicting files:\n${files}` +} + export async function updatePRState( repoPath: string, prNumber: number, diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 05805390b..22ab974f6 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -2806,7 +2806,6 @@ function PRActionsPanel({ 'w-full justify-center gap-2 bg-green-600 text-white hover:bg-green-700', 'disabled:cursor-not-allowed disabled:opacity-50' )} - disabled={mergePending || localState === 'closed' || localState === 'merged'} > {mergePending ? (