diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index e56627b4509..ec9079c63cc 100644 --- a/src/main/github/client.test.ts +++ b/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({ diff --git a/src/main/github/client.ts b/src/main/github/client.ts index bf472b11372..73feeb4cc3a 100644 --- a/src/main/github/client.ts +++ b/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 diff --git a/src/main/github/pr-refresh-coordinator.test.ts b/src/main/github/pr-refresh-coordinator.test.ts index ce924f29e35..57e20095141 100644 --- a/src/main/github/pr-refresh-coordinator.test.ts +++ b/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) + }) }) diff --git a/src/main/github/pr-refresh-coordinator.ts b/src/main/github/pr-refresh-coordinator.ts index c35baa9b240..58965406cbf 100644 --- a/src/main/github/pr-refresh-coordinator.ts +++ b/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 { 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 { 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 } 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 761a5fdc555..f71fc03d25c 100644 --- a/src/renderer/src/components/github-pr-merge-state.test.ts +++ b/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 diff --git a/src/renderer/src/components/github-pr-merge-state.ts b/src/renderer/src/components/github-pr-merge-state.ts index d20665972e3..8c8f1cd89f5 100644 --- a/src/renderer/src/components/github-pr-merge-state.ts +++ b/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 }