mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
feat: surface merge conflict warning in checks panel (#206)
This commit is contained in:
@@ -32,7 +32,8 @@ describe('getPRForBranch', () => {
|
||||
url: 'https://github.com/acme/widgets/pull/42',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
isDraft: false
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE'
|
||||
}
|
||||
])
|
||||
})
|
||||
@@ -58,12 +59,13 @@ describe('getPRForBranch', () => {
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable'
|
||||
],
|
||||
{ cwd: '/repo-root', encoding: 'utf-8' }
|
||||
)
|
||||
expect(pr?.number).toBe(42)
|
||||
expect(pr?.state).toBe('open')
|
||||
expect(pr?.mergeable).toBe('MERGEABLE')
|
||||
})
|
||||
|
||||
it('falls back to gh pr view when the remote cannot be resolved to GitHub', async () => {
|
||||
@@ -75,7 +77,8 @@ describe('getPRForBranch', () => {
|
||||
url: 'https://example.com/pr/7',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
isDraft: true
|
||||
isDraft: true,
|
||||
mergeable: 'CONFLICTING'
|
||||
})
|
||||
})
|
||||
|
||||
@@ -89,12 +92,13 @@ describe('getPRForBranch', () => {
|
||||
'view',
|
||||
'feature/test',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable'
|
||||
],
|
||||
{ cwd: '/non-github-repo', encoding: 'utf-8' }
|
||||
)
|
||||
expect(pr?.number).toBe(7)
|
||||
expect(pr?.state).toBe('draft')
|
||||
expect(pr?.mergeable).toBe('CONFLICTING')
|
||||
})
|
||||
|
||||
it('returns null for empty branch (e.g. during rebase with detached HEAD)', async () => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { execFile } from 'child_process'
|
||||
import { promisify } from 'util'
|
||||
import type { PRInfo, IssueInfo, PRCheckDetail } from '../../shared/types'
|
||||
import type { PRInfo, PRMergeableState, IssueInfo, PRCheckDetail } from '../../shared/types'
|
||||
import {
|
||||
mapCheckRunRESTStatus,
|
||||
mapCheckRunRESTConclusion,
|
||||
@@ -94,6 +94,7 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
|
||||
statusCheckRollup: unknown[]
|
||||
updatedAt: string
|
||||
isDraft?: boolean
|
||||
mergeable: string
|
||||
} | null = null
|
||||
|
||||
if (ownerRepo) {
|
||||
@@ -111,7 +112,7 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable'
|
||||
],
|
||||
{
|
||||
cwd: repoPath,
|
||||
@@ -128,7 +129,7 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
|
||||
'view',
|
||||
branchName,
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable'
|
||||
],
|
||||
{
|
||||
cwd: repoPath,
|
||||
@@ -148,7 +149,8 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
|
||||
state: mapPRState(data.state, data.isDraft),
|
||||
url: data.url,
|
||||
checksStatus: deriveCheckStatus(data.statusCheckRollup),
|
||||
updatedAt: data.updatedAt
|
||||
updatedAt: data.updatedAt,
|
||||
mergeable: (data.mergeable as PRMergeableState) ?? 'UNKNOWN'
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
|
||||
@@ -1,19 +1,9 @@
|
||||
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
import {
|
||||
CircleCheck,
|
||||
CircleX,
|
||||
LoaderCircle,
|
||||
CircleDashed,
|
||||
ExternalLink,
|
||||
RefreshCw,
|
||||
Check,
|
||||
X,
|
||||
Pencil
|
||||
} from 'lucide-react'
|
||||
import { LoaderCircle, ExternalLink, RefreshCw, Check, X, Pencil } from 'lucide-react'
|
||||
import { useAppStore } from '@/store'
|
||||
import { cn } from '@/lib/utils'
|
||||
import PRActions from './PRActions'
|
||||
import { PullRequestIcon, CHECK_ICON, CHECK_COLOR, prStateColor } from './checks-helpers'
|
||||
import { PullRequestIcon, prStateColor, MergeConflictWarning, ChecksList } from './checks-helpers'
|
||||
import type { PRInfo, PRCheckDetail } from '../../../../shared/types'
|
||||
|
||||
export default function ChecksPanel(): React.JSX.Element {
|
||||
@@ -246,30 +236,6 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
)
|
||||
}
|
||||
|
||||
// ── Sorted checks: failures first, then pending, then success ──
|
||||
const sortedChecks = [...checks].sort((a, b) => {
|
||||
const order = {
|
||||
failure: 0,
|
||||
timed_out: 0,
|
||||
cancelled: 1,
|
||||
pending: 2,
|
||||
neutral: 3,
|
||||
skipped: 4,
|
||||
success: 5
|
||||
}
|
||||
const aOrder = order[a.conclusion ?? 'pending'] ?? 3
|
||||
const bOrder = order[b.conclusion ?? 'pending'] ?? 3
|
||||
return aOrder - bOrder
|
||||
})
|
||||
|
||||
const passingCount = checks.filter((c) => c.conclusion === 'success').length
|
||||
const failingCount = checks.filter(
|
||||
(c) => c.conclusion === 'failure' || c.conclusion === 'timed_out'
|
||||
).length
|
||||
const pendingCount = checks.filter(
|
||||
(c) => c.conclusion === 'pending' || c.conclusion === null
|
||||
).length
|
||||
|
||||
return (
|
||||
<div className="flex-1 overflow-auto scrollbar-sleek">
|
||||
{/* PR Header */}
|
||||
@@ -353,79 +319,17 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Merge conflict warning — surfaced from GitHub's mergeable state so the
|
||||
user knows they must resolve conflicts before the PR can merge. */}
|
||||
<MergeConflictWarning mergeable={pr.mergeable} />
|
||||
|
||||
{/* Merge / Delete Worktree actions */}
|
||||
{worktree && repo && (
|
||||
<PRActions pr={pr} repo={repo} worktree={worktree} onRefreshPR={handleRefreshPR} />
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Checks Summary */}
|
||||
{checks.length > 0 && (
|
||||
<div className="flex items-center gap-3 px-3 py-2 border-b border-border text-[10px] text-muted-foreground">
|
||||
{passingCount > 0 && (
|
||||
<span className="flex items-center gap-1">
|
||||
<CircleCheck className="size-3 text-emerald-500" />
|
||||
{passingCount} passing
|
||||
</span>
|
||||
)}
|
||||
{failingCount > 0 && (
|
||||
<span className="flex items-center gap-1">
|
||||
<CircleX className="size-3 text-rose-500" />
|
||||
{failingCount} failing
|
||||
</span>
|
||||
)}
|
||||
{pendingCount > 0 && (
|
||||
<span className="flex items-center gap-1">
|
||||
<LoaderCircle className="size-3 text-amber-500" />
|
||||
{pendingCount} pending
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Checks List */}
|
||||
{checksLoading && checks.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-8">
|
||||
<LoaderCircle className="size-5 animate-spin text-muted-foreground" />
|
||||
</div>
|
||||
) : checks.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-8 text-[11px] text-muted-foreground">
|
||||
No checks configured
|
||||
</div>
|
||||
) : (
|
||||
<div className="py-1">
|
||||
{sortedChecks.map((check) => {
|
||||
const conclusion = check.conclusion ?? 'pending'
|
||||
const Icon = CHECK_ICON[conclusion] ?? CircleDashed
|
||||
const color = CHECK_COLOR[conclusion] ?? 'text-muted-foreground'
|
||||
|
||||
return (
|
||||
<div
|
||||
key={check.name}
|
||||
className={cn(
|
||||
'flex items-center gap-2 px-3 py-1.5 hover:bg-accent/40 transition-colors',
|
||||
check.url && 'cursor-pointer'
|
||||
)}
|
||||
onClick={() => {
|
||||
if (check.url) {
|
||||
window.api.shell.openUrl(check.url)
|
||||
}
|
||||
}}
|
||||
>
|
||||
<Icon
|
||||
className={cn(
|
||||
'size-3.5 shrink-0',
|
||||
color,
|
||||
conclusion === 'pending' && 'animate-spin'
|
||||
)}
|
||||
/>
|
||||
<span className="flex-1 truncate text-[12px] text-foreground">{check.name}</span>
|
||||
{check.url && <ExternalLink className="size-3 text-muted-foreground/40 shrink-0" />}
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
<ChecksList checks={checks} checksLoading={checksLoading} />
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -4,9 +4,12 @@ import {
|
||||
LoaderCircle,
|
||||
CircleDashed,
|
||||
CircleMinus,
|
||||
GitPullRequest
|
||||
GitPullRequest,
|
||||
AlertTriangle
|
||||
} from 'lucide-react'
|
||||
import type { PRInfo } from '../../../../shared/types'
|
||||
import { ExternalLink } from 'lucide-react'
|
||||
import { cn } from '@/lib/utils'
|
||||
import type { PRInfo, PRMergeableState, PRCheckDetail } from '../../../../shared/types'
|
||||
|
||||
export const PullRequestIcon = GitPullRequest
|
||||
|
||||
@@ -30,6 +33,133 @@ export const CHECK_COLOR: Record<string, string> = {
|
||||
timed_out: 'text-rose-500'
|
||||
}
|
||||
|
||||
/** Shown when GitHub reports the PR branch has merge conflicts. */
|
||||
export function MergeConflictWarning({
|
||||
mergeable
|
||||
}: {
|
||||
mergeable: PRMergeableState
|
||||
}): React.JSX.Element | null {
|
||||
if (mergeable !== 'CONFLICTING') {
|
||||
return null
|
||||
}
|
||||
return (
|
||||
<div className="flex items-start gap-2 rounded-md border border-amber-500/30 bg-amber-500/10 px-3 py-2">
|
||||
<AlertTriangle className="mt-0.5 size-3.5 shrink-0 text-amber-500" />
|
||||
<div>
|
||||
<div className="text-[11px] font-medium text-foreground">
|
||||
This branch has conflicts that must be resolved before merging
|
||||
</div>
|
||||
<div className="mt-0.5 text-[10px] text-muted-foreground">
|
||||
Resolve conflicts on GitHub or locally, then push
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
const CHECK_SORT_ORDER: Record<string, number> = {
|
||||
failure: 0,
|
||||
timed_out: 0,
|
||||
cancelled: 1,
|
||||
pending: 2,
|
||||
neutral: 3,
|
||||
skipped: 4,
|
||||
success: 5
|
||||
}
|
||||
|
||||
/** Renders the checks summary bar + scrollable check list. */
|
||||
export function ChecksList({
|
||||
checks,
|
||||
checksLoading
|
||||
}: {
|
||||
checks: PRCheckDetail[]
|
||||
checksLoading: boolean
|
||||
}): React.JSX.Element {
|
||||
const sorted = [...checks].sort(
|
||||
(a, b) =>
|
||||
(CHECK_SORT_ORDER[a.conclusion ?? 'pending'] ?? 3) -
|
||||
(CHECK_SORT_ORDER[b.conclusion ?? 'pending'] ?? 3)
|
||||
)
|
||||
const passingCount = checks.filter((c) => c.conclusion === 'success').length
|
||||
const failingCount = checks.filter(
|
||||
(c) => c.conclusion === 'failure' || c.conclusion === 'timed_out'
|
||||
).length
|
||||
const pendingCount = checks.filter(
|
||||
(c) => c.conclusion === 'pending' || c.conclusion === null
|
||||
).length
|
||||
|
||||
return (
|
||||
<>
|
||||
{/* Checks Summary */}
|
||||
{checks.length > 0 && (
|
||||
<div className="flex items-center gap-3 px-3 py-2 border-b border-border text-[10px] text-muted-foreground">
|
||||
{passingCount > 0 && (
|
||||
<span className="flex items-center gap-1">
|
||||
<CircleCheck className="size-3 text-emerald-500" />
|
||||
{passingCount} passing
|
||||
</span>
|
||||
)}
|
||||
{failingCount > 0 && (
|
||||
<span className="flex items-center gap-1">
|
||||
<CircleX className="size-3 text-rose-500" />
|
||||
{failingCount} failing
|
||||
</span>
|
||||
)}
|
||||
{pendingCount > 0 && (
|
||||
<span className="flex items-center gap-1">
|
||||
<LoaderCircle className="size-3 text-amber-500" />
|
||||
{pendingCount} pending
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Checks List */}
|
||||
{checksLoading && checks.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-8">
|
||||
<LoaderCircle className="size-5 animate-spin text-muted-foreground" />
|
||||
</div>
|
||||
) : checks.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-8 text-[11px] text-muted-foreground">
|
||||
No checks configured
|
||||
</div>
|
||||
) : (
|
||||
<div className="py-1">
|
||||
{sorted.map((check) => {
|
||||
const conclusion = check.conclusion ?? 'pending'
|
||||
const Icon = CHECK_ICON[conclusion] ?? CircleDashed
|
||||
const color = CHECK_COLOR[conclusion] ?? 'text-muted-foreground'
|
||||
return (
|
||||
<div
|
||||
key={check.name}
|
||||
className={cn(
|
||||
'flex items-center gap-2 px-3 py-1.5 hover:bg-accent/40 transition-colors',
|
||||
check.url && 'cursor-pointer'
|
||||
)}
|
||||
onClick={() => {
|
||||
if (check.url) {
|
||||
window.api.shell.openUrl(check.url)
|
||||
}
|
||||
}}
|
||||
>
|
||||
<Icon
|
||||
className={cn(
|
||||
'size-3.5 shrink-0',
|
||||
color,
|
||||
conclusion === 'pending' && 'animate-spin'
|
||||
)}
|
||||
/>
|
||||
<span className="flex-1 truncate text-[12px] text-foreground">{check.name}</span>
|
||||
{check.url && <ExternalLink className="size-3 text-muted-foreground/40 shrink-0" />}
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
export function prStateColor(state: PRInfo['state']): string {
|
||||
switch (state) {
|
||||
case 'merged':
|
||||
|
||||
@@ -36,6 +36,7 @@ function makePR(overrides: Partial<PRInfo> = {}): PRInfo {
|
||||
url: 'https://example.com/pr/12',
|
||||
checksStatus: 'pending',
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
mergeable: 'UNKNOWN',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
@@ -93,6 +93,8 @@ export type PRState = 'open' | 'closed' | 'merged' | 'draft'
|
||||
export type IssueState = 'open' | 'closed'
|
||||
export type CheckStatus = 'pending' | 'success' | 'failure' | 'neutral'
|
||||
|
||||
export type PRMergeableState = 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN'
|
||||
|
||||
export type PRInfo = {
|
||||
number: number
|
||||
title: string
|
||||
@@ -100,6 +102,7 @@ export type PRInfo = {
|
||||
url: string
|
||||
checksStatus: CheckStatus
|
||||
updatedAt: string
|
||||
mergeable: PRMergeableState
|
||||
}
|
||||
|
||||
export type PRCheckDetail = {
|
||||
|
||||
Reference in New Issue
Block a user