diff --git a/apps/web/components/github/pr-detail-panel.tsx b/apps/web/components/github/pr-detail-panel.tsx index a19b982fb9..83fd610876 100644 --- a/apps/web/components/github/pr-detail-panel.tsx +++ b/apps/web/components/github/pr-detail-panel.tsx @@ -6,7 +6,6 @@ import { IconRefresh, IconPlus, IconMinus, - IconAlertTriangle, IconGitMerge, IconCheck, IconLoader2, @@ -21,7 +20,7 @@ import { useActiveTaskPR, useTaskPR } from "@/hooks/domains/github/use-task-pr"; import { prPanelLabel } from "@/components/github/pr-utils"; import { usePRFeedback } from "@/hooks/domains/github/use-pr-feedback"; import { useGitHubStatus } from "@/hooks/domains/github/use-github-status"; -import { useCommentsStore } from "@/lib/state/slices/comments"; +import { useCommentsStore, isPRFeedbackComment } from "@/lib/state/slices/comments"; import type { PRFeedbackComment } from "@/lib/state/slices/comments"; import { useToast } from "@/components/toast-provider"; import { submitPRReview } from "@/lib/api/domains/github-api"; @@ -34,6 +33,7 @@ import { PRMarkdownBody, } from "./pr-shared"; import { PRMergeButton } from "./pr-merge-button"; +import { PRMergeabilityNotice, buildConflictResolutionMessage } from "./pr-mergeability-notice"; import { ReviewStateBadge } from "./pr-reviews-section"; import { ChecksSection } from "./pr-checks-section"; import { ReviewsSection } from "./pr-reviews-section"; @@ -111,6 +111,65 @@ function useAddPRFeedbackAsContext(sessionId: string, prNumber: number) { return { addAsContext }; } +// Sync live feedback data back to the store so topbar/other consumers stay up to date. +// Use primitive deps to avoid re-render loops from object reference changes. +// Guard: never regress the store to a less-terminal state (e.g. merged → open) +// because the feedback fetch may return stale data from before a backend poll update. +function useSyncLivePRState(taskPR: TaskPR, feedback: PRFeedback | null) { + const setTaskPR = useAppStore((s) => s.setTaskPR); + const prState = taskPR.state; + const prMergedAt = taskPR.merged_at ?? null; + const prClosedAt = taskPR.closed_at ?? null; + const prAdditions = taskPR.additions; + const prDeletions = taskPR.deletions; + const prMergeableState = taskPR.mergeable_state; + const prTaskId = taskPR.task_id; + useEffect(() => { + if (!feedback) return; + const livePR = feedback.pr; + // State priority: merged > closed > open. Never regress to a less-terminal state. + const stateRank = (s: string) => { + if (s === "merged") return 2; + if (s === "closed") return 1; + return 0; + }; + const effectiveState = stateRank(livePR.state) >= stateRank(prState) ? livePR.state : prState; + const effectiveMergedAt = effectiveState === prState ? prMergedAt : (livePR.merged_at ?? null); + const effectiveClosedAt = effectiveState === prState ? prClosedAt : (livePR.closed_at ?? null); + // Live mergeable_state is authoritative when present; otherwise keep the stored value. + const effectiveMergeableState = livePR.mergeable_state ?? prMergeableState; + if ( + effectiveState !== prState || + effectiveMergedAt !== prMergedAt || + effectiveClosedAt !== prClosedAt || + livePR.additions !== prAdditions || + livePR.deletions !== prDeletions || + effectiveMergeableState !== prMergeableState + ) { + setTaskPR(prTaskId, { + ...taskPR, + state: effectiveState as TaskPR["state"], + additions: livePR.additions, + deletions: livePR.deletions, + merged_at: effectiveMergedAt, + closed_at: effectiveClosedAt, + mergeable_state: effectiveMergeableState, + }); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + feedback, + prState, + prMergedAt, + prClosedAt, + prAdditions, + prDeletions, + prMergeableState, + prTaskId, + setTaskPR, + ]); +} + type PRPanelMetrics = { reviewCount: number; pendingReviewCount: number; @@ -251,51 +310,38 @@ function ApproveButton({ export function PRDetailContent({ taskPR, sessionId }: { taskPR: TaskPR; sessionId: string }) { const { feedback, loading, refresh } = usePRFeedback(taskPR.owner, taskPR.repo, taskPR.pr_number); const { addAsContext } = useAddPRFeedbackAsContext(sessionId, taskPR.pr_number); - const setTaskPR = useAppStore((s) => s.setTaskPR); - // Sync live feedback data back to the store so topbar/other consumers stay up to date. - // Use primitive deps to avoid re-render loops from object reference changes. - // Guard: never regress the store to a less-terminal state (e.g. merged → open) - // because the feedback fetch may return stale data from before a backend poll update. - const prState = taskPR.state; - const prMergedAt = taskPR.merged_at ?? null; - const prClosedAt = taskPR.closed_at ?? null; - const prAdditions = taskPR.additions; - const prDeletions = taskPR.deletions; - const prTaskId = taskPR.task_id; - useEffect(() => { - if (!feedback) return; - const livePR = feedback.pr; - // State priority: merged > closed > open. Never regress to a less-terminal state. - const stateRank = (s: string) => { - if (s === "merged") return 2; - if (s === "closed") return 1; - return 0; - }; - const effectiveState = stateRank(livePR.state) >= stateRank(prState) ? livePR.state : prState; - const effectiveMergedAt = effectiveState === prState ? prMergedAt : (livePR.merged_at ?? null); - const effectiveClosedAt = effectiveState === prState ? prClosedAt : (livePR.closed_at ?? null); - if ( - effectiveState !== prState || - effectiveMergedAt !== prMergedAt || - effectiveClosedAt !== prClosedAt || - livePR.additions !== prAdditions || - livePR.deletions !== prDeletions - ) { - setTaskPR(prTaskId, { - ...taskPR, - state: effectiveState as TaskPR["state"], - additions: livePR.additions, - deletions: livePR.deletions, - merged_at: effectiveMergedAt, - closed_at: effectiveClosedAt, - }); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [feedback, prState, prMergedAt, prClosedAt, prAdditions, prDeletions, prTaskId, setTaskPR]); + useSyncLivePRState(taskPR, feedback); const metrics = derivePanelMetrics(taskPR, feedback); + // True once a conflict prompt for this PR is already queued — avoids piling + // up identical instructions if the user clicks "Resolve conflicts" again. + const conflictQueued = useCommentsStore((s) => + s.pendingForChat.some((id) => { + const c = s.byId[id]; + return ( + !!c && + isPRFeedbackComment(c) && + c.feedbackType === "conflict" && + c.sessionId === sessionId && + c.prNumber === taskPR.pr_number + ); + }), + ); + + const onResolveConflicts = useCallback(() => { + if (conflictQueued) return; + addAsContext( + "conflict", + buildConflictResolutionMessage({ + prNumber: taskPR.pr_number, + headBranch: taskPR.head_branch, + baseBranch: taskPR.base_branch, + }), + ); + }, [addAsContext, conflictQueued, taskPR.pr_number, taskPR.head_branch, taskPR.base_branch]); + return (
@@ -467,17 +515,23 @@ function PRHeader({ metrics, loading, onRefresh, + onResolveConflicts, + conflictQueued, }: { taskPR: TaskPR; feedback: PRFeedback | null; metrics: PRPanelMetrics; loading: boolean; onRefresh: () => void; + onResolveConflicts: () => void; + conflictQueued: boolean; }) { const liveState = feedback?.pr.state ?? taskPR.state; const isDraft = feedback?.pr.draft ?? false; const isMergeable = feedback?.pr.mergeable ?? true; - const showWarnings = !isDraft && !isMergeable && liveState === "open"; + // Prefer the live feedback state (refreshed by the panel's Refresh button); + // fall back to the polled store value before feedback loads. + const mergeableState = feedback?.pr.mergeable_state ?? taskPR.mergeable_state; return (
@@ -499,14 +553,15 @@ function PRHeader({ {taskPR.base_branch}
- {showWarnings && ( -
- - - Not mergeable - -
- )} +
diff --git a/apps/web/components/github/pr-mergeability-notice.test.ts b/apps/web/components/github/pr-mergeability-notice.test.ts new file mode 100644 index 0000000000..103705caeb --- /dev/null +++ b/apps/web/components/github/pr-mergeability-notice.test.ts @@ -0,0 +1,78 @@ +import { describe, it, expect } from "vitest"; +import { describeMergeability, buildConflictResolutionMessage } from "./pr-mergeability-notice"; +import type { MergeableState } from "@/lib/types/github"; + +function describe_({ + state, + mergeable = false, + isDraft = false, + prState = "open", +}: { + state: MergeableState | undefined; + mergeable?: boolean; + isDraft?: boolean; + prState?: string; +}) { + return describeMergeability({ state, mergeable, isDraft, prState }); +} + +describe("describeMergeability", () => { + it("shows the conflict banner for a dirty PR", () => { + expect(describe_({ state: "dirty" })).toEqual({ kind: "banner" }); + }); + + it("shows a 'Blocked' chip for a blocked PR (even when GitHub reports mergeable)", () => { + expect(describe_({ state: "blocked", mergeable: true })).toEqual({ + kind: "chip", + label: "Blocked", + }); + }); + + it("shows a 'Behind base' chip for a behind PR", () => { + expect(describe_({ state: "behind", mergeable: true })).toEqual({ + kind: "chip", + label: "Behind base", + }); + }); + + it("shows nothing for clean / unstable / has_hooks / draft", () => { + expect(describe_({ state: "clean", mergeable: true })).toEqual({ kind: "none" }); + expect(describe_({ state: "unstable", mergeable: true })).toEqual({ kind: "none" }); + expect(describe_({ state: "has_hooks", mergeable: true })).toEqual({ kind: "none" }); + // "draft" enum on a non-draft PR object must not fall through to "Not mergeable". + expect(describe_({ state: "draft", mergeable: false })).toEqual({ kind: "none" }); + }); + + it("falls back to generic text for unknown states that GitHub deems non-mergeable", () => { + expect(describe_({ state: "unknown", mergeable: false })).toEqual({ kind: "text" }); + expect(describe_({ state: "", mergeable: false })).toEqual({ kind: "text" }); + expect(describe_({ state: undefined, mergeable: false })).toEqual({ kind: "text" }); + }); + + it("shows nothing for unknown states GitHub still considers mergeable", () => { + expect(describe_({ state: "unknown", mergeable: true })).toEqual({ kind: "none" }); + }); + + it("suppresses every notice for draft PRs", () => { + expect(describe_({ state: "dirty", isDraft: true })).toEqual({ kind: "none" }); + }); + + it("suppresses every notice for non-open PRs", () => { + expect(describe_({ state: "dirty", prState: "merged" })).toEqual({ kind: "none" }); + expect(describe_({ state: "dirty", prState: "closed" })).toEqual({ kind: "none" }); + }); +}); + +describe("buildConflictResolutionMessage", () => { + it("names the PR and both branches", () => { + const msg = buildConflictResolutionMessage({ + prNumber: 1132, + headBranch: "feature/x", + baseBranch: "main", + }); + expect(msg).toContain("#1132"); + expect(msg).toContain("`feature/x`"); + expect(msg).toContain("`main`"); + expect(msg.toLowerCase()).toContain("conflict"); + }); +}); diff --git a/apps/web/components/github/pr-mergeability-notice.tsx b/apps/web/components/github/pr-mergeability-notice.tsx new file mode 100644 index 0000000000..f86e8c46e5 --- /dev/null +++ b/apps/web/components/github/pr-mergeability-notice.tsx @@ -0,0 +1,155 @@ +"use client"; + +import { IconAlertTriangle } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import type { MergeableState } from "@/lib/types/github"; + +// --- Pure descriptor (unit-tested) --- + +export type MergeabilityNotice = + | { kind: "none" } + | { kind: "banner" } // dirty / merge conflicts + | { kind: "chip"; label: string } // blocked / behind base + | { kind: "text" }; // generic non-mergeable fallback + +/** Maps a PR's merge state to how prominently the panel surfaces it. */ +export function describeMergeability({ + state, + mergeable, + isDraft, + prState, +}: { + state: MergeableState | undefined; + mergeable: boolean; + isDraft: boolean; + prState: string; +}): MergeabilityNotice { + if (prState !== "open" || isDraft) return { kind: "none" }; + switch (state) { + case "dirty": + return { kind: "banner" }; + case "blocked": + return { kind: "chip", label: "Blocked" }; + case "behind": + return { kind: "chip", label: "Behind base" }; + case "clean": + case "unstable": + case "has_hooks": + case "draft": + // "draft" is gated above via isDraft, but handle the enum value too. + return { kind: "none" }; + default: + // unknown / "" / future states: defer to the legacy boolean signal. + return mergeable === false ? { kind: "text" } : { kind: "none" }; + } +} + +/** Chat-context message sent to the agent when the user clicks "Resolve conflicts". */ +export function buildConflictResolutionMessage({ + prNumber, + headBranch, + baseBranch, +}: { + prNumber: number; + headBranch: string; + baseBranch: string; +}): string { + return ( + `PR #${prNumber} (\`${headBranch}\` → \`${baseBranch}\`) has merge conflicts and ` + + `can't be merged automatically. Please merge \`${baseBranch}\` into \`${headBranch}\` ` + + `(or rebase onto it) and resolve the conflicts.` + ); +} + +// --- Presentation --- + +function ConflictBanner({ + baseBranch, + onResolveConflicts, + resolveDisabled, +}: { + baseBranch: string; + onResolveConflicts?: () => void; + resolveDisabled?: boolean; +}) { + return ( +
+ +
+
+ Merge conflicts with {baseBranch} +
+

+ This branch can't be merged automatically. Resolve the conflicts before merging. +

+ {onResolveConflicts && ( + + )} +
+
+ ); +} + +function MergeabilityChip({ label }: { label: string }) { + return ( + + + {label} + + ); +} + +function NotMergeableText() { + return ( + + + Not mergeable + + ); +} + +export function PRMergeabilityNotice({ + state, + mergeable, + isDraft, + prState, + baseBranch, + onResolveConflicts, + resolveDisabled, +}: { + state: MergeableState | undefined; + mergeable: boolean; + isDraft: boolean; + prState: string; + baseBranch: string; + onResolveConflicts?: () => void; + resolveDisabled?: boolean; +}) { + const notice = describeMergeability({ state, mergeable, isDraft, prState }); + if (notice.kind === "none") return null; + if (notice.kind === "banner") + return ( + + ); + return ( +
+ {notice.kind === "chip" ? : } +
+ ); +} diff --git a/apps/web/lib/state/slices/comments/types.ts b/apps/web/lib/state/slices/comments/types.ts index eba9850a5e..fabf095e00 100644 --- a/apps/web/lib/state/slices/comments/types.ts +++ b/apps/web/lib/state/slices/comments/types.ts @@ -52,7 +52,7 @@ export type FileEditorComment = CommentBase & { export type PRFeedbackComment = CommentBase & { source: "pr-feedback"; prNumber: number; - feedbackType: "check" | "review" | "comment"; + feedbackType: "check" | "review" | "comment" | "conflict"; /** Pre-formatted markdown text for display and sending */ content: string; }; diff --git a/apps/web/lib/types/github.ts b/apps/web/lib/types/github.ts index 00fe8d6fe5..484bf59133 100644 --- a/apps/web/lib/types/github.ts +++ b/apps/web/lib/types/github.ts @@ -55,6 +55,9 @@ export type GitHubPR = { repo_name: string; draft: boolean; mergeable: boolean; + /** Rich merge state (clean | blocked | behind | dirty | ...). Optional because + * legacy payloads predate it; falls back to TaskPR.mergeable_state. */ + mergeable_state?: MergeableState; additions: number; deletions: number; requested_reviewers: RequestedReviewer[];