diff --git a/AGENTS.md b/AGENTS.md index e876ae834d..682a28706d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -179,7 +179,7 @@ Scoped exception (FN-5819): shared-branch-group members (`branchContext.assignme ### Lazy-Loaded Heavy Views -These 19 views are lazy-loaded via `React.lazy()` with ``. +These 20 views are lazy-loaded via `React.lazy()` with ``. Keep this AGENTS inventory in sync with App lazy imports and `packages/dashboard/app/__tests__/lazy-loaded-views-docs.test.ts`. - `AgentsView` @@ -197,6 +197,7 @@ Keep this AGENTS inventory in sync with App lazy imports and `packages/dashboard - `TodoView` - `GoalsView` - `StashRecoveryView` +- `PullRequestView` - `SetupWizardModal` - `PluginManager` - `PiExtensionsManager` diff --git a/packages/dashboard/app/App.tsx b/packages/dashboard/app/App.tsx index ec73184f45..74f1a9dc2f 100644 --- a/packages/dashboard/app/App.tsx +++ b/packages/dashboard/app/App.tsx @@ -118,6 +118,7 @@ const DevServerView = lazy(() => import("./components/DevServerView").then((m) = const _TodoView = lazy(() => import("./components/TodoView").then((m) => ({ default: m.TodoView }))); const GoalsView = lazy(() => import("./components/GoalsView").then((m) => ({ default: m.GoalsView }))); const StashRecoveryView = lazy(() => import("./components/StashRecoveryView").then((m) => ({ default: m.StashRecoveryView }))); +const PullRequestView = lazy(() => import("./components/PullRequestView").then((m) => ({ default: m.PullRequestView }))); // Warm lazy chunks during browser idle so first navigation to each view is // instant. Each chunk is ~10–80 kB; total prefetch finishes well under a @@ -147,6 +148,7 @@ function prefetchLazyViews() { void import("./components/TodoView"); void import("./components/GoalsView"); void import("./components/StashRecoveryView"); + void import("./components/PullRequestView"); }); } @@ -757,6 +759,11 @@ function AppInner() { const [missionResumeSessionId, setMissionResumeSessionId] = useState(undefined); const [missionTargetId, setMissionTargetId] = useState(undefined); const [goalAnchorId, setGoalAnchorId] = useState(undefined); + const [selectedPrId, setSelectedPrId] = useState(() => { + if (typeof window === "undefined") return undefined; + const v = new URL(window.location.href).searchParams.get("pr"); + return v ?? undefined; + }); const [milestoneSliceResumeSessionId, setMilestoneSliceResumeSessionId] = useState(undefined); useEffect(() => { @@ -764,6 +771,11 @@ function AppInner() { setGoalAnchorId(undefined); } }, [goalAnchorId, taskView]); + useEffect(() => { + if (taskView !== "pull-requests" && selectedPrId !== undefined) { + setSelectedPrId(undefined); + } + }, [selectedPrId, taskView]); const [quickChatOpen, setQuickChatOpen] = useState(false); const [authTokenRecoveryOpen, setAuthTokenRecoveryOpen] = useState(false); const [dashboardHealth, setDashboardHealth] = useState(null); @@ -1563,6 +1575,16 @@ function AppInner() { ); } + if (taskView === "pull-requests") { + return ( + + + + + + ); + } + if (taskView === "insights") { if (!settingsLoaded || !insightsEnabled) { return null; diff --git a/packages/dashboard/app/__tests__/lazy-loaded-views-docs.test.ts b/packages/dashboard/app/__tests__/lazy-loaded-views-docs.test.ts index a55c0b0bd0..9699c5d481 100644 --- a/packages/dashboard/app/__tests__/lazy-loaded-views-docs.test.ts +++ b/packages/dashboard/app/__tests__/lazy-loaded-views-docs.test.ts @@ -18,6 +18,7 @@ const EXPECTED_DOCUMENTED_VIEWS = new Set([ "TodoView", "GoalsView", "StashRecoveryView", + "PullRequestView", "SetupWizardModal", "PluginManager", "PiExtensionsManager", @@ -40,6 +41,7 @@ const EXPECTED_APP_LEVEL_VIEWS = new Set([ "TodoView", "GoalsView", "StashRecoveryView", + "PullRequestView", ]); function extractLazyLoadedSection(agentsDoc: string): string { @@ -81,11 +83,11 @@ describe("AGENTS lazy-loaded views inventory", () => { const section = extractLazyLoadedSection(agentsDoc); const countMatch = section.match(/These\s+(\d+)\s+views\s+are lazy-loaded/); expect(countMatch).toBeTruthy(); - expect(Number(countMatch?.[1])).toBe(19); + expect(Number(countMatch?.[1])).toBe(20); const documentedViews = extractBacktickedNamesFromBullets(section); expect(new Set(documentedViews)).toEqual(EXPECTED_DOCUMENTED_VIEWS); - expect(documentedViews).toHaveLength(19); + expect(documentedViews).toHaveLength(20); expect(section).toContain("`ResearchView`"); expect(section).toContain("`TodoView`"); diff --git a/packages/dashboard/app/__tests__/pull-request-view.test.tsx b/packages/dashboard/app/__tests__/pull-request-view.test.tsx new file mode 100644 index 0000000000..67b1d695de --- /dev/null +++ b/packages/dashboard/app/__tests__/pull-request-view.test.tsx @@ -0,0 +1,160 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { PullRequestView, type PrDetail } from "../components/PullRequestView"; + +// Icons → simple stubs so assertions key on text/testids, not SVG internals. +vi.mock("lucide-react", () => { + const Stub = () => ; + return new Proxy({}, { get: () => Stub }); +}); + +function makeSummary(over: Partial = {}): PrDetail["summary"] { + return { + mergeable: "clean", + reviewDecision: "APPROVED", + checksRollup: "success", + conflicting: false, + autoMerge: false, + autoMergeReason: "Ready to merge", + autoMergeReady: true, + actionable: true, + active: true, + pendingThreads: 0, + disagreedThreads: 0, + ...over, + }; +} + +function makeDetail(over: Partial = {}): PrDetail { + return { + id: "PR-1", + sourceType: "task", + sourceId: "FN-1", + repo: "owner/repo", + headBranch: "feature/x", + state: "open", + prNumber: 42, + prUrl: "https://example/pr/42", + mergeable: "clean", + checksRollup: "success", + reviewDecision: "APPROVED", + autoMerge: false, + unverified: false, + responseRounds: 0, + threads: [], + summary: makeSummary(over.summary), + ...over, + }; +} + +describe("PullRequestView per-node-state rendering", () => { + it("creating → 'Creating PR…' placeholder", () => { + render(); + expect(screen.getByTestId("pr-view").dataset.state).toBe("creating"); + expect(screen.getByTestId("pr-creating").textContent).toContain("Creating PR"); + }); + + it("failed → failure reason + Retry PR creation action", async () => { + const onAction = vi.fn(async () => makeDetail({ state: "creating" })); + render( + , + ); + expect(screen.getByTestId("pr-failed").textContent).toContain("gh auth missing"); + fireEvent.click(screen.getByTestId("pr-retry-create")); + await waitFor(() => expect(onAction).toHaveBeenCalledWith("retry-create", "PR-1", undefined)); + }); + + it("unverified → 'Verifying with GitHub…', checks/threads hidden, merge disabled", () => { + render( + , + ); + expect(screen.getByTestId("pr-unverified").textContent).toContain("Verifying with GitHub"); + expect(screen.queryByTestId("pr-threads")).toBeNull(); + expect(screen.queryByTestId("pr-summary")).toBeNull(); + expect((screen.getByTestId("pr-merge") as HTMLButtonElement).disabled).toBe(true); + }); + + it("responding → banner with N pending threads, respond/retry disabled, per-thread pending markers", () => { + render( + , + ); + expect(screen.getByTestId("pr-responding").textContent).toContain("3 threads pending"); + expect((screen.getByTestId("pr-retry") as HTMLButtonElement).disabled).toBe(true); + expect(screen.getByTestId("pr-thread-pending")).toBeTruthy(); + }); + + it("open/await-review → action bar (Approve/Retry/Merge/Close) + auto-merge gate reason", () => { + render(); + expect(screen.getByTestId("pr-action-bar")).toBeTruthy(); + expect(screen.getByTestId("pr-approve")).toBeTruthy(); + expect(screen.getByTestId("pr-retry")).toBeTruthy(); + expect(screen.getByTestId("pr-merge")).toBeTruthy(); + expect(screen.getByTestId("pr-close")).toBeTruthy(); + expect(screen.getByTestId("pr-automerge-gate").textContent).toBe("Waiting for approval"); + }); + + it("conflict → Merge disabled + 'Resolve conflicts on GitHub' link", () => { + render( + , + ); + expect((screen.getByTestId("pr-merge") as HTMLButtonElement).disabled).toBe(true); + const link = screen.getByTestId("pr-conflict-link") as HTMLAnchorElement; + expect(link.textContent).toContain("Resolve conflicts on GitHub"); + expect(link.href).toContain("/pr/42"); + }); + + it("agent disagreements are visually distinguished from human-awaiting threads", () => { + render( + , + ); + const disagreed = screen.getByTestId("pr-thread-disagreed"); + expect(disagreed.dataset.agentDisagreement).toBe("true"); + expect(disagreed.className).toContain("pr-thread--agent-disagreement"); + const pending = screen.getByTestId("pr-thread-pending"); + expect(pending.dataset.agentDisagreement).toBe("false"); + }); + + it("merge uses a single confirm step, then fires the merge action", async () => { + const onAction = vi.fn(async () => makeDetail()); + render(); + fireEvent.click(screen.getByTestId("pr-merge")); + // Single-confirm: a confirm control appears (no heavy modal). + const confirm = await screen.findByTestId("pr-merge-confirm"); + fireEvent.click(confirm); + await waitFor(() => expect(onAction).toHaveBeenCalledWith("merge", "PR-1", undefined)); + }); + + it("auto-merge toggle dispatches the automerge action with enabled flag", async () => { + const onAction = vi.fn(async () => makeDetail({ autoMerge: true })); + render(); + fireEvent.click(screen.getByTestId("pr-automerge").querySelector("input")!); + await waitFor(() => expect(onAction).toHaveBeenCalledWith("automerge", "PR-1", { enabled: true })); + }); +}); diff --git a/packages/dashboard/app/components/PullRequestView.css b/packages/dashboard/app/components/PullRequestView.css new file mode 100644 index 0000000000..c22635b2de --- /dev/null +++ b/packages/dashboard/app/components/PullRequestView.css @@ -0,0 +1,254 @@ +.pr-view { + display: flex; + flex-direction: column; + gap: var(--space-md); + height: 100%; + min-height: 0; + overflow-y: auto; + -webkit-overflow-scrolling: touch; + padding: var(--space-lg); + color: var(--text); +} + +.pr-view--loading, +.pr-view--error { + display: flex; + align-items: center; + gap: var(--space-xs); + color: var(--text-muted); +} + +.pr-view--error { + color: var(--danger, #e5534b); +} + +/* identity header */ +.pr-identity { + display: flex; + align-items: center; + gap: var(--space-sm); + flex-wrap: wrap; + padding-bottom: var(--space-sm); + border-bottom: 1px solid var(--border); +} + +.pr-identity-repo { + font-weight: 600; +} + +.pr-identity-number { + color: var(--accent, var(--text)); + text-decoration: none; + display: inline-flex; + align-items: center; + gap: 2px; +} + +.pr-identity-branch { + color: var(--text-muted); + font-family: var(--font-mono, monospace); + font-size: 0.85em; +} + +.pr-identity-state { + margin-left: auto; + text-transform: uppercase; + font-size: 0.7em; + letter-spacing: 0.04em; + padding: 2px 6px; + border-radius: 4px; + background: var(--surface-2, rgba(127, 127, 127, 0.15)); + color: var(--text-muted); +} + +.pr-identity-state--failed { + background: rgba(229, 83, 75, 0.18); + color: var(--danger, #e5534b); +} + +.pr-identity-state--merged { + background: rgba(130, 80, 223, 0.18); +} + +/* placeholders / banners / notices */ +.pr-placeholder, +.pr-notice, +.pr-banner, +.pr-error-reason { + display: flex; + align-items: center; + gap: var(--space-xs); + padding: var(--space-sm) var(--space-md); + border-radius: 6px; + background: var(--surface-2, rgba(127, 127, 127, 0.1)); +} + +.pr-banner--responding { + background: rgba(54, 130, 220, 0.14); +} + +.pr-notice--unverified { + background: rgba(220, 170, 54, 0.14); +} + +.pr-error-reason { + background: rgba(229, 83, 75, 0.14); + color: var(--danger, #e5534b); +} + +/* action bar */ +.pr-action-bar { + display: flex; + align-items: center; + gap: var(--space-sm); + flex-wrap: wrap; +} + +.pr-action { + display: inline-flex; + align-items: center; + gap: 4px; + padding: 4px 10px; + border: 1px solid var(--border); + border-radius: 6px; + background: var(--surface, transparent); + color: var(--text); + cursor: pointer; + font-size: 0.85em; +} + +.pr-action:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +.pr-action--merge, +.pr-action--merge-confirm { + border-color: var(--accent, var(--border)); +} + +.pr-action--merge-confirm { + background: var(--accent, #2f81f7); + color: #fff; +} + +.pr-action--close { + border-color: rgba(229, 83, 75, 0.4); +} + +.pr-automerge-toggle { + display: inline-flex; + align-items: center; + gap: 6px; + margin-left: auto; + font-size: 0.82em; + color: var(--text-muted); +} + +.pr-automerge-gate { + padding: 1px 6px; + border-radius: 4px; + background: var(--surface-2, rgba(127, 127, 127, 0.15)); +} + +.pr-conflict-link { + display: inline-flex; + align-items: center; + gap: 4px; + color: var(--danger, #e5534b); + text-decoration: none; + font-size: 0.85em; +} + +/* merge-readiness summary */ +.pr-summary { + display: flex; + gap: var(--space-md); + flex-wrap: wrap; + padding: var(--space-sm) 0; + border-top: 1px solid var(--border); + border-bottom: 1px solid var(--border); +} + +.pr-summary-item { + display: inline-flex; + align-items: center; + gap: 4px; + font-size: 0.85em; + color: var(--text-muted); +} + +.pr-icon-success { + color: var(--success, #3fb950); +} + +.pr-icon-failure { + color: var(--danger, #e5534b); +} + +.pr-icon-pending { + color: var(--warning, #d29922); +} + +/* threads */ +.pr-threads { + display: flex; + flex-direction: column; + gap: var(--space-sm); +} + +.pr-threads-empty { + color: var(--text-muted); + font-size: 0.85em; +} + +.pr-thread { + border: 1px solid var(--border); + border-radius: 6px; + padding: var(--space-sm); +} + +.pr-thread--agent-disagreement { + border-left: 3px solid var(--warning, #d29922); + background: rgba(210, 153, 34, 0.08); +} + +.pr-thread--pending { + border-left: 3px solid var(--text-muted); +} + +.pr-thread-head { + display: flex; + align-items: center; + gap: var(--space-sm); + font-size: 0.82em; +} + +.pr-thread-pending, +.pr-thread-disagreed, +.pr-thread-fixed { + display: inline-flex; + align-items: center; + gap: 4px; +} + +.pr-thread-id { + color: var(--text-muted); + font-family: var(--font-mono, monospace); + font-size: 0.85em; + margin-left: auto; +} + +.pr-thread-reply { + margin-top: 6px; + margin-left: var(--space-md); + padding: var(--space-xs) var(--space-sm); + border-left: 2px solid var(--border); + color: var(--text-muted); + font-size: 0.82em; +} + +.pr-inline-error { + color: var(--danger, #e5534b); + font-size: 0.85em; +} diff --git a/packages/dashboard/app/components/PullRequestView.tsx b/packages/dashboard/app/components/PullRequestView.tsx new file mode 100644 index 0000000000..95b09eddd2 --- /dev/null +++ b/packages/dashboard/app/components/PullRequestView.tsx @@ -0,0 +1,409 @@ +import { useCallback, useEffect, useState } from "react"; +import { + GitPullRequest, + GitMerge, + CheckCircle, + XCircle, + Clock, + AlertTriangle, + ExternalLink, + RotateCcw, + ThumbsUp, + MessageSquare, +} from "lucide-react"; +import { api } from "../api"; +import "./PullRequestView.css"; + +// Mirrors the route's serialized entity (register-pull-requests-routes.ts). +export type PrThread = { + prEntityId: string; + threadId: string; + headOid: string; + outcome: "fixed" | "disagreed" | "pending"; + fixCommitSha?: string; + updatedAt: number; +}; + +export type PrSummary = { + mergeable: string; + reviewDecision: string | null; + checksRollup: string; + conflicting: boolean; + autoMerge: boolean; + autoMergeReason: string; + autoMergeReady: boolean; + actionable: boolean; + active: boolean; + pendingThreads: number; + disagreedThreads: number; +}; + +export type PrDetail = { + id: string; + sourceType: "task" | "branch-group"; + sourceId: string; + repo: string; + headBranch: string; + baseBranch?: string; + state: "creating" | "open" | "responding" | "merged" | "closed" | "failed"; + prNumber?: number; + prUrl?: string; + mergeable?: string; + checksRollup?: string; + reviewDecision?: string | null; + autoMerge: boolean; + unverified: boolean; + failureReason?: string; + responseRounds: number; + threads: PrThread[]; + summary: PrSummary; +}; + +type ActionKind = "approve" | "merge" | "retry" | "close" | "automerge" | "retry-create"; + +export interface PullRequestViewProps { + /** When provided, render this detail directly (tests / parent-supplied data). */ + detail?: PrDetail | null; + /** Entity id to self-fetch when `detail` is not provided. */ + pullRequestId?: string; + projectId?: string; + /** Override the action dispatcher (tests). Defaults to the POST routes. */ + onAction?: (kind: ActionKind, id: string, body?: Record) => Promise; + /** Override the fetcher (tests). */ + loadPullRequest?: (id: string) => Promise; +} + +function defaultLoad(projectId?: string) { + return async (id: string): Promise => { + const q = projectId ? `?projectId=${encodeURIComponent(projectId)}` : ""; + const res = await api<{ pullRequest: PrDetail }>(`/pull-requests/${id}${q}`); + return res.pullRequest; + }; +} + +function defaultAction(projectId?: string) { + return async (kind: ActionKind, id: string, body?: Record): Promise => { + const path = kind === "automerge" ? "automerge" : kind; + const res = await api<{ pullRequest: PrDetail }>(`/pull-requests/${id}/${path}`, { + method: "POST", + body: JSON.stringify({ ...(body ?? {}), ...(projectId ? { projectId } : {}) }), + headers: { "content-type": "application/json" }, + }); + return res.pullRequest; + }; +} + +function ChecksIcon({ rollup }: { rollup: string }) { + if (rollup === "success") return ; + if (rollup === "failure") return ; + if (rollup === "pending") return ; + return —; +} + +export function PullRequestView(props: PullRequestViewProps) { + const { detail: detailProp, pullRequestId, projectId, onAction, loadPullRequest } = props; + const [detail, setDetail] = useState(detailProp ?? null); + const [error, setError] = useState(null); + const [busy, setBusy] = useState(null); + const [confirmingMerge, setConfirmingMerge] = useState(false); + + const load = loadPullRequest ?? defaultLoad(projectId); + const dispatch = onAction ?? defaultAction(projectId); + + const refresh = useCallback(async () => { + if (detailProp) { + setDetail(detailProp); + return; + } + if (!pullRequestId) return; + try { + setError(null); + setDetail(await load(pullRequestId)); + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to load PR"); + } + }, [detailProp, pullRequestId, load]); + + useEffect(() => { + void refresh(); + }, [refresh]); + + // Live updates: re-poll on the store-event / SSE channel the rest of the app + // uses. We listen for the lightweight "store-changed" window event the SSE + // bridge dispatches; each tick re-reads authoritative state from the route. + useEffect(() => { + if (detailProp || !pullRequestId) return; + const handler = () => void refresh(); + window.addEventListener("fusion:store-changed", handler); + return () => window.removeEventListener("fusion:store-changed", handler); + }, [detailProp, pullRequestId, refresh]); + + const runAction = useCallback( + async (kind: ActionKind, body?: Record) => { + if (!detail) return; + try { + setBusy(kind); + setError(null); + const fresh = await dispatch(kind, detail.id, body); + setDetail(fresh); + } catch (err) { + setError(err instanceof Error ? err.message : `Action ${kind} failed`); + } finally { + setBusy(null); + setConfirmingMerge(false); + } + }, + [detail, dispatch], + ); + + if (error && !detail) { + return ( +
+ {error} +
+ ); + } + if (!detail) { + return ( +
+ Loading PR… +
+ ); + } + + const { state, summary } = detail; + + // ── creating ─────────────────────────────────────────────────────────────── + if (state === "creating") { + return ( +
+ +
+ Creating PR… +
+
+ ); + } + + // ── failed ─────────────────────────────────────────────────────────────── + if (state === "failed") { + return ( +
+ +
+ + {detail.failureReason ?? "PR creation failed"} +
+
+ +
+ {error &&
{error}
} +
+ ); + } + + // ── unverified ───────────────────────────────────────────────────────────── + if (detail.unverified) { + return ( +
+ +
+ Verifying with GitHub… +
+
+ +
+ {/* checks/threads hidden while unverified */} +
+ ); + } + + const conflicting = summary.conflicting; + + return ( +
+ + + {/* responding banner */} + {state === "responding" && ( +
+ Response run in progress — {summary.pendingThreads} threads + pending +
+ )} + + {/* ── action bar ──────────────────────────────────────────────────── */} +
+ + + {!confirmingMerge ? ( + + ) : ( + + )} + + + +
+ + {/* conflict link */} + {conflicting && detail.prUrl && ( + + Resolve conflicts on GitHub + + )} + + {/* ── merge-readiness summary ─────────────────────────────────────── */} +
+ + Mergeable: {summary.mergeable} + + + Review: {summary.reviewDecision ?? "none"} + + + {summary.checksRollup} + +
+ + {/* ── threads (agent replies nested) ───────────────────────────────── */} +
+ {detail.threads.length === 0 ? ( +
No review threads.
+ ) : ( + detail.threads.map((thread) => ( +
+
+ {thread.outcome === "pending" && ( + + pending + + )} + {thread.outcome === "disagreed" && ( + + agent disagreed + + )} + {thread.outcome === "fixed" && ( + + fixed + + )} + {thread.threadId} +
+ {thread.fixCommitSha && ( +
+ Agent reply — fix {thread.fixCommitSha.slice(0, 8)} +
+ )} +
+ )) + )} +
+ + {error &&
{error}
} +
+ ); +} + +function PrIdentityHeader({ detail }: { detail: PrDetail }) { + return ( +
+ + {detail.repo} + {detail.prNumber != null ? ( + detail.prUrl ? ( + + #{detail.prNumber} + + ) : ( + #{detail.prNumber} + ) + ) : null} + {detail.headBranch} + {detail.state} +
+ ); +} diff --git a/packages/dashboard/app/components/TaskCard.css b/packages/dashboard/app/components/TaskCard.css index 7be8c7f0cc..96942ecb4b 100644 --- a/packages/dashboard/app/components/TaskCard.css +++ b/packages/dashboard/app/components/TaskCard.css @@ -187,6 +187,26 @@ text-transform: lowercase; } +/* Unified PR entity node-state badge (R12). Clickable, links to the PR view. */ +.card-pr-node-badge { + display: inline-flex; + align-items: center; + gap: 3px; + cursor: pointer; + background: color-mix(in srgb, var(--text-muted) 14%, transparent); + border-color: color-mix(in srgb, var(--text-muted) 30%, transparent); + color: var(--text-muted); + text-transform: none; + letter-spacing: 0; +} + +/* DISTINCT error badge for the failed node-state (never the open-PR badge). */ +.card-pr-node-badge--failed { + background: color-mix(in srgb, var(--color-danger, #e5534b) 18%, transparent); + border-color: color-mix(in srgb, var(--color-danger, #e5534b) 35%, transparent); + color: var(--color-danger, #e5534b); +} + .card-status-badge.stalled-review { background: color-mix(in srgb, var(--color-warning) 18%, transparent); color: var(--color-warning); diff --git a/packages/dashboard/app/components/TaskCard.tsx b/packages/dashboard/app/components/TaskCard.tsx index 56037e61ce..acdc741cb4 100644 --- a/packages/dashboard/app/components/TaskCard.tsx +++ b/packages/dashboard/app/components/TaskCard.tsx @@ -2,7 +2,7 @@ import "./TaskCard.css"; import { useTranslation } from "react-i18next"; import type { TFunction } from "i18next"; import { memo, useCallback, useState, useRef, useEffect, useMemo, type ReactElement } from "react"; -import { Link, Clock, Layers, Pencil, ChevronDown, Folder, Target, Bot, Trash2, RotateCw, Zap, GitBranch, GitPullRequest } from "lucide-react"; +import { Link, Clock, Layers, Pencil, ChevronDown, Folder, Target, Bot, Trash2, RotateCw, Zap, GitBranch, GitPullRequest, AlertTriangle } from "lucide-react"; import type { Task, TaskDetail, Column, ColumnId, PrInfo, IssueInfo, TaskPriority, GithubIssueAction } from "@fusion/core"; import { DEFAULT_TASK_PRIORITY, @@ -407,6 +407,12 @@ interface TaskCardProps { /** Card-placed custom field definitions for this task's workflow (U13/KTD-14). * Empty/undefined → no field badges render (card byte-identical to today). */ cardFieldDefs?: WorkflowFieldDefinition[]; + /** Unified PR entity node-state for this task's work, surfaced on the card (R12). + * When present, the card shows a node-state badge linking to the PR view. The + * `failed` state renders a DISTINCT error badge (not the open-PR badge). */ + prNode?: { id: string; state: "creating" | "open" | "responding" | "merged" | "closed" | "failed"; prNumber?: number }; + /** Called when the PR node badge is clicked — opens the dedicated PR view (R12). */ + onOpenPullRequest?: (prEntityId: string) => void; } function getTaskPrimaryPrInfo(task: Pick): PrInfo | undefined { @@ -540,6 +546,10 @@ function areTaskCardPropsEqual(previous: TaskCardProps, next: TaskCardProps): bo previous.taskStuckTimeoutMs === next.taskStuckTimeoutMs && previous.prAuthAvailable === next.prAuthAvailable && previous.autoMergeEnabled === next.autoMergeEnabled && + previous.onOpenPullRequest === next.onOpenPullRequest && + previous.prNode?.id === next.prNode?.id && + previous.prNode?.state === next.prNode?.state && + previous.prNode?.prNumber === next.prNode?.prNumber && previous.cardFieldDefs === next.cardFieldDefs && (previous.cardFieldDefs == null && next.cardFieldDefs == null ? true @@ -658,6 +668,8 @@ function TaskCardComponent({ prAuthAvailable, autoMergeEnabled = false, cardFieldDefs, + prNode, + onOpenPullRequest, }: TaskCardProps) { const { t } = useTranslation("app"); const columnLabel = useColumnLabel(); @@ -1875,6 +1887,41 @@ function TaskCardComponent({ ) : null} )} + {prNode && ( + prNode.state === "failed" ? ( + + ) : ( + + ) + )} {isAgentCreated && ( { + if (err instanceof ApiError) { + sendErrorResponse(res, err.statusCode, err.message, { details: err.details }); + return; + } + sendErrorResponse(res, 500, err instanceof Error ? err.message : "Internal server error"); + }); +} + +function buildEntity(overrides: Partial = {}): PrEntity { + return { + id: "PR-1", + sourceType: "task", + sourceId: "FN-1", + repo: "owner/repo", + headBranch: "feature/x", + state: "open", + prNumber: 42, + prUrl: "https://example/pr/42", + mergeable: "clean", + checksRollup: "success", + reviewDecision: "APPROVED", + autoMerge: false, + unverified: false, + responseRounds: 0, + createdAt: Date.now(), + updatedAt: Date.now(), + ...overrides, + }; +} + +function createStore(entity: PrEntity, threads: PrThreadState[] = []) { + let current = { ...entity }; + const store = { + getPrEntity: vi.fn((id: string) => (id === current.id ? current : null)), + listActivePrEntities: vi.fn(() => [current]), + listPrThreadStates: vi.fn(() => threads), + updatePrEntity: vi.fn((_id: string, patch: Partial) => { + current = { ...current, ...patch } as PrEntity; + return current; + }), + getTask: vi.fn(async (id: string) => ({ id, column: "in-review" } as Task)), + } as unknown as TaskStore; + return { store, getCurrent: () => current, setCurrent: (e: PrEntity) => { current = e; } }; +} + +function mount(store: TaskStore, opts?: Parameters[1]) { + const app = express(); + app.use(express.json()); + app.use("/api/pull-requests", createPullRequestsRouter(store, opts)); + attachErrorHandler(app); + return app; +} + +describe("pull request routes", () => { + let entity: PrEntity; + let threads: PrThreadState[]; + + beforeEach(() => { + entity = buildEntity(); + threads = [ + { prEntityId: "PR-1", threadId: "T1", headOid: "abc", outcome: "pending", updatedAt: Date.now() }, + { prEntityId: "PR-1", threadId: "T2", headOid: "abc", outcome: "disagreed", updatedAt: Date.now() }, + ]; + }); + + it("GET list returns entity with checks/threads/merge/conflict summary", async () => { + const { store } = createStore(entity, threads); + const app = mount(store); + const res = await REQUEST(app, "GET", "/api/pull-requests"); + expect(res.status).toBe(200); + expect(res.body.pullRequests).toHaveLength(1); + const pr = res.body.pullRequests[0]; + expect(pr.threads).toHaveLength(2); + expect(pr.summary.checksRollup).toBe("success"); + expect(pr.summary.mergeable).toBe("clean"); + expect(pr.summary.conflicting).toBe(false); + expect(pr.summary.pendingThreads).toBe(1); + expect(pr.summary.disagreedThreads).toBe(1); + }); + + it("GET list filters by repo and status", async () => { + const { store } = createStore(entity, threads); + const app = mount(store); + let res = await REQUEST(app, "GET", "/api/pull-requests?repo=other/repo"); + expect(res.body.pullRequests).toHaveLength(0); + res = await REQUEST(app, "GET", "/api/pull-requests?status=closed"); + expect(res.body.pullRequests).toHaveLength(0); + res = await REQUEST(app, "GET", "/api/pull-requests?status=open"); + expect(res.body.pullRequests).toHaveLength(1); + }); + + it("GET :id reports conflicting summary and gate reason", async () => { + const conflict = buildEntity({ mergeable: "conflicting", autoMerge: true }); + const { store } = createStore(conflict, threads); + const app = mount(store); + const res = await REQUEST(app, "GET", "/api/pull-requests/PR-1"); + expect(res.status).toBe(200); + expect(res.body.pullRequest.summary.conflicting).toBe(true); + expect(res.body.pullRequest.summary.autoMergeReason).toBe("Blocked: conflict"); + }); + + it("GET :id returns 404 for unknown PR", async () => { + const { store } = createStore(entity); + const app = mount(store); + const res = await REQUEST(app, "GET", "/api/pull-requests/PR-404"); + expect(res.status).toBe(404); + }); + + it("merge re-fetches authoritative state before acting (not a stale client copy)", async () => { + const { store } = createStore(entity, threads); + const mergePr = vi.fn(async () => ({ released: true })); + const app = mount(store, { mergePr }); + // Client sends a stale body claiming an old/wrong state — the route must ignore it. + const res = await REQUEST( + app, + "POST", + "/api/pull-requests/PR-1/merge", + JSON.stringify({ entity: { id: "PR-1", state: "creating", mergeable: "conflicting" } }), + { "content-type": "application/json" }, + ); + expect(res.status).toBe(200); + // getPrEntity is the authoritative re-read; it must have been consulted. + expect((store.getPrEntity as unknown as ReturnType)).toHaveBeenCalledWith("PR-1"); + // The capability received the AUTHORITATIVE entity (clean/open), not the stale client copy. + expect(mergePr).toHaveBeenCalledTimes(1); + const arg = mergePr.mock.calls[0][0] as { entity: PrEntity }; + expect(arg.entity.state).toBe("open"); + expect(arg.entity.mergeable).toBe("clean"); + }); + + it("merge is rejected (409) when the authoritative entity is conflicting", async () => { + const conflict = buildEntity({ mergeable: "conflicting" }); + const { store } = createStore(conflict, threads); + const mergePr = vi.fn(async () => ({ released: true })); + const app = mount(store, { mergePr }); + const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/merge", JSON.stringify({}), { + "content-type": "application/json", + }); + expect(res.status).toBe(409); + expect(mergePr).not.toHaveBeenCalled(); + }); + + it("approve/retry/close route to the injected engine capabilities", async () => { + const { store } = createStore(entity, threads); + const approvePr = vi.fn(async () => ({ released: true, action: "approve" })); + const retryPr = vi.fn(async () => ({ released: true, action: "retry" })); + const closePr = vi.fn(async () => ({ released: true, action: "close" })); + const app = mount(store, { approvePr, retryPr, closePr }); + + for (const [path, spy] of [["approve", approvePr], ["retry", retryPr], ["close", closePr]] as const) { + const res = await REQUEST(app, "POST", `/api/pull-requests/PR-1/${path}`, JSON.stringify({}), { + "content-type": "application/json", + }); + expect(res.status).toBe(200); + expect(spy).toHaveBeenCalledTimes(1); + expect(res.body.pullRequest.id).toBe("PR-1"); + } + }); + + it("retry-create only acts on failed entities and routes to retryCreate", async () => { + const failed = buildEntity({ state: "failed", failureReason: "auth" }); + const { store } = createStore(failed); + const retryCreate = vi.fn(async () => ({ released: true })); + const app = mount(store, { retryCreate }); + const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/retry-create", JSON.stringify({}), { + "content-type": "application/json", + }); + expect(res.status).toBe(200); + expect(retryCreate).toHaveBeenCalledTimes(1); + + // open entity → retry-create rejected (wrong state) + const { store: openStore } = createStore(buildEntity({ state: "open" })); + const retryCreate2 = vi.fn(); + const openApp = mount(openStore, { retryCreate: retryCreate2 as unknown as () => Promise> }); + const res2 = await REQUEST(openApp, "POST", "/api/pull-requests/PR-1/retry-create", JSON.stringify({}), { + "content-type": "application/json", + }); + expect(res2.status).toBe(409); + expect(retryCreate2).not.toHaveBeenCalled(); + }); + + it("action 400s when the capability is not wired", async () => { + const { store } = createStore(entity); + const app = mount(store, {}); // no approvePr + const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/approve", JSON.stringify({}), { + "content-type": "application/json", + }); + expect(res.status).toBe(400); + }); + + it("automerge toggle persists the flip and returns the gate reason", async () => { + const { store, getCurrent } = createStore(buildEntity({ autoMerge: false })); + const app = mount(store); + const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/automerge", JSON.stringify({ enabled: true }), { + "content-type": "application/json", + }); + expect(res.status).toBe(200); + expect(getCurrent().autoMerge).toBe(true); + expect(res.body.pullRequest.summary.autoMergeReason).toBe("Ready to merge"); + }); +}); + +describe("column move-backward guard (R16)", () => { + // COLUMNS order: triage(0) todo(1) in-progress(2) in-review(3) done(4). + it("blocks in-review (3) → in-progress (2) while an open PR exists, with guidance", () => { + expect( + isBackwardMoveBlockedByOpenPr({ + fromIndex: 3, + toIndex: 2, + activePrEntity: buildEntity({ state: "open" }), + }), + ).toBe(true); + expect(PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE).toBe( + "This task has an open PR. Merge or close the PR before moving it back.", + ); + }); + + it("allows the backward move once the PR is terminal (no active entity)", () => { + expect( + isBackwardMoveBlockedByOpenPr({ fromIndex: 3, toIndex: 2, activePrEntity: null }), + ).toBe(false); + // A terminal entity should also not block (defensive: store excludes these). + expect( + isBackwardMoveBlockedByOpenPr({ + fromIndex: 3, + toIndex: 2, + activePrEntity: buildEntity({ state: "closed" }), + }), + ).toBe(false); + }); + + it("never blocks a forward move even with an open PR", () => { + expect( + isBackwardMoveBlockedByOpenPr({ + fromIndex: 3, + toIndex: 4, + activePrEntity: buildEntity({ state: "open" }), + }), + ).toBe(false); + }); +}); diff --git a/packages/dashboard/src/routes/register-integrated-routers.ts b/packages/dashboard/src/routes/register-integrated-routers.ts index bd8282aa67..561e99f8d6 100644 --- a/packages/dashboard/src/routes/register-integrated-routers.ts +++ b/packages/dashboard/src/routes/register-integrated-routers.ts @@ -13,8 +13,10 @@ import { createDevServerRouter } from "../dev-server-routes.js"; import type { AiSessionStore } from "../ai-session-store.js"; import { createStashRecoveryRouter } from "./register-stash-recovery-routes.js"; import { createBranchGroupsRouter } from "./register-branch-groups-routes.js"; +import { createPullRequestsRouter } from "./register-pull-requests-routes.js"; import { GitHubClient, closeGroupPullRequest, reconcileGroupPullRequest } from "../github.js"; -import { reconcileBranchGroupPr } from "@fusion/engine"; +import { reconcileBranchGroupPr, releaseHeldTaskByEvent } from "@fusion/engine"; +import type { PrEntity } from "@fusion/core"; interface IntegratedRoutersOptions { router: Router; @@ -115,6 +117,29 @@ export function registerIntegratedRouters({ return store.getBranchGroup(group.id) ?? group; }, })); + + // Unified PR entity view + user-controlled actions (U7, R11/R12/R13). Each + // side-effecting action maps to a manual hold-release: the workflow's + // user-controlled release edges own the GitHub side effects, so the route just + // releases the entity's source task with an action-specific event tag. The + // route layer already re-reads authoritative entity state before invoking these + // callbacks (never a stale client copy). The engine primitive is imported + // statically (FN-3049 — no runtime `await import`). + const releaseForPr = async (entity: PrEntity, eventTag: string): Promise> => { + // task-sourced entities release the task directly; branch-group-sourced + // entities release the group's representative task (the sourceId is the task + // id the workflow placed on the await hold in both cases). + const result = await releaseHeldTaskByEvent(store, entity.sourceId, eventTag); + return { released: result.released, toColumn: result.toColumn, rejection: result.rejection }; + }; + + router.use("/pull-requests", createPullRequestsRouter(store, { + approvePr: ({ entity }) => releaseForPr(entity, "pr-approve"), + mergePr: ({ entity }) => releaseForPr(entity, "pr-merge"), + retryPr: ({ entity }) => releaseForPr(entity, "pr-retry"), + closePr: ({ entity }) => releaseForPr(entity, "pr-close"), + retryCreate: ({ entity }) => releaseForPr(entity, "pr-retry-create"), + })); } export function registerIntegratedDevServerRouter({ router, store }: DevServerRouterOptions): void { diff --git a/packages/dashboard/src/routes/register-pull-requests-routes.ts b/packages/dashboard/src/routes/register-pull-requests-routes.ts new file mode 100644 index 0000000000..8b8ecede1e --- /dev/null +++ b/packages/dashboard/src/routes/register-pull-requests-routes.ts @@ -0,0 +1,239 @@ +import { Router, type Request } from "express"; +import type { PrEntity, PrThreadState, TaskStore } from "@fusion/core"; +import { + isPrEntityActive, + isPrEntityActionable, + isPrEntityAutoMergeReady, +} from "@fusion/core"; +import { badRequest, notFound, ApiError } from "../api-error.js"; + +/** + * Injected engine capabilities for the user-controlled PR actions (U7, R13). + * + * Each action maps to a manual hold-release (the workflow's user-controlled + * release edges own the real GitHub side effects): approve/merge/retry/close all + * fire the same release authority the scheduler's hold-release sweep uses. The + * router never imports the engine directly (FN-3049) — capabilities arrive as + * option callbacks wired in register-integrated-routers.ts. When a capability is + * omitted the corresponding action 400s ("unavailable") rather than no-op'ing + * silently. + * + * All side-effecting callbacks receive the AUTHORITATIVE entity the route just + * re-read from the store — never a client-supplied copy. Acting on a stale + * client copy is the bug class documented in + * docs/solutions/logic-errors/queued-chat-message-flush-trusts-stale-isgenerating.md. + */ +export interface PullRequestsRouterOptions { + /** Release the PR's source task to advance toward merge (approve / force-merge). */ + approvePr?: (input: { entity: PrEntity; projectId?: string }) => Promise>; + /** Merge the PR via the workflow's merge release (force-merge). */ + mergePr?: (input: { entity: PrEntity; projectId?: string }) => Promise>; + /** Request another review-response round (rework release). */ + retryPr?: (input: { entity: PrEntity; projectId?: string }) => Promise>; + /** Close the PR terminally and reconcile the entity. */ + closePr?: (input: { entity: PrEntity; projectId?: string }) => Promise>; + /** Retry a failed PR creation (state === "failed", R4). */ + retryCreate?: (input: { entity: PrEntity; projectId?: string }) => Promise>; +} + +function parseProjectId(req: Request): string | undefined { + const value = req.query.projectId ?? req.body?.projectId; + return typeof value === "string" && value.trim() ? value.trim() : undefined; +} + +/** + * The live auto-merge gate reason shown next to the toggle (R11). Mirrors the + * engine's auto-merge-ready predicate ordering so the UI never disagrees with + * what the gate will actually do. + */ +export function autoMergeGateReason(entity: PrEntity): string { + if (!entity.autoMerge) return "Auto-merge off"; + if (entity.mergeable === "conflicting") return "Blocked: conflict"; + if (entity.reviewDecision !== "APPROVED") return "Waiting for approval"; + if (entity.checksRollup !== "success") return "Waiting for checks"; + if (entity.mergeable !== "clean") return "Waiting for checks"; + if (isPrEntityAutoMergeReady(entity)) return "Ready to merge"; + return "Waiting for checks"; +} + +/** Whether the entity is in a hard conflict (Merge must be disabled, R11). */ +export function isPrConflicting(entity: PrEntity): boolean { + return entity.mergeable === "conflicting"; +} + +/** Structured rejection message for the R16 column-move-backward block. */ +export const PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE = + "This task has an open PR. Merge or close the PR before moving it back."; + +/** + * R16: should a column move be blocked because the task has an open PR? + * + * A "backward" move (lower column index) of a task that still has an ACTIVE + * (non-terminal) PR entity is rejected — the PR's lifecycle is workflow-owned and + * dragging the card back would orphan the open GitHub PR. Forward moves and moves + * of tasks whose PR is terminal (merged/closed/failed → no active entity) pass. + * + * Pure so the move route and tests consult one definition. + */ +export function isBackwardMoveBlockedByOpenPr(input: { + fromIndex: number; + toIndex: number; + activePrEntity: Pick | null | undefined; +}): boolean { + const { fromIndex, toIndex, activePrEntity } = input; + if (fromIndex < 0 || toIndex < 0) return false; + if (toIndex >= fromIndex) return false; // not backward + return Boolean(activePrEntity && isPrEntityActive(activePrEntity)); +} + +/** + * Build the merge-readiness summary the view renders above the checks list. + * Pure derivation from authoritative entity state. + */ +export function buildPrSummary(entity: PrEntity, threads: PrThreadState[]) { + const pendingThreads = threads.filter((t) => t.outcome === "pending").length; + const disagreedThreads = threads.filter((t) => t.outcome === "disagreed").length; + return { + mergeable: entity.mergeable ?? "unknown", + reviewDecision: entity.reviewDecision ?? null, + checksRollup: entity.checksRollup ?? "none", + conflicting: isPrConflicting(entity), + autoMerge: entity.autoMerge, + autoMergeReason: autoMergeGateReason(entity), + autoMergeReady: isPrEntityAutoMergeReady(entity), + actionable: isPrEntityActionable(entity), + active: isPrEntityActive(entity), + pendingThreads, + disagreedThreads, + }; +} + +function serializePr(entity: PrEntity, threads: PrThreadState[]) { + return { + ...entity, + threads, + summary: buildPrSummary(entity, threads), + }; +} + +export function createPullRequestsRouter(store: TaskStore, options?: PullRequestsRouterOptions): Router { + const router = Router(); + + // GET /api/pull-requests — list active entities, optional repo/status filter. + router.get("/", async (_req, res) => { + const repoRaw = _req.query.repo; + const statusRaw = _req.query.status; + const repo = typeof repoRaw === "string" && repoRaw.trim() ? repoRaw.trim() : undefined; + const status = typeof statusRaw === "string" && statusRaw.trim() ? statusRaw.trim() : undefined; + if ( + status && + !["creating", "open", "responding", "merged", "closed", "failed"].includes(status) + ) { + throw badRequest( + "status must be one of: creating, open, responding, merged, closed, failed", + ); + } + + let entities = store.listActivePrEntities(); + if (repo) entities = entities.filter((e) => e.repo === repo); + if (status) entities = entities.filter((e) => e.state === status); + + const pullRequests = entities.map((entity) => + serializePr(entity, store.listPrThreadStates(entity.id)), + ); + res.json({ pullRequests }); + }); + + // GET /api/pull-requests/:id — entity + thread states + checks/merge/conflict summary. + router.get("/:id", async (req, res) => { + const id = String(req.params.id ?? "").trim(); + if (!id) throw badRequest("id is required"); + const entity = store.getPrEntity(id); + if (!entity) throw notFound("PR entity not found"); + res.json({ pullRequest: serializePr(entity, store.listPrThreadStates(id)) }); + }); + + /** + * Shared action handler: re-read the AUTHORITATIVE entity (never trust a client + * copy), gate it, then dispatch to the injected capability. Returns the freshly + * re-read serialized entity so the client replaces its stale copy. + */ + function makeAction( + name: string, + capability: ((input: { entity: PrEntity; projectId?: string }) => Promise>) | undefined, + opts: { requireActive?: boolean; requireState?: PrEntity["state"]; rejectConflict?: boolean } = {}, + ) { + return async (req: Request, res: import("express").Response) => { + const id = String(req.params.id ?? "").trim(); + if (!id) throw badRequest("id is required"); + + // Re-fetch authoritative state — the side effect must never gate on a + // stale client/SSE-delivered copy. + const entity = store.getPrEntity(id); + if (!entity) throw notFound("PR entity not found"); + + if (opts.requireState && entity.state !== opts.requireState) { + throw new ApiError(409, `PR is not in '${opts.requireState}' state`, { + code: "pr-wrong-state", + retryable: false, + }); + } + if (opts.requireActive && !isPrEntityActive(entity)) { + throw new ApiError(409, "PR is already terminal (merged/closed/failed)", { + code: "pr-terminal", + retryable: false, + }); + } + if (opts.rejectConflict && isPrConflicting(entity)) { + throw new ApiError(409, "Resolve conflicts on GitHub before merging", { + code: "pr-conflict", + retryable: false, + }); + } + + if (!capability) { + throw badRequest(`${name} is unavailable`); + } + + const result = await capability({ entity, projectId: parseProjectId(req) }); + // Re-read after the action so the response reflects authoritative state. + const fresh = store.getPrEntity(id) ?? entity; + res.json({ + ...result, + pullRequest: serializePr(fresh, store.listPrThreadStates(id)), + }); + }; + } + + router.post("/:id/approve", makeAction("Approve", options?.approvePr, { requireActive: true })); + router.post( + "/:id/merge", + makeAction("Merge", options?.mergePr, { requireActive: true, rejectConflict: true }), + ); + router.post("/:id/retry", makeAction("Retry", options?.retryPr, { requireActive: true })); + router.post("/:id/close", makeAction("Close", options?.closePr, { requireActive: true })); + router.post( + "/:id/retry-create", + makeAction("Retry PR creation", options?.retryCreate, { requireState: "failed" }), + ); + + // Toggle auto-merge. Re-reads authoritative state then persists the flip. + router.post("/:id/automerge", async (req, res) => { + const id = String(req.params.id ?? "").trim(); + if (!id) throw badRequest("id is required"); + const entity = store.getPrEntity(id); + if (!entity) throw notFound("PR entity not found"); + if (!isPrEntityActive(entity)) { + throw new ApiError(409, "PR is already terminal (merged/closed/failed)", { + code: "pr-terminal", + retryable: false, + }); + } + const enabled = + typeof req.body?.enabled === "boolean" ? req.body.enabled : !entity.autoMerge; + const updated = store.updatePrEntity(id, { autoMerge: enabled }); + res.json({ pullRequest: serializePr(updated, store.listPrThreadStates(id)) }); + }); + + return router; +} diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index a876cc871e..a2bc6bf064 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -45,6 +45,7 @@ import { createTrackingIssueForTask } from "../github-tracking-hook.js"; import { parseGitHubBadgeUrl } from "./register-git-github.js"; import { planTaskWorktreePath, promoteHeldTask } from "@fusion/engine"; import { buildBoardWorkflowsPayload } from "./board-workflows.js"; +import { isBackwardMoveBlockedByOpenPr, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE } from "./register-pull-requests-routes.js"; import type { RunAuditEventInput } from "@fusion/core"; import { ApiError, badRequest, conflict, notFound } from "../api-error.js"; import type { ApiRoutesContext } from "./types.js"; @@ -1364,6 +1365,35 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw badRequest("preserveProgress must be a boolean"); } + // R16: block moving a PR-await task "backward" (e.g. in-review → in-progress) + // while it still has an open PR entity. The PR's lifecycle is workflow-owned; + // dragging the card back would orphan the open GitHub PR. The user must + // merge or close the PR first (a user-controlled release advances it + // forward; this guard only rejects backward drags). Once the entity is + // terminal (merged/closed/failed) the move is allowed. + const moveTarget = column as Column; + const guardTask = await scopedStore.getTask(req.params.id); + if (guardTask) { + const activePrEntity = + scopedStore.getActivePrEntityBySource?.("task", guardTask.id) ?? + (guardTask.branchContext?.groupId + ? scopedStore.getActivePrEntityBySource?.("branch-group", guardTask.branchContext.groupId) + : null); + if ( + isBackwardMoveBlockedByOpenPr({ + fromIndex: COLUMNS.indexOf(guardTask.column as Column), + toIndex: COLUMNS.indexOf(moveTarget), + activePrEntity, + }) + ) { + throw new ApiError(409, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE, { + code: "pr-open-blocks-move-back", + messageKey: "board.rejection.prOpenBlocksMoveBack", + retryable: false, + }); + } + } + // When manually promoting to in-progress, supply an allocator so // moveTask assigns a worktree path under its cross-task allocation // lock. This mirrors scheduler dispatch semantics — without it, a