From 1821943d062d9fcd9e963648a0334fe9127df970 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 19 Mar 2026 23:14:21 -0700 Subject: [PATCH] =?UTF-8?q?feat(review):=20PR=20context=20panel=20?= =?UTF-8?q?=E2=80=94=20summary,=20comments,=20and=20checks=20(#350)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(review): add PR context panel with summary, comments, and checks tabs When reviewing a GitHub PR, the ReviewPanel now shows icon tabs for Summary (PR body, labels, linked issues), Comments (chronological thread of comments + reviews), and Checks (CI status, merge readiness). Tabs only appear in PR mode; local review is unchanged. Data fetched lazily via new /api/pr-context endpoint on first tab click. Uses DOMPurify for safe HTML rendering of GitHub markdown content. Inline markdown renderer extended with link and bare URL support. Closes #348 Co-Authored-By: Claude Opus 4.6 (1M context) * fix(review): reset fetch guard on error and detect inline HTML in PR bodies Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- bun.lock | 10 +- package.json | 6 +- packages/review-editor/App.tsx | 1 + .../review-editor/components/PRChecksTab.tsx | 170 +++++++++ .../components/PRCommentsTab.tsx | 120 +++++++ .../review-editor/components/PRSummaryTab.tsx | 187 ++++++++++ .../review-editor/components/ReviewPanel.tsx | 333 ++++++++++++------ packages/review-editor/hooks/usePRContext.ts | 35 ++ .../utils/renderInlineMarkdown.tsx | 20 +- packages/server/pr.ts | 10 +- packages/server/review.ts | 26 +- packages/shared/pr-provider.ts | 127 +++++++ 12 files changed, 924 insertions(+), 121 deletions(-) create mode 100644 packages/review-editor/components/PRChecksTab.tsx create mode 100644 packages/review-editor/components/PRCommentsTab.tsx create mode 100644 packages/review-editor/components/PRSummaryTab.tsx create mode 100644 packages/review-editor/hooks/usePRContext.ts diff --git a/bun.lock b/bun.lock index 4727e716..968ff4cb 100644 --- a/bun.lock +++ b/bun.lock @@ -6,6 +6,10 @@ "dependencies": { "@pierre/diffs": "^1.1.0-beta.19", "diff": "^8.0.3", + "dompurify": "^3.3.3", + }, + "devDependencies": { + "@types/dompurify": "^3.2.0", }, }, "apps/hook": { @@ -929,6 +933,8 @@ "@types/debug": ["@types/debug@4.1.12", "", { "dependencies": { "@types/ms": "*" } }, "sha512-vIChWdVG3LG1SMxEvI/AK+FWJthlrqlTu7fbrlywTkkaONwk/UAGaULXRlf8vkzFBLVm0zkMdCquhL5aOjhXPQ=="], + "@types/dompurify": ["@types/dompurify@3.2.0", "", { "dependencies": { "dompurify": "*" } }, "sha512-Fgg31wv9QbLDA0SpTOXO3MaxySc4DKGLi8sna4/Utjo4r3ZRPdCt4UQee8BWr+Q5z21yifghREPJGYaEOEIACg=="], + "@types/estree": ["@types/estree@1.0.8", "", {}, "sha512-dWHzHa2WqEXI/O1E9OjrocMTKJl2mSrEolh1Iomrv6U+JuNwaHXsXx9bLu5gG7BUWFIN0skIQJQ/L1rIex4X6w=="], "@types/estree-jsx": ["@types/estree-jsx@1.0.5", "", { "dependencies": { "@types/estree": "*" } }, "sha512-52CcUVNFyfb1A2ALocQw/Dd1BQFNmSdkuC3BkZ6iqhdMfQz7JWOFRuJFloOzjk+6WijU56m9oKXFAXc7o3Towg=="], @@ -1301,7 +1307,7 @@ "domhandler": ["domhandler@5.0.3", "", { "dependencies": { "domelementtype": "^2.3.0" } }, "sha512-cgwlv/1iFQiFnU96XXgROh8xTeetsnJiDsTc7TYCLFd9+/WNkIqPTxiM/8pSd8VIrhXGTf1Ny1q1hquVqDJB5w=="], - "dompurify": ["dompurify@3.3.1", "", { "optionalDependencies": { "@types/trusted-types": "^2.0.7" } }, "sha512-qkdCKzLNtrgPFP1Vo+98FRzJnBRGe4ffyCea9IwHB1fyxPOeNTHpLKYGd4Uk9xvNoH0ZoOjwZxNptyMwqrId1Q=="], + "dompurify": ["dompurify@3.3.3", "", { "optionalDependencies": { "@types/trusted-types": "^2.0.7" } }, "sha512-Oj6pzI2+RqBfFG+qOaOLbFXLQ90ARpcGG6UePL82bJLtdsa6CYJD7nmiU8MW9nQNOtCHV3lZ/Bzq1X0QYbBZCA=="], "domutils": ["domutils@3.2.2", "", { "dependencies": { "dom-serializer": "^2.0.0", "domelementtype": "^2.3.0", "domhandler": "^5.0.3" } }, "sha512-6kZKyUajlDuqlHKVX1w7gyslj9MPIXzIFiz/rGu35uC1wMi+kMhQwGhl4lt9unC9Vb9INnY9Z3/ZA3+FhASLaw=="], @@ -2499,6 +2505,8 @@ "mdast-util-find-and-replace/escape-string-regexp": ["escape-string-regexp@5.0.0", "", {}, "sha512-/veY75JbMK4j1yjvuUxuVsiS/hr/4iHs9FTT6cgTexxdE0Ly/glccBAkloH/DofkjRbZU3bnoj38mOmhkZ0lHw=="], + "mermaid/dompurify": ["dompurify@3.3.1", "", { "optionalDependencies": { "@types/trusted-types": "^2.0.7" } }, "sha512-qkdCKzLNtrgPFP1Vo+98FRzJnBRGe4ffyCea9IwHB1fyxPOeNTHpLKYGd4Uk9xvNoH0ZoOjwZxNptyMwqrId1Q=="], + "mermaid/marked": ["marked@16.4.2", "", { "bin": { "marked": "bin/marked.js" } }, "sha512-TI3V8YYWvkVf3KJe1dRkpnjs68JUPyEa5vjKrp1XEEJUAOaQc+Qj+L1qWbPd0SJuAdQkFU0h73sXXqwDYxsiDA=="], "micromatch/picomatch": ["picomatch@2.3.1", "", {}, "sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA=="], diff --git a/package.json b/package.json index 1250d89e..3fe96f12 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,10 @@ }, "dependencies": { "@pierre/diffs": "^1.1.0-beta.19", - "diff": "^8.0.3" + "diff": "^8.0.3", + "dompurify": "^3.3.3" + }, + "devDependencies": { + "@types/dompurify": "^3.2.0" } } diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 43ed7705..c901528c 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -980,6 +980,7 @@ const ReviewApp: React.FC = () => { width={panelResize.width} editorAnnotations={editorAnnotations} onDeleteEditorAnnotation={deleteEditorAnnotation} + prMetadata={prMetadata} /> diff --git a/packages/review-editor/components/PRChecksTab.tsx b/packages/review-editor/components/PRChecksTab.tsx new file mode 100644 index 00000000..f1f0f21f --- /dev/null +++ b/packages/review-editor/components/PRChecksTab.tsx @@ -0,0 +1,170 @@ +import React, { useMemo } from 'react'; +import type { PRContext, PRCheck } from '@plannotator/shared/pr-provider'; + +interface PRChecksTabProps { + context: PRContext; +} + +const DECISION_STYLES: Record = { + APPROVED: { bg: 'bg-success/15', text: 'text-success', label: 'Approved' }, + CHANGES_REQUESTED: { bg: 'bg-destructive/15', text: 'text-destructive', label: 'Changes Requested' }, + REVIEW_REQUIRED: { bg: 'bg-yellow-500/15', text: 'text-yellow-500', label: 'Review Required' }, +}; + +const MERGE_STATUS_STYLES: Record = { + CLEAN: { bg: 'bg-success/15', text: 'text-success', label: 'Ready to merge' }, + BLOCKED: { bg: 'bg-yellow-500/15', text: 'text-yellow-500', label: 'Blocked' }, + BEHIND: { bg: 'bg-yellow-500/15', text: 'text-yellow-500', label: 'Behind base branch' }, + DIRTY: { bg: 'bg-destructive/15', text: 'text-destructive', label: 'Has conflicts' }, + UNKNOWN: { bg: 'bg-muted', text: 'text-muted-foreground', label: 'Unknown' }, +}; + +function CheckIcon({ check }: { check: PRCheck }) { + if (check.status !== 'COMPLETED') { + // In progress or queued + return ( + + + + ); + } + + if (check.conclusion === 'SUCCESS') { + return ( + + + + ); + } + + if (check.conclusion === 'FAILURE' || check.conclusion === 'TIMED_OUT') { + return ( + + + + ); + } + + if (check.conclusion === 'SKIPPED' || check.conclusion === 'NEUTRAL') { + return ( + + + + ); + } + + // Fallback + return ( + + + + ); +} + +export const PRChecksTab: React.FC = ({ context }) => { + // Group checks by workflow + const groupedChecks = useMemo(() => { + const groups = new Map(); + for (const check of context.checks) { + const workflow = check.workflowName || 'Other'; + const existing = groups.get(workflow) || []; + existing.push(check); + groups.set(workflow, existing); + } + return groups; + }, [context.checks]); + + const checkSummary = useMemo(() => { + const total = context.checks.length; + const passed = context.checks.filter(c => c.conclusion === 'SUCCESS').length; + const failed = context.checks.filter(c => c.conclusion === 'FAILURE' || c.conclusion === 'TIMED_OUT').length; + const pending = context.checks.filter(c => c.status !== 'COMPLETED').length; + const skipped = context.checks.filter(c => c.conclusion === 'SKIPPED' || c.conclusion === 'NEUTRAL').length; + return { total, passed, failed, pending, skipped }; + }, [context.checks]); + + const isMerged = context.state === 'MERGED'; + const isClosed = context.state === 'CLOSED'; + const decisionStyle = DECISION_STYLES[context.reviewDecision]; + const mergeStyle = isMerged + ? { bg: 'bg-violet-500/15', text: 'text-violet-400', label: 'Merged' } + : isClosed + ? { bg: 'bg-destructive/15', text: 'text-destructive', label: 'Closed' } + : MERGE_STATUS_STYLES[context.mergeStateStatus] ?? MERGE_STATUS_STYLES.UNKNOWN; + const mergeableConflict = !isMerged && !isClosed && context.mergeable === 'CONFLICTING'; + + return ( +
+ {/* Merge readiness */} +
+

+ Merge Status +

+ +
+ {/* Review decision */} + {decisionStyle && ( +
+ + {decisionStyle.label} + +
+ )} + + {/* Merge state */} +
+ + {mergeableConflict ? 'Has merge conflicts' : mergeStyle.label} + +
+
+
+ + {/* Check runs */} + {context.checks.length > 0 && ( +
+
+

+ Checks +

+ + {checkSummary.passed}/{checkSummary.total} passed + {checkSummary.failed > 0 && {checkSummary.failed} failed} + {checkSummary.pending > 0 && {checkSummary.pending} pending} + +
+ +
+ {Array.from(groupedChecks.entries()).map(([workflow, checks]) => ( +
+
{workflow}
+
+ {checks.map((check, i) => ( + + + + {check.name} + + + ))} +
+
+ ))} +
+
+ )} + + {context.checks.length === 0 && !decisionStyle && ( +
+

No checks configured.

+
+ )} +
+ ); +}; diff --git a/packages/review-editor/components/PRCommentsTab.tsx b/packages/review-editor/components/PRCommentsTab.tsx new file mode 100644 index 00000000..a6d13122 --- /dev/null +++ b/packages/review-editor/components/PRCommentsTab.tsx @@ -0,0 +1,120 @@ +import React, { useMemo } from 'react'; +import type { PRContext, PRComment, PRReview } from '@plannotator/shared/pr-provider'; +import { MarkdownBody } from './PRSummaryTab'; + +interface PRCommentsTabProps { + context: PRContext; +} + +type TimelineEntry = + | { kind: 'comment'; data: PRComment } + | { kind: 'review'; data: PRReview }; + +const REVIEW_STATE_STYLES: Record = { + APPROVED: { bg: 'bg-success/15', text: 'text-success', label: 'Approved' }, + CHANGES_REQUESTED: { bg: 'bg-destructive/15', text: 'text-destructive', label: 'Changes Requested' }, + COMMENTED: { bg: 'bg-muted', text: 'text-muted-foreground', label: 'Commented' }, + DISMISSED: { bg: 'bg-muted', text: 'text-muted-foreground/60', label: 'Dismissed' }, +}; + +function formatRelativeTime(iso: string): string { + if (!iso) return ''; + const now = Date.now(); + const then = new Date(iso).getTime(); + if (isNaN(then)) return ''; + + const diff = now - then; + const minutes = Math.floor(diff / 60000); + const hours = Math.floor(minutes / 60); + const days = Math.floor(hours / 24); + + if (minutes < 1) return 'just now'; + if (minutes < 60) return `${minutes}m ago`; + if (hours < 24) return `${hours}h ago`; + if (days < 30) return `${days}d ago`; + + return new Date(iso).toLocaleDateString(undefined, { month: 'short', day: 'numeric' }); +} + +export const PRCommentsTab: React.FC = ({ context }) => { + const timeline = useMemo(() => { + const entries: TimelineEntry[] = [ + ...context.comments.map((c): TimelineEntry => ({ kind: 'comment', data: c })), + ...context.reviews + .filter((r) => r.state !== 'COMMENTED' || r.body) + .map((r): TimelineEntry => ({ kind: 'review', data: r })), + ]; + + entries.sort((a, b) => { + const timeA = a.kind === 'comment' ? a.data.createdAt : a.data.submittedAt; + const timeB = b.kind === 'comment' ? b.data.createdAt : b.data.submittedAt; + return new Date(timeA).getTime() - new Date(timeB).getTime(); + }); + + return entries; + }, [context.comments, context.reviews]); + + if (timeline.length === 0) { + return ( +
+
+ + + +
+

No comments on this PR.

+
+ ); + } + + return ( +
+ {timeline.map((entry) => { + if (entry.kind === 'review') { + const review = entry.data; + const style = REVIEW_STATE_STYLES[review.state] ?? REVIEW_STATE_STYLES.COMMENTED; + + return ( +
+
+
+ + {review.author || 'unknown'} + + + {style.label} + +
+ + {formatRelativeTime(review.submittedAt)} + +
+ {review.body && ( +
+ +
+ )} +
+ ); + } + + const comment = entry.data; + return ( +
+
+ + {comment.author || 'unknown'} + + + {formatRelativeTime(comment.createdAt)} + +
+
+ +
+
+ ); + })} +
+ ); +}; diff --git a/packages/review-editor/components/PRSummaryTab.tsx b/packages/review-editor/components/PRSummaryTab.tsx new file mode 100644 index 00000000..fc130aac --- /dev/null +++ b/packages/review-editor/components/PRSummaryTab.tsx @@ -0,0 +1,187 @@ +import React, { useMemo } from 'react'; +import DOMPurify from 'dompurify'; +import { parseMarkdownToBlocks } from '@plannotator/ui/utils/parser'; +import { renderInlineMarkdown } from '../utils/renderInlineMarkdown'; +import type { PRContext } from '@plannotator/shared/pr-provider'; +import type { PRMetadata } from '@plannotator/shared/pr-provider'; + +interface PRSummaryTabProps { + context: PRContext; + metadata: PRMetadata; +} + +/** Check if content contains HTML tags that should be rendered natively. */ +const HTML_TAG_RE = /<[a-z][a-z0-9]*[\s/>]/i; +const containsHtml = (text: string) => HTML_TAG_RE.test(text); + +/** Sanitize HTML using DOMPurify — defense-in-depth for GitHub API content. */ +function sanitizeHtml(html: string): string { + return DOMPurify.sanitize(html, { + ALLOWED_TAGS: [ + 'sub', 'sup', 'b', 'i', 'em', 'strong', 'br', 'hr', 'p', 'span', + 'del', 'ins', 'mark', 'small', 'abbr', 'kbd', 'var', 'samp', + 'details', 'summary', 'blockquote', 'ul', 'ol', 'li', + 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'pre', 'code', + 'table', 'thead', 'tbody', 'tr', 'th', 'td', + 'a', 'img', 'div', + ], + ALLOWED_ATTR: ['href', 'src', 'alt', 'title', 'rel', 'target', 'width', 'height', 'align'], + }); +} + +/** + * Render inline content — if it contains HTML tags, sanitize and render. + * Otherwise use our markdown renderer. + */ +function renderContent(content: string): React.ReactNode { + if (containsHtml(content)) { + return ; + } + return renderInlineMarkdown(content); +} + +/** Render a simplified block-level markdown view (no annotation infrastructure). */ +export function MarkdownBody({ markdown }: { markdown: string }) { + const blocks = useMemo(() => parseMarkdownToBlocks(markdown), [markdown]); + + return ( +
+ {blocks.map((block) => { + switch (block.type) { + case 'heading': { + const Tag = `h${Math.min(block.level ?? 1, 6)}` as keyof JSX.IntrinsicElements; + const sizes: Record = { + 1: 'text-base font-bold', + 2: 'text-sm font-semibold', + 3: 'text-xs font-semibold', + }; + return ( + + {renderContent(block.content)} + + ); + } + case 'code': + return ( + 
+                {block.content}
+
+ ); + case 'list-item': + return ( +
+ {block.checked !== undefined ? (block.checked ? '☑' : '☐') : '•'} + {renderContent(block.content)} +
+ ); + case 'blockquote': + return ( +
+ {renderContent(block.content)} +
+ ); + case 'hr': + return
; + default: + if (!block.content) return null; + // If the entire paragraph is HTML, sanitize and render + if (containsHtml(block.content)) { + return
; + } + return

{renderInlineMarkdown(block.content)}

; + } + })} +
+ ); +} + +export const PRSummaryTab: React.FC = ({ context, metadata }) => { + return ( +
+ {/* PR title + state */} +
+
+ + {context.isDraft ? 'Draft' : context.state} + + + {metadata.title} + +
+ +
+ {metadata.author} wants to merge {metadata.headBranch} into {metadata.baseBranch} +
+
+ + {/* Labels */} + {context.labels.length > 0 && ( +
+ {context.labels.map((label) => ( + + {label.name} + + ))} +
+ )} + + {/* Linked issues */} + {context.linkedIssues.length > 0 && ( +
+

+ Linked Issues +

+ {context.linkedIssues.map((issue) => ( + + + + + + #{issue.number} + {issue.repo && ({issue.repo})} + + ))} +
+ )} + + {/* PR body */} + {context.body ? ( +
+

+ Description +

+ +
+ ) : ( +

No description provided.

+ )} +
+ ); +}; diff --git a/packages/review-editor/components/ReviewPanel.tsx b/packages/review-editor/components/ReviewPanel.tsx index 5c9aac22..bfd700e5 100644 --- a/packages/review-editor/components/ReviewPanel.tsx +++ b/packages/review-editor/components/ReviewPanel.tsx @@ -1,10 +1,15 @@ -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import { CodeAnnotation, type EditorAnnotation } from '@plannotator/ui/types'; import { isCurrentUser } from '@plannotator/ui/utils/identity'; import { EditorAnnotationCard } from '@plannotator/ui/components/EditorAnnotationCard'; import { HighlightedCode } from './HighlightedCode'; import { detectLanguage } from '../utils/detectLanguage'; import { renderInlineMarkdown } from '../utils/renderInlineMarkdown'; +import { usePRContext } from '../hooks/usePRContext'; +import { PRSummaryTab } from './PRSummaryTab'; +import { PRCommentsTab } from './PRCommentsTab'; +import { PRChecksTab } from './PRChecksTab'; +import type { PRMetadata } from '@plannotator/shared/pr-provider'; interface DiffFile { path: string; @@ -12,6 +17,8 @@ interface DiffFile { deletions: number; } +type ReviewPanelTab = 'annotations' | 'summary' | 'comments' | 'checks'; + interface ReviewPanelProps { isOpen: boolean; onToggle: () => void; @@ -24,6 +31,7 @@ interface ReviewPanelProps { width?: number; editorAnnotations?: EditorAnnotation[]; onDeleteEditorAnnotation?: (id: string) => void; + prMetadata?: PRMetadata | null; } const SuggestionPreview: React.FC<{ code: string; originalCode?: string; language?: string }> = ({ code, originalCode, language }) => { @@ -87,6 +95,29 @@ function compareCodeAnnotations(a: CodeAnnotation, b: CodeAnnotation): number { : a.lineStart - b.lineStart; } +const PR_TABS: { id: ReviewPanelTab; label: string; icon: React.ReactNode }[] = [ + { id: 'annotations', label: 'Annotations', icon: ( + + + + )}, + { id: 'summary', label: 'Summary', icon: ( + + + + )}, + { id: 'comments', label: 'Comments', icon: ( + + + + )}, + { id: 'checks', label: 'Checks', icon: ( + + + + )}, +]; + export const ReviewPanel: React.FC = ({ isOpen, onToggle, @@ -99,9 +130,33 @@ export const ReviewPanel: React.FC = ({ width, editorAnnotations, onDeleteEditorAnnotation, + prMetadata, }) => { const totalCount = annotations.length + (editorAnnotations?.length ?? 0); const [copied, setCopied] = useState(false); + const [activeTab, setActiveTab] = useState('annotations'); + const hasPRTabs = !!prMetadata; + + const { prContext, isLoading: isPRContextLoading, error: prContextError, fetchContext } = usePRContext(prMetadata ?? null); + + // Fetch PR context on first click of a PR tab + const handleTabChange = (tab: ReviewPanelTab) => { + setActiveTab(tab); + if (tab !== 'annotations' && !prContext && !isPRContextLoading) { + fetchContext(); + } + }; + + // Small status dot for checks tab + const checksStatusDot = prContext + ? prContext.checks.some(c => c.conclusion === 'FAILURE' || c.conclusion === 'TIMED_OUT') + ? 'bg-destructive' + : prContext.checks.some(c => c.status !== 'COMPLETED') + ? 'bg-yellow-500' + : prContext.checks.length > 0 + ? 'bg-success' + : null + : null; const handleQuickCopy = async () => { if (!feedbackMarkdown) return; @@ -113,6 +168,7 @@ export const ReviewPanel: React.FC = ({ console.error('Failed to copy:', e); } }; + // Group annotations by file const groupedAnnotations = React.useMemo(() => { const grouped = new Map(); @@ -121,7 +177,6 @@ export const ReviewPanel: React.FC = ({ existing.push(ann); grouped.set(ann.filePath, existing); } - // Sort file comments first, then line comments for (const [, anns] of grouped) { anns.sort(compareCodeAnnotations); } @@ -134,136 +189,186 @@ export const ReviewPanel: React.FC = ({