fix: address review findings (#2377)

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,

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<typeof ghRepoExecOptions>
+
 const ORCA_REPO = 'stablyai/orca'
 
 async function assertRateLimitBudget(bucket: RateLimitBucketKind): Promise<void> {
@@ -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<string | null> {
+  // 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,

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 ? (
                     <LoaderCircle className="size-3.5 animate-spin" />