fix: address review findings (#4328)

src/main/github/client.test.ts

@@ -1015,6 +1015,42 @@ describe('getPRForBranch', () => {
     })
   })
 
+  it('treats GitHub DIRTY merge state as conflicting when mergeable is still unknown', async () => {
+    getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
+    ghExecFileAsyncMock
+      .mockResolvedValueOnce({
+        stdout: JSON.stringify({
+          number: 42,
+          title: 'Fix PR discovery',
+          state: 'OPEN',
+          url: 'https://github.com/acme/widgets/pull/42',
+          statusCheckRollup: [],
+          updatedAt: '2026-03-28T00:00:00Z',
+          isDraft: false,
+          mergeable: 'UNKNOWN',
+          mergeStateStatus: 'DIRTY',
+          baseRefName: 'main',
+          headRefName: 'feature/test',
+          baseRefOid: 'base-oid',
+          headRefOid: 'head-oid'
+        })
+      })
+      .mockResolvedValueOnce({
+        stdout: JSON.stringify({ data: { repository: { mergeQueue: null } } })
+      })
+    gitExecFileAsyncMock
+      .mockResolvedValueOnce({ stdout: '' })
+      .mockResolvedValueOnce({ stdout: 'latest-base-oid\n' })
+      .mockResolvedValueOnce({ stdout: 'merge-base-oid\n' })
+      .mockResolvedValueOnce({ stdout: '1\n' })
+      .mockResolvedValueOnce({ stdout: 'result-tree-oid\u0000src/conflict.ts\u0000' })
+
+    const pr = await getPRForBranch('/repo-root', 'feature/test', 42)
+
+    expect(pr?.mergeable).toBe('CONFLICTING')
+    expect(pr?.conflictSummary?.files).toEqual(['src/conflict.ts'])
+  })
+
   it('omits conflict summaries for SSH-backed repos', async () => {
     getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
     ghExecFileAsyncMock.mockResolvedValueOnce({

src/main/github/client.ts

@@ -1808,6 +1808,14 @@ function mapRestPRMergeable(pr: RestPullRequest): PRMergeableState {
   return 'UNKNOWN'
 }
 
+function derivePullRequestMergeable(data: PullRequestLookupData): PRMergeableState {
+  const mergeable = normalizePRMergeable(data.mergeable)
+  if (mergeable === 'CONFLICTING' || data.mergeStateStatus === 'DIRTY') {
+    return 'CONFLICTING'
+  }
+  return mergeable ?? 'UNKNOWN'
+}
+
 function mapRestPullRequest(pr: RestPullRequest): PullRequestLookupData {
   return {
     number: pr.number,
@@ -2324,9 +2332,10 @@ export async function getPRForBranchOutcome(
       return { kind: 'no-pr', fetchedAt: Date.now() }
     }
 
+    const mergeable = derivePullRequestMergeable(data)
     const conflictSummary =
       !connectionId &&
-      data.mergeable === 'CONFLICTING' &&
+      mergeable === 'CONFLICTING' &&
       data.baseRefName &&
       data.baseRefOid &&
       data.headRefOid
@@ -2343,7 +2352,7 @@ export async function getPRForBranchOutcome(
         url: data.url,
         checksStatus: deriveCheckStatus(data.statusCheckRollup),
         updatedAt: data.updatedAt,
-        mergeable: (data.mergeable as PRMergeableState) ?? 'UNKNOWN',
+        mergeable,
         ...(data.reviewDecision !== undefined ? { reviewDecision: data.reviewDecision } : {}),
         ...(data.autoMergeEnabled !== undefined ? { autoMergeEnabled: data.autoMergeEnabled } : {}),
         ...(data.mergeQueueRequired !== undefined

src/main/github/pr-refresh-coordinator.test.ts

@@ -481,4 +481,48 @@ describe('pr-refresh-coordinator', () => {
 
     expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(2)
   })
+
+  it('does a prompt visible follow-up after a manual refresh returns unknown mergeability', async () => {
+    const { refreshPRNow, reportVisiblePRRefreshCandidates } =
+      await import('./pr-refresh-coordinator')
+    const visibleCandidate = makeCandidate()
+    const candidate = makeCandidate({
+      cachedFetchedAt: Date.now(),
+      cachedHasPR: true,
+      cachedPRState: 'open',
+      cachedChecksStatus: 'success',
+      cachedMergeable: 'MERGEABLE',
+      cachedMergeStateStatus: 'CLEAN'
+    })
+    getPRForBranchOutcomeMock
+      .mockResolvedValueOnce({
+        kind: 'found',
+        pr: makePR({ checksStatus: 'success', mergeable: 'MERGEABLE' }),
+        fetchedAt: Date.now()
+      })
+      .mockResolvedValueOnce({
+        kind: 'found',
+        pr: makePR({ checksStatus: 'success', mergeable: 'UNKNOWN' }),
+        fetchedAt: Date.now()
+      })
+      .mockResolvedValueOnce({
+        kind: 'found',
+        pr: makePR({ checksStatus: 'success', mergeable: 'CONFLICTING' }),
+        fetchedAt: Date.now()
+      })
+
+    reportVisiblePRRefreshCandidates([visibleCandidate], 1, 1)
+    await vi.advanceTimersByTimeAsync(0)
+    await refreshPRNow(candidate)
+
+    expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(2)
+
+    await vi.advanceTimersByTimeAsync(2_499)
+
+    expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(2)
+
+    await vi.advanceTimersByTimeAsync(1)
+
+    expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(3)
+  })
 })

src/main/github/pr-refresh-coordinator.ts

@@ -20,6 +20,7 @@ type QueueEntry = {
   reason: GitHubPRRefreshReason
   priority: number
   dueAt: number
+  bypassBackgroundBudget?: boolean
   windowId?: number
 }
 
@@ -30,6 +31,7 @@ type PRRefreshOutcomeObserver = (
 
 const MIN_BACKGROUND_REFRESH_AGE_MS = 60_000
 const MERGEABILITY_PENDING_REFRESH_MS = 10_000
+const MANUAL_MERGEABILITY_PENDING_REFRESH_MS = 2_500
 const BACKGROUND_BUDGET_WINDOW_MS = 5 * 60_000
 const MIN_BACKGROUND_SPACING_MS = 10_000
 const BACKGROUND_BUDGET_MAX = 20
@@ -133,6 +135,10 @@ function isBudgetedBackground(reason: GitHubPRRefreshReason): boolean {
   return reason === 'visible' || reason === 'swr'
 }
 
+function isBudgetedQueueEntry(entry: QueueEntry): boolean {
+  return isBudgetedBackground(entry.reason) && entry.bypassBackgroundBudget !== true
+}
+
 function validateCandidate(
   candidate: GitHubPRRefreshCandidate
 ): GitHubPRRefreshSkippedReason | null {
@@ -273,7 +279,8 @@ function scheduleVisibleFollowUp(
   outcome: PRRefreshOutcome,
   priority: number,
   aliases: GitHubPRRefreshAlias[],
-  windowId?: number
+  windowId?: number,
+  options?: { pendingMergeabilityDelayMs?: number }
 ): void {
   if (!isVisibleKey(key)) {
     // Why: manual/active refreshes can remove the queued visible retry after
@@ -302,7 +309,15 @@ function scheduleVisibleFollowUp(
   }
   errorBackoff.delete(key)
   const followUpCandidate = visibleCandidateAfterOutcome(candidate, outcome)
-  const dueAt = freshRetryAt(followUpCandidate) ?? Date.now()
+  const regularDueAt = freshRetryAt(followUpCandidate) ?? Date.now()
+  const pendingMergeabilityDueAt =
+    options?.pendingMergeabilityDelayMs !== undefined && isMergeabilityPendingOutcome(outcome)
+      ? outcome.fetchedAt + options.pendingMergeabilityDelayMs
+      : null
+  const dueAt =
+    pendingMergeabilityDueAt === null
+      ? regularDueAt
+      : Math.min(regularDueAt, pendingMergeabilityDueAt)
   // Why: coalesced linked-PR refreshes may represent several local branches.
   // Preserve every alias for the next visible follow-up so all cache entries
   // keep receiving periodic updates.
@@ -313,6 +328,9 @@ function scheduleVisibleFollowUp(
     reason: 'visible',
     priority,
     dueAt,
+    // Why: this manual one-shot fixes GitHub's transient UNKNOWN state; visible
+    // spacing would otherwise delay it past the intended prompt retry window.
+    bypassBackgroundBudget: pendingMergeabilityDueAt !== null,
     windowId
   })
   scheduleDrain(Math.max(0, dueAt - Date.now()))
@@ -351,6 +369,15 @@ function hasResolvedMergeStateStatus(status: string | null | undefined): boolean
   return status === 'CLEAN' || status === 'BEHIND' || status === 'BLOCKED'
 }
 
+function isMergeabilityPendingOutcome(outcome: PRRefreshOutcome): boolean {
+  return (
+    outcome.kind === 'found' &&
+    outcome.pr.state === 'open' &&
+    outcome.pr.mergeable === 'UNKNOWN' &&
+    !hasResolvedMergeStateStatus(outcome.pr.mergeStateStatus)
+  )
+}
+
 function backgroundRefreshBuckets(): ('core' | 'graphql')[] {
   // Why: branch refreshes prefer REST but can still fall back to `gh pr list`
   // when local head-owner metadata is unavailable. Guard both buckets until the
@@ -422,7 +449,7 @@ async function drainQueue(): Promise<void> {
         return
       }
 
-      const budgetDelay = isBudgetedBackground(next.reason) ? nextBudgetDelay() : 0
+      const budgetDelay = isBudgetedQueueEntry(next) ? nextBudgetDelay() : 0
       if (budgetDelay > 0) {
         scheduleDrain(budgetDelay)
         return
@@ -479,7 +506,7 @@ async function drainQueue(): Promise<void> {
           scheduleDrain(Math.max(1_000, retryAt - Date.now()))
           continue
         }
-        if (isBudgetedBackground(next.reason)) {
+        if (isBudgetedQueueEntry(next)) {
           noteBackgroundStart()
         }
         for (const bucket of buckets) {
@@ -622,6 +649,10 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
   )
   outcomeObserver?.(candidate, outcome)
   broadcast({ aliases, reason: 'manual', outcome, requestStartedAt }, requestSequence)
-  scheduleVisibleFollowUp(key, candidate, outcome, 40, aliases)
+  scheduleVisibleFollowUp(key, candidate, outcome, 40, aliases, undefined, {
+    // Why: GitHub often reports UNKNOWN immediately after `gh pr reopen`;
+    // do one prompt visible retry so conflicts replace the transient label.
+    pendingMergeabilityDelayMs: MANUAL_MERGEABILITY_PENDING_REFRESH_MS
+  })
   return outcome
 }

src/renderer/src/components/github-pr-merge-state.test.ts

@@ -42,6 +42,9 @@ describe('presentGitHubPRMergeState', () => {
     expect(
       presentGitHubPRMergeState(pr({ mergeable: 'CONFLICTING', mergeStateStatus: 'DIRTY' }))
     ).toMatchObject({ label: 'Conflicts', directMergeAvailable: false })
+    expect(
+      presentGitHubPRMergeState(pr({ mergeable: 'UNKNOWN', mergeStateStatus: 'DIRTY' }))
+    ).toMatchObject({ label: 'Conflicts', directMergeAvailable: false })
     expect(presentGitHubPRMergeState(pr({ mergeStateStatus: 'BEHIND' }))).toMatchObject({
       label: 'Behind',
       directMergeAvailable: false
@@ -62,6 +65,15 @@ describe('presentGitHubPRMergeState', () => {
     })
   })
 
+  it('labels unresolved GitHub mergeability as checking', () => {
+    expect(
+      presentGitHubPRMergeState(pr({ mergeable: 'UNKNOWN', mergeStateStatus: null }))
+    ).toMatchObject({
+      label: 'Checking',
+      directMergeAvailable: false
+    })
+  })
+
   it('suppresses auto-merge actions for non-open PR states', () => {
     expect(
       presentGitHubPRMergeState(pr({ state: 'closed', mergeQueueRequired: true })).autoMergeAction

src/renderer/src/components/github-pr-merge-state.ts

@@ -131,7 +131,7 @@ export function presentGitHubPRMergeState(
       autoMergeAction
     }
   }
-  if (item.mergeable === 'CONFLICTING') {
+  if (item.mergeable === 'CONFLICTING' || item.mergeStateStatus === 'DIRTY') {
     return {
       label: 'Conflicts',
       tone: DANGER_TONE,
@@ -187,9 +187,9 @@ export function presentGitHubPRMergeState(
     }
   }
   return {
-    label: 'Unknown',
+    label: 'Checking',
     tone: MUTED_TONE,
-    tooltip: 'GitHub has not reported a final merge status',
+    tooltip: 'GitHub is still computing this pull request merge status',
     directMergeAvailable: false,
     autoMergeAction
   }