diff --git a/packages/core/src/git/router-schemas.ts b/packages/core/src/git/router-schemas.ts index d50f37dadd..54c94a490e 100644 --- a/packages/core/src/git/router-schemas.ts +++ b/packages/core/src/git/router-schemas.ts @@ -434,6 +434,37 @@ export const updatePrByUrlOutput = z.object({ }); export type UpdatePrByUrlOutput = z.infer; +export type { + ApprovePrOutput, + GetPrChecksOutput, + GetPrCommentsOutput, + MergePrOutput, + PrCheck, + PrCheckBucket, + PrConversationComment, + PrInfoByUrlOutput, + PrMergeMethod, +} from "@posthog/shared"; +// Native PR review schemas (PR overview, approve/merge, CI checks, +// conversation comments) are defined once in `@posthog/shared`'s git domain +// and re-exported here for the host router and UI. +export { + approvePrInput, + approvePrOutput, + getPrChecksInput, + getPrChecksOutput, + getPrCommentsInput, + getPrCommentsOutput, + getPrInfoByUrlInput, + getPrInfoByUrlOutput, + mergePrInput, + mergePrOutput, + prCheckBucketSchema, + prCheckSchema, + prConversationCommentSchema, + prMergeMethodSchema, +} from "@posthog/shared"; + export const getBranchChangedFilesInput = z.object({ repo: z.string(), branch: z.string(), diff --git a/packages/git/src/gh.ts b/packages/git/src/gh.ts index 4314a48aae..46d59e6516 100644 --- a/packages/git/src/gh.ts +++ b/packages/git/src/gh.ts @@ -25,8 +25,17 @@ export interface GhExecOptions { * MCP tool awaiting it — indefinitely. Omit for no timeout. */ timeoutMs?: number; + /** + * Max stdout/stderr bytes before the child is killed. Node's execFile + * default is 1 MiB, which paginated `gh api` calls (PR files, comments) + * blow past on busy PRs — the call then dies with "maxBuffer length + * exceeded" instead of returning data. + */ + maxBuffer?: number; } +const DEFAULT_MAX_BUFFER = 32 * 1024 * 1024; + export function execGh( args: string[], options: GhExecOptions = {}, @@ -37,7 +46,12 @@ export function execGh( const child = childProcess.execFile( "gh", args, - { cwd: options.cwd, env, timeout: options.timeoutMs ?? 0 }, + { + cwd: options.cwd, + env, + timeout: options.timeoutMs ?? 0, + maxBuffer: options.maxBuffer ?? DEFAULT_MAX_BUFFER, + }, (error, stdout, stderr) => { if (!error) { resolve({ stdout, stderr, exitCode: 0 }); diff --git a/packages/host-router/src/routers/git.router.ts b/packages/host-router/src/routers/git.router.ts index 35c98088cd..79ffa22dfb 100644 --- a/packages/host-router/src/routers/git.router.ts +++ b/packages/host-router/src/routers/git.router.ts @@ -10,6 +10,8 @@ import { GIT_WORKSPACE_CLIENT, } from "@posthog/core/git/identifiers"; import { + approvePrInput, + approvePrOutput, checkoutBranchInput, checkoutBranchOutput, cloneRepositoryInput, @@ -58,10 +60,16 @@ import { getLocalBranchChangedFilesOutput, getPrChangedFilesInput, getPrChangedFilesOutput, + getPrChecksInput, + getPrChecksOutput, + getPrCommentsInput, + getPrCommentsOutput, getPrDetailsByUrlInput, getPrDetailsByUrlOutput, getPrDiffStatsBatchInput, getPrDiffStatsBatchOutput, + getPrInfoByUrlInput, + getPrInfoByUrlOutput, getPrReviewCommentsInput, getPrReviewCommentsOutput, getPrTemplateInput, @@ -72,6 +80,8 @@ import { ghStatusOutput, gitStateSnapshotSchema, gitStatusOutput, + mergePrInput, + mergePrOutput, openPrInput, openPrOutput, prStatusInput, @@ -529,6 +539,52 @@ export const gitRouter = router({ }), ), + getPrInfoByUrl: publicProcedure + .input(getPrInfoByUrlInput) + .output(getPrInfoByUrlOutput.nullable()) + .query(({ ctx, input }) => + getWorkspaceClient(ctx.container).git.getPrInfoByUrl.query({ + prUrl: input.prUrl, + }), + ), + + getPrChecks: publicProcedure + .input(getPrChecksInput) + .output(getPrChecksOutput) + .query(({ ctx, input }) => + getWorkspaceClient(ctx.container).git.getPrChecks.query({ + prUrl: input.prUrl, + }), + ), + + getPrComments: publicProcedure + .input(getPrCommentsInput) + .output(getPrCommentsOutput) + .query(({ ctx, input }) => + getWorkspaceClient(ctx.container).git.getPrComments.query({ + prUrl: input.prUrl, + }), + ), + + approvePr: publicProcedure + .input(approvePrInput) + .output(approvePrOutput) + .mutation(({ ctx, input }) => + getWorkspaceClient(ctx.container).git.approvePr.mutate({ + prUrl: input.prUrl, + }), + ), + + mergePr: publicProcedure + .input(mergePrInput) + .output(mergePrOutput) + .mutation(({ ctx, input }) => + getWorkspaceClient(ctx.container).git.mergePr.mutate({ + prUrl: input.prUrl, + method: input.method, + }), + ), + getPrReviewComments: publicProcedure .input(getPrReviewCommentsInput) .output(getPrReviewCommentsOutput) diff --git a/packages/shared/src/git-domain.ts b/packages/shared/src/git-domain.ts index 80a0ee885a..24cb09add1 100644 --- a/packages/shared/src/git-domain.ts +++ b/packages/shared/src/git-domain.ts @@ -67,3 +67,105 @@ export type GithubPullRequest = GithubRef; // git-interaction UI (PR status menu actions). export const prActionTypeSchema = z.enum(["close", "reopen", "ready", "draft"]); export type PrActionType = z.infer; + +// Native PR review schemas (PR overview, approve/merge, CI checks, +// conversation comments). Defined once here and re-exported by +// `@posthog/core/git/router-schemas` and workspace-server's git schemas so +// the tRPC layers on both sides of the boundary share one source of truth. + +/** Full PR overview (title/body/branches/stats) for the native in-app PR view. */ +export const getPrInfoByUrlInput = z.object({ prUrl: z.string() }); + +export const getPrInfoByUrlOutput = z.object({ + number: z.number(), + title: z.string(), + body: z.string(), + author: z.string().nullable(), + state: z.string(), + merged: z.boolean(), + draft: z.boolean(), + /** GitHub computes mergeability asynchronously; null until it settles. */ + mergeable: z.boolean().nullable(), + /** + * GitHub's `mergeable_state`: "clean" | "unstable" | "blocked" | "dirty" | + * "behind" | "draft" | "unknown". "blocked" means branch protection forbids + * the merge for this viewer — e.g. a required approving review is missing + * (authors can't approve their own PRs) or required checks are failing. + * Kept as a plain string so an undocumented value can't fail the parse. + */ + mergeStateStatus: z.string().catch("unknown"), + baseRefName: z.string().nullable(), + headRefName: z.string().nullable(), + additions: z.number(), + deletions: z.number(), + changedFiles: z.number(), +}); + +export type PrInfoByUrlOutput = z.infer; + +export const approvePrInput = z.object({ prUrl: z.string() }); + +export const approvePrOutput = z.object({ + success: z.boolean(), + message: z.string(), +}); + +export type ApprovePrOutput = z.infer; + +export const prMergeMethodSchema = z.enum(["merge", "squash", "rebase"]); +export type PrMergeMethod = z.infer; + +export const mergePrInput = z.object({ + prUrl: z.string(), + method: prMergeMethodSchema, +}); + +export const mergePrOutput = z.object({ + success: z.boolean(), + message: z.string(), +}); + +export type MergePrOutput = z.infer; + +// CI check runs / commit statuses for a PR, via `gh pr checks`. +export const prCheckBucketSchema = z.enum([ + "fail", + "cancel", + "pending", + "pass", + "skipping", +]); +export type PrCheckBucket = z.infer; + +export const prCheckSchema = z.object({ + name: z.string(), + bucket: prCheckBucketSchema, + link: z.string().nullable(), + workflow: z.string().nullable(), + description: z.string().nullable(), +}); +export type PrCheck = z.infer; + +export const getPrChecksInput = z.object({ prUrl: z.string() }); +/** Null means the checks couldn't be fetched; [] means none reported. */ +export const getPrChecksOutput = z.array(prCheckSchema).nullable(); +export type GetPrChecksOutput = z.infer; + +// Conversation (issue) comments and review summaries on a PR. Inline review +// comments live in `prReviewThreadSchema` above. +export const prConversationCommentSchema = z.object({ + id: z.number(), + author: z.string(), + avatarUrl: z.string().nullable(), + body: z.string(), + createdAt: z.string(), + url: z.string().nullable(), +}); +export type PrConversationComment = z.infer; + +export const getPrCommentsInput = z.object({ prUrl: z.string() }); +/** Null means the comments couldn't be fetched; [] means none. */ +export const getPrCommentsOutput = z + .array(prConversationCommentSchema) + .nullable(); +export type GetPrCommentsOutput = z.infer; diff --git a/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx b/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx index eda4f18b06..e648537e1d 100644 --- a/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx +++ b/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx @@ -2,7 +2,7 @@ import { type FileDiffMetadata, processFile } from "@pierre/diffs"; import type { PrCommentThread } from "@posthog/core/code-review/types"; import { isBinaryFile } from "@posthog/shared"; import type { ChangedFile } from "@posthog/shared/domain-types"; -import { useMemo } from "react"; +import { type ReactNode, useMemo } from "react"; import { DeferredDiffPlaceholder, DiffFileHeader } from "../reviewShellParts"; import type { DiffOptions } from "../types"; import { InteractiveFileDiff } from "./InteractiveFileDiff"; @@ -17,6 +17,8 @@ interface PatchedFileDiffProps { externalUrl?: string; prUrl?: string | null; commentThreads?: Map; + /** Extra controls in the file header row (e.g. a "Viewed" toggle). */ + headerTrailing?: ReactNode; } export function PatchedFileDiff({ @@ -29,6 +31,7 @@ export function PatchedFileDiff({ externalUrl, prUrl, commentThreads, + headerTrailing, }: PatchedFileDiffProps) { const fileDiff = useMemo((): FileDiffMetadata | undefined => { if (!file.patch) return undefined; @@ -60,6 +63,7 @@ export function PatchedFileDiff({ collapsed={collapsed} onToggle={onToggle} externalUrl={externalUrl} + headerTrailing={headerTrailing} /> ); } @@ -74,6 +78,7 @@ export function PatchedFileDiff({ collapsed={collapsed} onToggle={onToggle} externalUrl={externalUrl} + headerTrailing={headerTrailing} /> ); } @@ -90,6 +95,7 @@ export function PatchedFileDiff({ fileDiff={fd} collapsed={collapsed} onToggle={onToggle} + trailing={headerTrailing} /> )} /> diff --git a/packages/ui/src/features/code-review/reviewShellParts.tsx b/packages/ui/src/features/code-review/reviewShellParts.tsx index e320965c57..c2bb0af7a6 100644 --- a/packages/ui/src/features/code-review/reviewShellParts.tsx +++ b/packages/ui/src/features/code-review/reviewShellParts.tsx @@ -29,7 +29,7 @@ export { const STICKY_HEADER_CSS = `[data-diffs-header] { position: sticky; top: 0; z-index: 1; background: var(--gray-2); }`; -function useDiffOptions() { +export function useDiffOptions() { const viewMode = useDiffViewerStore((s) => s.viewMode); const wordWrap = useDiffViewerStore((s) => s.wordWrap); const loadFullFiles = useDiffViewerStore((s) => s.loadFullFiles); @@ -209,6 +209,7 @@ export function DiffFileHeader({ onDiscard, onStage, staged, + trailing, }: { fileDiff: FileDiffMetadata; collapsed: boolean; @@ -217,6 +218,8 @@ export function DiffFileHeader({ onDiscard?: () => void; onStage?: () => void; staged?: boolean; + /** Extra controls rendered after the action buttons (e.g. a "Viewed" toggle). */ + trailing?: ReactNode; }) { const fullPath = fileDiff.prevName && fileDiff.prevName !== fileDiff.name @@ -234,7 +237,7 @@ export function DiffFileHeader({ collapsed={collapsed} onToggle={onToggle} trailing={ - (onStage || onDiscard || onOpenFile) && ( + (onStage || onDiscard || onOpenFile || trailing) && ( {onStage && ( @@ -278,6 +281,7 @@ export function DiffFileHeader({ )} + {trailing} ) } @@ -294,6 +298,7 @@ export function DeferredDiffPlaceholder({ onToggle, onShow, externalUrl, + headerTrailing, }: { filePath: string; linesAdded: number; @@ -303,6 +308,8 @@ export function DeferredDiffPlaceholder({ onToggle: () => void; onShow?: () => void; externalUrl?: string; + /** Extra controls in the header row (e.g. a "Viewed" toggle). */ + headerTrailing?: ReactNode; }) { const { dirPath, fileName } = splitFilePath(filePath); @@ -315,6 +322,13 @@ export function DeferredDiffPlaceholder({ deletions={linesRemoved} collapsed={collapsed} onToggle={onToggle} + trailing={ + headerTrailing && ( + + {headerTrailing} + + ) + } /> {!collapsed && (
diff --git a/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx b/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx index 0031823dfc..ccaf1944e5 100644 --- a/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx +++ b/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx @@ -49,6 +49,8 @@ interface InboxDetailFrameProps { Icon: ComponentType; title: string; }; + /** Sections rendered in the main column under the summary (e.g. PR files changed). */ + belowSummary?: ReactNode; /** Optional "Evidence" section icon + title; null hides it. */ evidenceSection: { Icon: ComponentType; @@ -76,6 +78,7 @@ export function InboxDetailFrame({ metaSuffix, primaryAction, summarySection, + belowSummary, evidenceSection, showDismiss = true, children, @@ -173,7 +176,7 @@ export function InboxDetailFrame({ */}
-
+
+ {belowSummary}
diff --git a/packages/ui/src/features/inbox/components/PullRequestDetail.tsx b/packages/ui/src/features/inbox/components/PullRequestDetail.tsx index c29555bf97..a4667ec27d 100644 --- a/packages/ui/src/features/inbox/components/PullRequestDetail.tsx +++ b/packages/ui/src/features/inbox/components/PullRequestDetail.tsx @@ -17,6 +17,10 @@ import { ReportTasksSection } from "@posthog/ui/features/inbox/components/Report import { SuggestedReviewersSection } from "@posthog/ui/features/inbox/components/SuggestedReviewersSection"; import { ReportImplementationPrLink } from "@posthog/ui/features/inbox/components/utils/ReportImplementationPrLink"; import { copyInboxReportLink } from "@posthog/ui/features/inbox/utils/copyInboxReportLink"; +import { PrChecksSection } from "@posthog/ui/features/pr-review/PrChecksSection"; +import { PrCommentsSection } from "@posthog/ui/features/pr-review/PrCommentsSection"; +import { PrFilesChangedSection } from "@posthog/ui/features/pr-review/PrFilesChangedSection"; +import { PrReviewActions } from "@posthog/ui/features/pr-review/PrReviewActions"; import { Text } from "@radix-ui/themes"; interface PullRequestDetailProps { @@ -111,11 +115,26 @@ function PullRequestDetailContent({ report }: { report: SignalReport }) { } summarySection={{ Icon: GitPullRequestIcon, title: "Summary" }} + belowSummary={ + prRef && report.implementation_pr_url ? ( + <> + + + + + + ) : undefined + } evidenceSection={{ Icon: MagnifyingGlassIcon, title: "Evidence" }} > - + ); } diff --git a/packages/ui/src/features/inbox/components/detail/ArtefactCommit.tsx b/packages/ui/src/features/inbox/components/detail/ArtefactCommit.tsx index ff924493b3..47cbf8fe01 100644 --- a/packages/ui/src/features/inbox/components/detail/ArtefactCommit.tsx +++ b/packages/ui/src/features/inbox/components/detail/ArtefactCommit.tsx @@ -9,15 +9,19 @@ import { useState } from "react"; /** * Renders a `commit` artefact: commit metadata plus a collapsible diff of the commit against * its parent, fetched lazily (on first expand) from the team's GitHub integration. + * `hideDiff` drops the diff toggle — used on the PR detail, where the main + * column already shows every changed file. */ export function ArtefactCommit({ reportId, artefactId, content, + hideDiff = false, }: { reportId: string; artefactId: string; content: CommitContent; + hideDiff?: boolean; }) { const [expanded, setExpanded] = useState(false); @@ -43,21 +47,23 @@ export function ArtefactCommit({ ) : null} - + {!hideDiff && ( + + )} - {expanded ? ( + {expanded && !hideDiff ? ( {diffQuery.isLoading ? ( ); case "task_run": @@ -392,9 +395,11 @@ function ArtefactBody({ function ArtefactRow({ reportId, artefact, + hideCommitDiffs, }: { reportId: string; artefact: AnySignalReportArtefact; + hideCommitDiffs?: boolean; }) { const [showRaw, setShowRaw] = useState(false); const location = locationLabel(artefact); @@ -436,7 +441,11 @@ function ArtefactRow({ - + {showRaw ? (
           {JSON.stringify(artefact, null, 2)}
@@ -449,9 +458,12 @@ function ArtefactRow({
 export function ArtefactLogList({
   reportId,
   artefacts,
+  hideCommitDiffs,
 }: {
   reportId: string;
   artefacts: AnySignalReportArtefact[];
+  /** Drop the per-commit diff toggle (PR detail shows the full diff already). */
+  hideCommitDiffs?: boolean;
 }) {
   if (artefacts.length === 0) {
     return null;
@@ -469,6 +481,7 @@ export function ArtefactLogList({
           key={artefact.id}
           reportId={reportId}
           artefact={artefact}
+          hideCommitDiffs={hideCommitDiffs}
         />
       ))}
     
diff --git a/packages/ui/src/features/inbox/components/detail/ReportActivitySection.tsx b/packages/ui/src/features/inbox/components/detail/ReportActivitySection.tsx
index 09b966df88..641442048d 100644
--- a/packages/ui/src/features/inbox/components/detail/ReportActivitySection.tsx
+++ b/packages/ui/src/features/inbox/components/detail/ReportActivitySection.tsx
@@ -10,7 +10,14 @@ import { Text } from "@radix-ui/themes";
  * wherever it is rendered. Renders nothing while loading or when the report
  * has no artefacts.
  */
-export function ReportActivitySection({ reportId }: { reportId: string }) {
+export function ReportActivitySection({
+  reportId,
+  hideCommitDiffs,
+}: {
+  reportId: string;
+  /** Drop the per-commit diff toggle (PR detail shows the full diff already). */
+  hideCommitDiffs?: boolean;
+}) {
   // The log is a live work record — agents append artefacts while the report is
   // open, so don't let the app-wide 5-minute staleTime sit on it. Poll gently
   // while mounted.
@@ -32,7 +39,11 @@ export function ReportActivitySection({ reportId }: { reportId: string }) {
         
       }
     >
-      
+      
     
   );
 }
diff --git a/packages/ui/src/features/pr-review/PrChecksSection.tsx b/packages/ui/src/features/pr-review/PrChecksSection.tsx
new file mode 100644
index 0000000000..7c07644795
--- /dev/null
+++ b/packages/ui/src/features/pr-review/PrChecksSection.tsx
@@ -0,0 +1,243 @@
+import {
+  ArrowSquareOutIcon,
+  CheckCircleIcon,
+  CheckIcon,
+  ChecksIcon,
+  CircleNotchIcon,
+  MinusCircleIcon,
+  ProhibitIcon,
+  XCircleIcon,
+} from "@phosphor-icons/react";
+import type { PrCheck, PrCheckBucket } from "@posthog/core/git/router-schemas";
+import { Spinner } from "@posthog/quill";
+import { DetailSection } from "@posthog/ui/features/inbox/components/DetailSection";
+import { useMemo, useState } from "react";
+import { openExternalUrl } from "../../shell/openExternal";
+import { usePrChecks } from "./usePrChecks";
+
+/** Display order: failed first, then running, then succeeded, skipped last. */
+const BUCKET_META: Array<{
+  bucket: PrCheckBucket;
+  label: string;
+  labelClass: string;
+}> = [
+  { bucket: "fail", label: "failed", labelClass: "text-(--red-11)" },
+  { bucket: "cancel", label: "cancelled", labelClass: "text-gray-11" },
+  { bucket: "pending", label: "running", labelClass: "text-(--amber-11)" },
+  { bucket: "pass", label: "successful", labelClass: "text-(--green-11)" },
+  { bucket: "skipping", label: "skipped", labelClass: "text-gray-10" },
+];
+
+const BUCKET_ORDER: Record = {
+  fail: 0,
+  cancel: 1,
+  pending: 2,
+  pass: 3,
+  skipping: 4,
+};
+
+/** Buckets that need attention — shown by default. */
+const DEFAULT_VISIBLE: PrCheckBucket[] = ["fail", "cancel", "pending"];
+
+interface PrChecksSectionProps {
+  prUrl: string;
+}
+
+/**
+ * CI status list for a PR. The header carries one checkbox per status bucket
+ * (failed, running, successful, …) that filters the rows below; only the
+ * attention-worthy buckets (failed, running) are shown by default.
+ */
+export function PrChecksSection({ prUrl }: PrChecksSectionProps) {
+  const checksQuery = usePrChecks(prUrl);
+  const checks = checksQuery.data;
+  const [visibleBuckets, setVisibleBuckets] = useState>(
+    () => new Set(DEFAULT_VISIBLE),
+  );
+
+  const sorted = useMemo(
+    () =>
+      [...(checks ?? [])].sort(
+        (a, b) =>
+          BUCKET_ORDER[a.bucket] - BUCKET_ORDER[b.bucket] ||
+          checkLabel(a).localeCompare(checkLabel(b)),
+      ),
+    [checks],
+  );
+
+  const counts = useMemo(() => {
+    const out: Record = {
+      fail: 0,
+      cancel: 0,
+      pending: 0,
+      pass: 0,
+      skipping: 0,
+    };
+    for (const check of sorted) out[check.bucket]++;
+    return out;
+  }, [sorted]);
+
+  if (checksQuery.isLoading) {
+    return (
+      
+        
+ + Loading checks… +
+
+ ); + } + + if (!checks) { + return ( + +
+ Couldn't load CI checks for this pull request. +
+
+ ); + } + + if (checks.length === 0) return null; + + const toggleBucket = (bucket: PrCheckBucket) => { + setVisibleBuckets((prev) => { + const next = new Set(prev); + if (next.has(bucket)) next.delete(bucket); + else next.add(bucket); + return next; + }); + }; + + const visible = sorted.filter((check) => visibleBuckets.has(check.bucket)); + + return ( + + {BUCKET_META.map(({ bucket, label, labelClass }) => + counts[bucket] > 0 ? ( + toggleBucket(bucket)} + labelClass={labelClass} + label={`${counts[bucket]} ${label}`} + /> + ) : null, + )} + + } + > + {visible.length > 0 && ( +
+ {visible.map((check, index) => ( + + ))} +
+ )} +
+ ); +} + +function BucketFilterCheckbox({ + checked, + onToggle, + label, + labelClass, +}: { + checked: boolean; + onToggle: () => void; + label: string; + labelClass: string; +}) { + return ( + + ); +} + +function CheckRow({ check }: { check: PrCheck }) { + return ( + + ); +} + +function checkLabel(check: PrCheck): string { + return check.workflow ? `${check.workflow} / ${check.name}` : check.name; +} + +function CheckBucketIcon({ bucket }: { bucket: PrCheckBucket }) { + switch (bucket) { + case "fail": + return ( + + ); + case "cancel": + return ; + case "pending": + return ( + + ); + case "pass": + return ( + + ); + case "skipping": + return ; + } +} diff --git a/packages/ui/src/features/pr-review/PrCommentsSection.tsx b/packages/ui/src/features/pr-review/PrCommentsSection.tsx new file mode 100644 index 0000000000..cf2d574367 --- /dev/null +++ b/packages/ui/src/features/pr-review/PrCommentsSection.tsx @@ -0,0 +1,179 @@ +import { ArrowSquareOutIcon, ChatCircleIcon } from "@phosphor-icons/react"; +import { Spinner } from "@posthog/quill"; +import { MarkdownRenderer } from "@posthog/ui/features/editor/components/MarkdownRenderer"; +import { NestedButton } from "@posthog/ui/primitives/NestedButton"; +import { RelativeTimestamp } from "@posthog/ui/primitives/RelativeTimestamp"; +import { useMemo, useState } from "react"; +import { openExternalUrl } from "../../shell/openExternal"; +import { PrSectionHeader } from "./PrSectionHeader"; +import { usePrComments } from "./usePrComments"; +import { usePrReviewThreads } from "./usePrReviewThreads"; + +interface CommentItem { + key: string; + author: string; + avatarUrl: string | null; + body: string; + createdAt: string; + url: string | null; + /** Set for inline review comments; null for conversation comments. */ + filePath: string | null; + resolved: boolean; +} + +interface PrCommentsSectionProps { + prUrl: string; +} + +/** + * Collapsed-by-default list of everything said on a PR: conversation + * comments plus inline review comments, in chronological order. + */ +export function PrCommentsSection({ prUrl }: PrCommentsSectionProps) { + const commentsQuery = usePrComments(prUrl); + const threadsQuery = usePrReviewThreads(prUrl); + const [collapsed, setCollapsed] = useState(true); + + const items = useMemo((): CommentItem[] => { + // Conversation items mix issue comments and review summaries, whose ids + // come from different GitHub id spaces — key on createdAt too. + const conversation = (commentsQuery.data ?? []).map( + (comment): CommentItem => ({ + key: `conv-${comment.id}-${comment.createdAt}`, + author: comment.author, + avatarUrl: comment.avatarUrl, + body: comment.body, + createdAt: comment.createdAt, + url: comment.url, + filePath: null, + resolved: false, + }), + ); + const review = (threadsQuery.data ?? []).flatMap((thread) => + thread.comments.map( + (comment): CommentItem => ({ + key: `review-${comment.id}`, + author: comment.user.login, + avatarUrl: comment.user.avatar_url || null, + body: comment.body, + createdAt: comment.created_at, + url: `${prUrl}#discussion_r${comment.id}`, + filePath: thread.filePath, + resolved: thread.isResolved, + }), + ), + ); + return [...conversation, ...review].sort((a, b) => + a.createdAt.localeCompare(b.createdAt), + ); + }, [commentsQuery.data, threadsQuery.data, prUrl]); + + if (commentsQuery.isLoading || threadsQuery.isLoading) { + return ( + {}} + summary={ + + + Loading… + + } + /> + ); + } + + const conversationFailed = + commentsQuery.isError || commentsQuery.data === null; + const threadsFailed = threadsQuery.isError; + + if (items.length === 0) { + // A partial failure with nothing else to show must read as an error — + // silently hiding the section here would look like "no comments". + if (conversationFailed || threadsFailed) { + const detail = + (commentsQuery.error ?? threadsQuery.error)?.message ?? null; + return ( +
+ Couldn't load comments for this pull request. + {detail && ( + + {detail} + + )} +
+ ); + } + return null; + } + + return ( +
+ setCollapsed(!collapsed)} + summary={ + + {items.length} comment{items.length === 1 ? "" : "s"} + + } + /> + {!collapsed && ( +
+ {items.map((item) => ( + + ))} +
+ )} +
+ ); +} + +function CommentRow({ item }: { item: CommentItem }) { + return ( +
+
+ {item.avatarUrl && ( + + )} + {item.author} + + {item.filePath && ( + + {item.filePath} + + )} + {item.resolved && ( + + Resolved + + )} + {item.url && ( + { + if (item.url) openExternalUrl(item.url); + }} + > + + + )} +
+
+ +
+
+ ); +} diff --git a/packages/ui/src/features/pr-review/PrFilesChangedSection.tsx b/packages/ui/src/features/pr-review/PrFilesChangedSection.tsx new file mode 100644 index 0000000000..63e10d8b04 --- /dev/null +++ b/packages/ui/src/features/pr-review/PrFilesChangedSection.tsx @@ -0,0 +1,214 @@ +import { CheckIcon, GitDiffIcon } from "@phosphor-icons/react"; +import { Button, Spinner } from "@posthog/quill"; +import { PatchedFileDiff } from "@posthog/ui/features/code-review/components/PatchedFileDiff"; +import { useDiffOptions } from "@posthog/ui/features/code-review/reviewShellParts"; +import { usePrChangedFiles } from "@posthog/ui/features/git-interaction/useGitQueries"; +import { DetailSection } from "@posthog/ui/features/inbox/components/DetailSection"; +import { NestedButton } from "@posthog/ui/primitives/NestedButton"; +import { useMemo, useRef, useState } from "react"; +import { + fileViewedFingerprint, + isFileViewed, + usePrViewedFilesStore, +} from "./prViewedFilesStore"; + +interface PrFilesChangedSectionProps { + prUrl: string; +} + +/** + * GitHub-style "Files changed" list for a PR: one collapsible diff per file, + * all collapsed by default. An expanded file gets a footer row with the + * "Viewed" toggle; marking a file viewed folds it back up. + */ +export function PrFilesChangedSection({ prUrl }: PrFilesChangedSectionProps) { + const filesQuery = usePrChangedFiles(prUrl); + const diffOptions = useDiffOptions(); + const viewedByPr = usePrViewedFilesStore((s) => s.viewedByPr); + const markViewed = usePrViewedFilesStore((s) => s.markViewed); + const unmarkViewed = usePrViewedFilesStore((s) => s.unmarkViewed); + + // Per-file collapse overrides on top of a section-wide baseline, so + // expand/collapse-all is one state flip instead of a map rebuild. + const [baselineCollapsed, setBaselineCollapsed] = useState(true); + const [collapseOverrides, setCollapseOverrides] = useState< + Map + >(new Map()); + const fileContainerRefs = useRef | null>(null); + // Lazy init so the Map isn't rebuilt (and discarded) on every render. + if (fileContainerRefs.current === null) { + fileContainerRefs.current = new Map(); + } + const fileContainers = fileContainerRefs.current; + + const files = filesQuery.data; + + const viewedCount = useMemo( + () => + (files ?? []).filter((file) => isFileViewed(viewedByPr, prUrl, file)) + .length, + [files, viewedByPr, prUrl], + ); + + if (filesQuery.isLoading) { + return ( + +
+ + Loading changed files… +
+
+ ); + } + + if (filesQuery.isError || !files) { + return ( + +
+ Couldn't load the changed files for this pull request. +
+
+ ); + } + + if (files.length === 0) { + return ( + +
No changed files.
+
+ ); + } + + const isCollapsed = (path: string) => + collapseOverrides.get(path) ?? baselineCollapsed; + const allExpanded = files.every((file) => !isCollapsed(file.path)); + + const setAllCollapsed = (collapsed: boolean) => { + setBaselineCollapsed(collapsed); + setCollapseOverrides(new Map()); + }; + + return ( + + + {viewedCount} / {files.length} viewed + + + + } + > +
+ {files.map((file) => { + const viewed = isFileViewed(viewedByPr, prUrl, file); + const collapsed = isCollapsed(file.path); + const setCollapsed = (next: boolean) => + setCollapseOverrides((prev) => new Map(prev).set(file.path, next)); + // Folding removes a diff that can be taller than the viewport, + // which would leave it staring at blank space below — scroll the + // folded file back into view. rAF runs after React commits the + // collapse but before the browser paints. + const collapseAndReveal = () => { + setCollapsed(true); + requestAnimationFrame(() => { + fileContainers + .get(file.path) + ?.scrollIntoView({ block: "nearest" }); + }); + }; + const handleViewedChange = (next: boolean) => { + if (next) { + markViewed(prUrl, file.path, fileViewedFingerprint(file)); + // Fold the file away once it's read, like GitHub. + if (!collapsed) collapseAndReveal(); + } else { + unmarkViewed(prUrl, file.path); + } + }; + return ( +
{ + if (el) fileContainers.set(file.path, el); + else fileContainers.delete(file.path); + }} + className="overflow-hidden rounded-md border border-(--gray-5)" + > + setCollapsed(!collapsed)} + externalUrl={`${prUrl}/files`} + prUrl={prUrl} + headerTrailing={ + collapsed ? ( + + ) : undefined + } + /> + {!collapsed && ( +
+ + +
+ )} +
+ ); + })} +
+
+ ); +} + +/** + * NestedButton because the collapsed placement sits inside the file header + * row, which is itself a ` + + )} +
+ ); + } + + const approved = approve.isSuccess && approve.data.success; + const approveDisabled = !info || approve.isPending || approved; + // Failed fetch (null / error) means CI status is unknown — that must lock + // the merge too, or a transient gh error would silently unlock red checks. + const checksUnavailable = checksQuery.isError || checksQuery.data === null; + // Branch protection ("blocked") is viewer-aware: it covers repos that + // require an approving review — including the PR author, who can't approve + // their own PR. Repos without such rules report "clean"/"unstable". + const blockedByProtection = info?.mergeStateStatus === "blocked"; + const behindBase = info?.mergeStateStatus === "behind"; + // Same gate as github.com: red checks, conflicts, or branch protection + // lock the merge button. + const mergeBlockedReason = draft + ? null // the draft branch below renders its own note + CTA + : failedChecks > 0 + ? `${failedChecks} check${failedChecks === 1 ? " is" : "s are"} failing — merging is blocked until they pass.` + : hasConflicts + ? "This branch has conflicts that must be resolved before merging." + : blockedByProtection + ? "Branch protection blocks this merge — an approving review from another user may be required." + : behindBase + ? "This branch is out of date with the base branch and must be updated before merging." + : checksUnavailable + ? "CI status couldn't be loaded — merging is blocked until checks are known." + : null; + const mergeDisabled = + !info || + draft || + merge.isPending || + checksQuery.data == null || + mergeBlockedReason !== null; + + return ( +
+ + + + + + + + } + /> + + {MERGE_METHODS.map((m) => ( + setMethod(m)}> + {MERGE_METHOD_LABELS[m]} + + ))} + + + + {draft && ( + <> + + Draft pull requests can't be merged. + + + + )} + {mergeBlockedReason && ( + + {mergeBlockedReason} + + )} +
+ ); +} diff --git a/packages/ui/src/features/pr-review/PrSectionHeader.tsx b/packages/ui/src/features/pr-review/PrSectionHeader.tsx new file mode 100644 index 0000000000..6a3227a421 --- /dev/null +++ b/packages/ui/src/features/pr-review/PrSectionHeader.tsx @@ -0,0 +1,50 @@ +import type { IconProps } from "@phosphor-icons/react"; +import { CaretDownIcon } from "@phosphor-icons/react"; +import { Text } from "@radix-ui/themes"; +import type { ComponentType, ReactNode } from "react"; + +/** + * Clickable section header matching the DetailSection chrome, for the + * collapsible PR sections (checks, comments). The right slot stays visible + * while collapsed so it can carry a summary. + */ +export function PrSectionHeader({ + Icon, + title, + collapsed, + onToggle, + summary, +}: { + Icon: ComponentType; + title: string; + collapsed: boolean; + onToggle: () => void; + summary?: ReactNode; +}) { + return ( + + ); +} diff --git a/packages/ui/src/features/pr-review/PullRequestView.tsx b/packages/ui/src/features/pr-review/PullRequestView.tsx new file mode 100644 index 0000000000..a060a39b39 --- /dev/null +++ b/packages/ui/src/features/pr-review/PullRequestView.tsx @@ -0,0 +1,166 @@ +import { + ArrowSquareOutIcon, + GitPullRequestIcon, + TextAlignLeftIcon, +} from "@phosphor-icons/react"; +import { parseGithubUrl } from "@posthog/git/utils"; +import { + Button, + Empty, + EmptyDescription, + EmptyHeader, + EmptyMedia, + EmptyTitle, + Spinner, +} from "@posthog/quill"; +import { MarkdownRenderer } from "@posthog/ui/features/editor/components/MarkdownRenderer"; +import { DetailSection } from "@posthog/ui/features/inbox/components/DetailSection"; +import { useMemo } from "react"; +import { openExternalUrl } from "../../shell/openExternal"; +import { PrChecksSection } from "./PrChecksSection"; +import { PrCommentsSection } from "./PrCommentsSection"; +import { PrFilesChangedSection } from "./PrFilesChangedSection"; +import { PrReviewActions } from "./PrReviewActions"; +import { usePrInfo } from "./usePrInfo"; + +interface PullRequestViewProps { + prUrl: string; +} + +/** + * Native, full-page pull request view: description, files changed (with + * GitHub-style "Viewed" tracking) and approve/merge actions — no browser + * round-trip. Opened from the sidebar's PR badge. + */ +export function PullRequestView({ prUrl }: PullRequestViewProps) { + const prRef = useMemo(() => (prUrl ? parseGithubUrl(prUrl) : null), [prUrl]); + const isPr = prRef?.kind === "pr"; + const infoQuery = usePrInfo(isPr ? prUrl : null); + const info = infoQuery.data; + + if (!isPr) { + return ( +
+ + + + + + No pull request + + This link doesn't point to a GitHub pull request. + + + +
+ ); + } + + return ( +
+
+
+
+ + + {prRef.owner}/{prRef.repo}#{prRef.number} + + {info && ( + + )} +
+
+

+ {info ? ( + info.title + ) : ( + + + Loading pull request… + + )} +

+ +
+ {info && ( +
+ {info.author && {info.author}} + {info.baseRefName && info.headRefName && ( + <> + · + + {info.headRefName} → {info.baseRefName} + + + )} + · + + +{info.additions}{" "} + −{info.deletions} + +
+ )} +
+ + + {info?.body.trim() ? ( +
+ +
+ ) : ( +
+ No description provided. +
+ )} +
+ + + + + + + + +
+
+ ); +} + +function PrStateBadge({ + state, + merged, + draft, +}: { + state: string; + merged: boolean; + draft: boolean; +}) { + const { label, className } = merged + ? { label: "Merged", className: "bg-(--purple-3) text-(--purple-11)" } + : state.toLowerCase() === "closed" + ? { label: "Closed", className: "bg-(--red-3) text-(--red-11)" } + : draft + ? { label: "Draft", className: "bg-(--gray-3) text-(--gray-11)" } + : { label: "Open", className: "bg-(--green-3) text-(--green-11)" }; + + return ( + + {label} + + ); +} diff --git a/packages/ui/src/features/pr-review/prViewedFilesStore.test.ts b/packages/ui/src/features/pr-review/prViewedFilesStore.test.ts new file mode 100644 index 0000000000..8bb224cdbe --- /dev/null +++ b/packages/ui/src/features/pr-review/prViewedFilesStore.test.ts @@ -0,0 +1,87 @@ +import type { ChangedFile } from "@posthog/shared/domain-types"; +import { beforeEach, describe, expect, it } from "vitest"; +import { + fileViewedFingerprint, + isFileViewed, + MAX_TRACKED_PRS, + usePrViewedFilesStore, +} from "./prViewedFilesStore"; + +const PR_URL = "https://github.com/posthog/code/pull/123"; + +function file(overrides: Partial = {}): ChangedFile { + return { + path: "src/a.ts", + status: "modified", + patch: "@@ -1 +1 @@\n-old\n+new", + ...overrides, + }; +} + +describe("prViewedFilesStore", () => { + beforeEach(() => { + usePrViewedFilesStore.setState({ viewedByPr: {} }); + }); + + it("marks a file viewed and reads it back", () => { + const f = file(); + usePrViewedFilesStore + .getState() + .markViewed(PR_URL, f.path, fileViewedFingerprint(f)); + + const { viewedByPr } = usePrViewedFilesStore.getState(); + expect(isFileViewed(viewedByPr, PR_URL, f)).toBe(true); + expect(isFileViewed(viewedByPr, "https://other", f)).toBe(false); + }); + + it("drops viewed state when the file's diff changes", () => { + const f = file(); + usePrViewedFilesStore + .getState() + .markViewed(PR_URL, f.path, fileViewedFingerprint(f)); + + const changed = file({ patch: "@@ -1 +1 @@\n-old\n+newer" }); + const { viewedByPr } = usePrViewedFilesStore.getState(); + expect(isFileViewed(viewedByPr, PR_URL, changed)).toBe(false); + }); + + it("unmarks a viewed file", () => { + const f = file(); + const store = usePrViewedFilesStore.getState(); + store.markViewed(PR_URL, f.path, fileViewedFingerprint(f)); + usePrViewedFilesStore.getState().unmarkViewed(PR_URL, f.path); + + const { viewedByPr } = usePrViewedFilesStore.getState(); + expect(isFileViewed(viewedByPr, PR_URL, f)).toBe(false); + }); + + it.each([ + ["status change", file({ status: "deleted" })], + ["patch change", file({ patch: "@@ -2 +2 @@\n-x\n+y" })], + [ + "line-count change on a patch-less file", + file({ patch: undefined, linesAdded: 3, linesRemoved: 1 }), + ], + ])("fingerprint differs on %s", (_label, changed) => { + expect(fileViewedFingerprint(changed)).not.toBe( + fileViewedFingerprint(file()), + ); + }); + + it("evicts the stalest PR beyond the cap", () => { + const f = file(); + const fingerprint = fileViewedFingerprint(f); + for (let i = 0; i < MAX_TRACKED_PRS + 1; i++) { + usePrViewedFilesStore + .getState() + .markViewed(`https://github.com/o/r/pull/${i}`, f.path, fingerprint); + } + + const { viewedByPr } = usePrViewedFilesStore.getState(); + expect(Object.keys(viewedByPr)).toHaveLength(MAX_TRACKED_PRS); + expect(viewedByPr["https://github.com/o/r/pull/0"]).toBeUndefined(); + expect( + viewedByPr[`https://github.com/o/r/pull/${MAX_TRACKED_PRS}`], + ).toBeDefined(); + }); +}); diff --git a/packages/ui/src/features/pr-review/prViewedFilesStore.ts b/packages/ui/src/features/pr-review/prViewedFilesStore.ts new file mode 100644 index 0000000000..15485a65f5 --- /dev/null +++ b/packages/ui/src/features/pr-review/prViewedFilesStore.ts @@ -0,0 +1,106 @@ +import type { ChangedFile } from "@posthog/shared/domain-types"; +import { create } from "zustand"; +import { persist } from "zustand/middleware"; + +/** Cap on tracked PRs so the persisted map can't grow unboundedly. */ +export const MAX_TRACKED_PRS = 50; + +interface PrViewedEntry { + /** Last write, used to evict the stalest PR past the cap. */ + updatedAt: number; + /** filePath -> fingerprint of the diff when the file was marked viewed. */ + files: Record; +} + +interface PrViewedFilesState { + viewedByPr: Record; +} + +interface PrViewedFilesActions { + markViewed: (prUrl: string, filePath: string, fingerprint: string) => void; + unmarkViewed: (prUrl: string, filePath: string) => void; +} + +type PrViewedFilesStore = PrViewedFilesState & PrViewedFilesActions; + +/** + * GitHub-style "Viewed" semantics: a file stays viewed only while its diff is + * unchanged. The fingerprint captures the patch when the user checks the box; + * if the PR gains commits that touch the file, the fingerprint stops matching + * and the file drops back to unviewed — same behavior as github.com. + */ +export function fileViewedFingerprint(file: ChangedFile): string { + const basis = `${file.status}:${ + file.patch ?? `${file.linesAdded ?? 0}/${file.linesRemoved ?? 0}` + }`; + // djb2 — cheap and stable; change detection only, not integrity. + let hash = 5381; + for (let i = 0; i < basis.length; i++) { + hash = ((hash << 5) + hash + basis.charCodeAt(i)) | 0; + } + return String(hash >>> 0); +} + +function evictStalest( + viewedByPr: Record, +): Record { + const urls = Object.keys(viewedByPr); + if (urls.length <= MAX_TRACKED_PRS) return viewedByPr; + let stalest: string | null = null; + let stalestAt = Number.POSITIVE_INFINITY; + for (const url of urls) { + const at = viewedByPr[url]?.updatedAt ?? 0; + if (at < stalestAt) { + stalest = url; + stalestAt = at; + } + } + if (stalest === null) return viewedByPr; + const next = { ...viewedByPr }; + delete next[stalest]; + return next; +} + +export const usePrViewedFilesStore = create()( + persist( + (set) => ({ + viewedByPr: {}, + markViewed: (prUrl, filePath, fingerprint) => + set((state) => { + const entry = state.viewedByPr[prUrl]; + const next: PrViewedEntry = { + updatedAt: Date.now(), + files: { ...entry?.files, [filePath]: fingerprint }, + }; + return { + viewedByPr: evictStalest({ + ...state.viewedByPr, + [prUrl]: next, + }), + }; + }), + unmarkViewed: (prUrl, filePath) => + set((state) => { + const entry = state.viewedByPr[prUrl]; + if (!entry || !(filePath in entry.files)) return state; + const files = { ...entry.files }; + delete files[filePath]; + return { + viewedByPr: { + ...state.viewedByPr, + [prUrl]: { updatedAt: Date.now(), files }, + }, + }; + }), + }), + { name: "pr-viewed-files" }, + ), +); + +export function isFileViewed( + viewedByPr: Record, + prUrl: string, + file: ChangedFile, +): boolean { + return viewedByPr[prUrl]?.files[file.path] === fileViewedFingerprint(file); +} diff --git a/packages/ui/src/features/pr-review/useApprovePr.ts b/packages/ui/src/features/pr-review/useApprovePr.ts new file mode 100644 index 0000000000..0d5a71ac7f --- /dev/null +++ b/packages/ui/src/features/pr-review/useApprovePr.ts @@ -0,0 +1,25 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { toast } from "../../primitives/toast"; + +export function useApprovePr(prUrl: string) { + const trpc = useHostTRPC(); + const queryClient = useQueryClient(); + + return useMutation({ + ...trpc.git.approvePr.mutationOptions(), + onSuccess: async (result) => { + if (!result.success) { + toast.error(result.message || "Failed to approve pull request"); + return; + } + toast.success("Pull request approved"); + await queryClient.invalidateQueries( + trpc.git.getPrReviewComments.queryFilter({ prUrl }), + ); + }, + onError: (error) => { + toast.error(error.message || "Failed to approve pull request"); + }, + }); +} diff --git a/packages/ui/src/features/pr-review/useMarkPrReady.ts b/packages/ui/src/features/pr-review/useMarkPrReady.ts new file mode 100644 index 0000000000..393bb93748 --- /dev/null +++ b/packages/ui/src/features/pr-review/useMarkPrReady.ts @@ -0,0 +1,31 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { toast } from "../../primitives/toast"; + +/** Flip a draft PR to "ready for review". */ +export function useMarkPrReady(prUrl: string) { + const trpc = useHostTRPC(); + const queryClient = useQueryClient(); + + return useMutation({ + ...trpc.git.updatePrByUrl.mutationOptions(), + onSuccess: async (result) => { + if (!result.success) { + toast.error(result.message || "Failed to mark as ready for review"); + return; + } + toast.success("Marked as ready for review"); + await Promise.all([ + queryClient.invalidateQueries( + trpc.git.getPrInfoByUrl.queryFilter({ prUrl }), + ), + queryClient.invalidateQueries( + trpc.git.getPrDetailsByUrl.queryFilter({ prUrl }), + ), + ]); + }, + onError: (error) => { + toast.error(error.message || "Failed to mark as ready for review"); + }, + }); +} diff --git a/packages/ui/src/features/pr-review/useMergePr.ts b/packages/ui/src/features/pr-review/useMergePr.ts new file mode 100644 index 0000000000..4742c42556 --- /dev/null +++ b/packages/ui/src/features/pr-review/useMergePr.ts @@ -0,0 +1,34 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { toast } from "../../primitives/toast"; + +export function useMergePr(prUrl: string) { + const trpc = useHostTRPC(); + const queryClient = useQueryClient(); + + return useMutation({ + ...trpc.git.mergePr.mutationOptions(), + onSuccess: async (result) => { + if (!result.success) { + toast.error(result.message || "Failed to merge pull request"); + return; + } + toast.success("Pull request merged"); + // The PR's state (and everything derived from it) just changed. + await Promise.all([ + queryClient.invalidateQueries( + trpc.git.getPrInfoByUrl.queryFilter({ prUrl }), + ), + queryClient.invalidateQueries( + trpc.git.getPrDetailsByUrl.queryFilter({ prUrl }), + ), + queryClient.invalidateQueries( + trpc.git.getPrDiffStatsBatch.pathFilter(), + ), + ]); + }, + onError: (error) => { + toast.error(error.message || "Failed to merge pull request"); + }, + }); +} diff --git a/packages/ui/src/features/pr-review/usePrChecks.ts b/packages/ui/src/features/pr-review/usePrChecks.ts new file mode 100644 index 0000000000..63f2d77fe9 --- /dev/null +++ b/packages/ui/src/features/pr-review/usePrChecks.ts @@ -0,0 +1,19 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useQuery } from "@tanstack/react-query"; + +/** + * CI checks for a PR. Polls while the view is mounted and the window is + * visible (TanStack pauses intervals for hidden documents by default), so a + * running pipeline keeps updating without a manual refresh. + */ +export function usePrChecks(prUrl: string | null) { + const trpc = useHostTRPC(); + return useQuery({ + ...trpc.git.getPrChecks.queryOptions({ prUrl: prUrl as string }), + enabled: !!prUrl, + staleTime: 10_000, + refetchInterval: 15_000, + placeholderData: (prev) => prev, + retry: 1, + }); +} diff --git a/packages/ui/src/features/pr-review/usePrComments.ts b/packages/ui/src/features/pr-review/usePrComments.ts new file mode 100644 index 0000000000..e0ac6ff574 --- /dev/null +++ b/packages/ui/src/features/pr-review/usePrComments.ts @@ -0,0 +1,14 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useQuery } from "@tanstack/react-query"; + +/** Conversation (issue) comments on a PR. */ +export function usePrComments(prUrl: string | null) { + const trpc = useHostTRPC(); + return useQuery({ + ...trpc.git.getPrComments.queryOptions({ prUrl: prUrl as string }), + enabled: !!prUrl, + staleTime: 30_000, + placeholderData: (prev) => prev, + retry: 1, + }); +} diff --git a/packages/ui/src/features/pr-review/usePrInfo.ts b/packages/ui/src/features/pr-review/usePrInfo.ts new file mode 100644 index 0000000000..22b02f15bf --- /dev/null +++ b/packages/ui/src/features/pr-review/usePrInfo.ts @@ -0,0 +1,14 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useQuery } from "@tanstack/react-query"; + +/** Full PR overview (title, body, state, branches, stats) from GitHub. */ +export function usePrInfo(prUrl: string | null) { + const trpc = useHostTRPC(); + return useQuery({ + ...trpc.git.getPrInfoByUrl.queryOptions({ prUrl: prUrl as string }), + enabled: !!prUrl, + staleTime: 30_000, + placeholderData: (prev) => prev, + retry: 1, + }); +} diff --git a/packages/ui/src/features/pr-review/usePrReviewThreads.ts b/packages/ui/src/features/pr-review/usePrReviewThreads.ts new file mode 100644 index 0000000000..f526b37725 --- /dev/null +++ b/packages/ui/src/features/pr-review/usePrReviewThreads.ts @@ -0,0 +1,14 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useQuery } from "@tanstack/react-query"; + +/** Inline review threads (code comments) on a PR. */ +export function usePrReviewThreads(prUrl: string | null) { + const trpc = useHostTRPC(); + return useQuery({ + ...trpc.git.getPrReviewComments.queryOptions({ prUrl: prUrl as string }), + enabled: !!prUrl, + staleTime: 30_000, + placeholderData: (prev) => prev, + retry: 1, + }); +} diff --git a/packages/ui/src/features/pr-review/useReopenPr.ts b/packages/ui/src/features/pr-review/useReopenPr.ts new file mode 100644 index 0000000000..ea53f6b762 --- /dev/null +++ b/packages/ui/src/features/pr-review/useReopenPr.ts @@ -0,0 +1,31 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { toast } from "../../primitives/toast"; + +/** Reopen a closed (not merged) PR. */ +export function useReopenPr(prUrl: string) { + const trpc = useHostTRPC(); + const queryClient = useQueryClient(); + + return useMutation({ + ...trpc.git.updatePrByUrl.mutationOptions(), + onSuccess: async (result) => { + if (!result.success) { + toast.error(result.message || "Failed to reopen pull request"); + return; + } + toast.success("Pull request reopened"); + await Promise.all([ + queryClient.invalidateQueries( + trpc.git.getPrInfoByUrl.queryFilter({ prUrl }), + ), + queryClient.invalidateQueries( + trpc.git.getPrDetailsByUrl.queryFilter({ prUrl }), + ), + ]); + }, + onError: (error) => { + toast.error(error.message || "Failed to reopen pull request"); + }, + }); +} diff --git a/packages/ui/src/features/sidebar/components/items/TaskItem.tsx b/packages/ui/src/features/sidebar/components/items/TaskItem.tsx index fad072d4f6..63f5beaa9e 100644 --- a/packages/ui/src/features/sidebar/components/items/TaskItem.tsx +++ b/packages/ui/src/features/sidebar/components/items/TaskItem.tsx @@ -3,11 +3,11 @@ import { parseGithubUrl } from "@posthog/git/utils"; import type { WorkspaceMode } from "@posthog/shared"; import { formatRelativeTimeShort } from "@posthog/shared"; import type { TaskRunStatus } from "@posthog/shared/domain-types"; +import { navigateToPullRequestView } from "@posthog/ui/router/navigationBridge"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { DotsCircleSpinner } from "../../../../primitives/DotsCircleSpinner"; import { NestedButton } from "../../../../primitives/NestedButton"; import { Tooltip } from "../../../../primitives/Tooltip"; -import { openExternalUrl } from "../../../../shell/openExternal"; import type { SidebarPrState } from "../../useTaskPrStatus"; import { SidebarItem } from "../SidebarItem"; import { ICON_SIZE, TaskIcon } from "./TaskIcon"; @@ -19,7 +19,7 @@ function PrBadge({ url, number }: { url: string; number: number }) { aria-label={`Open pull request #${number}`} className="flex h-4 shrink-0 cursor-pointer items-center gap-0.5 rounded bg-gray-3 px-1 text-[11px] text-gray-11 transition-colors hover:bg-gray-4 hover:text-gray-12" onActivate={() => { - openExternalUrl(url); + navigateToPullRequestView(url); }} > diff --git a/packages/ui/src/router/navigationBridge.ts b/packages/ui/src/router/navigationBridge.ts index f5859e384e..906c83e06b 100644 --- a/packages/ui/src/router/navigationBridge.ts +++ b/packages/ui/src/router/navigationBridge.ts @@ -26,6 +26,13 @@ export function navigateToTaskDetail(taskId: string): void { }); } +export function navigateToPullRequestView(prUrl: string): void { + void getRouterOrNull()?.navigate({ + to: "/code/pr", + search: { prUrl }, + }); +} + export function navigateToTaskPending(key: string): void { void getRouterOrNull()?.navigate({ to: "/code/tasks/pending/$key", diff --git a/packages/ui/src/router/routeTree.gen.ts b/packages/ui/src/router/routeTree.gen.ts index c378c10fa9..c56ae84b79 100644 --- a/packages/ui/src/router/routeTree.gen.ts +++ b/packages/ui/src/router/routeTree.gen.ts @@ -24,6 +24,7 @@ import { Route as WebsiteHomeRouteImport } from './routes/website/home' import { Route as WebsiteCommandCenterRouteImport } from './routes/website/command-center' import { Route as SettingsCategoryRouteImport } from './routes/settings/$category' import { Route as FoldersFolderIdRouteImport } from './routes/folders/$folderId' +import { Route as CodePrRouteImport } from './routes/code/pr' import { Route as CodeInboxRouteImport } from './routes/code/inbox' import { Route as CodeHomeRouteImport } from './routes/code/home' import { Route as CodeArchivedRouteImport } from './routes/code/archived' @@ -149,6 +150,11 @@ const FoldersFolderIdRoute = FoldersFolderIdRouteImport.update({ path: '/folders/$folderId', getParentRoute: () => rootRouteImport, } as any) +const CodePrRoute = CodePrRouteImport.update({ + id: '/code/pr', + path: '/code/pr', + getParentRoute: () => rootRouteImport, +} as any) const CodeInboxRoute = CodeInboxRouteImport.update({ id: '/code/inbox', path: '/code/inbox', @@ -427,6 +433,7 @@ export interface FileRoutesByFullPath { '/code/archived': typeof CodeArchivedRoute '/code/home': typeof CodeHomeRoute '/code/inbox': typeof CodeInboxRouteWithChildren + '/code/pr': typeof CodePrRoute '/folders/$folderId': typeof FoldersFolderIdRoute '/settings/$category': typeof SettingsCategoryRoute '/website/command-center': typeof WebsiteCommandCenterRoute @@ -490,6 +497,7 @@ export interface FileRoutesByTo { '/skills': typeof SkillsRoute '/code/archived': typeof CodeArchivedRoute '/code/home': typeof CodeHomeRoute + '/code/pr': typeof CodePrRoute '/folders/$folderId': typeof FoldersFolderIdRoute '/settings/$category': typeof SettingsCategoryRoute '/website/command-center': typeof WebsiteCommandCenterRoute @@ -549,6 +557,7 @@ export interface FileRoutesById { '/code/archived': typeof CodeArchivedRoute '/code/home': typeof CodeHomeRoute '/code/inbox': typeof CodeInboxRouteWithChildren + '/code/pr': typeof CodePrRoute '/folders/$folderId': typeof FoldersFolderIdRoute '/settings/$category': typeof SettingsCategoryRoute '/website/command-center': typeof WebsiteCommandCenterRoute @@ -617,6 +626,7 @@ export interface FileRouteTypes { | '/code/archived' | '/code/home' | '/code/inbox' + | '/code/pr' | '/folders/$folderId' | '/settings/$category' | '/website/command-center' @@ -680,6 +690,7 @@ export interface FileRouteTypes { | '/skills' | '/code/archived' | '/code/home' + | '/code/pr' | '/folders/$folderId' | '/settings/$category' | '/website/command-center' @@ -738,6 +749,7 @@ export interface FileRouteTypes { | '/code/archived' | '/code/home' | '/code/inbox' + | '/code/pr' | '/folders/$folderId' | '/settings/$category' | '/website/command-center' @@ -805,6 +817,7 @@ export interface RootRouteChildren { CodeArchivedRoute: typeof CodeArchivedRoute CodeHomeRoute: typeof CodeHomeRoute CodeInboxRoute: typeof CodeInboxRouteWithChildren + CodePrRoute: typeof CodePrRoute FoldersFolderIdRoute: typeof FoldersFolderIdRoute SettingsCategoryRoute: typeof SettingsCategoryRoute CodeIndexRoute: typeof CodeIndexRoute @@ -920,6 +933,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof FoldersFolderIdRouteImport parentRoute: typeof rootRouteImport } + '/code/pr': { + id: '/code/pr' + path: '/code/pr' + fullPath: '/code/pr' + preLoaderRoute: typeof CodePrRouteImport + parentRoute: typeof rootRouteImport + } '/code/inbox': { id: '/code/inbox' path: '/code/inbox' @@ -1497,6 +1517,7 @@ const rootRouteChildren: RootRouteChildren = { CodeArchivedRoute: CodeArchivedRoute, CodeHomeRoute: CodeHomeRoute, CodeInboxRoute: CodeInboxRouteWithChildren, + CodePrRoute: CodePrRoute, FoldersFolderIdRoute: FoldersFolderIdRoute, SettingsCategoryRoute: SettingsCategoryRoute, CodeIndexRoute: CodeIndexRoute, diff --git a/packages/ui/src/router/routes/code/pr.tsx b/packages/ui/src/router/routes/code/pr.tsx new file mode 100644 index 0000000000..1a361a29e4 --- /dev/null +++ b/packages/ui/src/router/routes/code/pr.tsx @@ -0,0 +1,14 @@ +import { PullRequestView } from "@posthog/ui/features/pr-review/PullRequestView"; +import { createFileRoute } from "@tanstack/react-router"; + +export const Route = createFileRoute("/code/pr")({ + component: PullRequestRoute, + validateSearch: (search: Record): { prUrl: string } => ({ + prUrl: typeof search.prUrl === "string" ? search.prUrl : "", + }), +}); + +function PullRequestRoute() { + const { prUrl } = Route.useSearch(); + return ; +} diff --git a/packages/workspace-server/src/services/git/schemas.ts b/packages/workspace-server/src/services/git/schemas.ts index 6cc2cd86f9..66941df7eb 100644 --- a/packages/workspace-server/src/services/git/schemas.ts +++ b/packages/workspace-server/src/services/git/schemas.ts @@ -428,6 +428,37 @@ export const updatePrByUrlOutput = z.object({ export type UpdatePrByUrlOutput = z.infer; +export type { + ApprovePrOutput, + GetPrChecksOutput, + GetPrCommentsOutput, + MergePrOutput, + PrCheck, + PrCheckBucket, + PrConversationComment, + PrInfoByUrlOutput, + PrMergeMethod, +} from "@posthog/shared"; +// Native PR review schemas (PR overview, approve/merge, CI checks, +// conversation comments) are defined once in `@posthog/shared`'s git domain +// and re-exported here for the tRPC procedure definitions and GitService. +export { + approvePrInput, + approvePrOutput, + getPrChecksInput, + getPrChecksOutput, + getPrCommentsInput, + getPrCommentsOutput, + getPrInfoByUrlInput, + getPrInfoByUrlOutput, + mergePrInput, + mergePrOutput, + prCheckBucketSchema, + prCheckSchema, + prConversationCommentSchema, + prMergeMethodSchema, +} from "@posthog/shared"; + export const getPrTemplateInput = directoryPathInput; export const getPrTemplateOutput = z.object({ diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index ff94d5a4f7..7fcfd86b02 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -48,12 +48,15 @@ import { TypedEventEmitter } from "@posthog/shared"; import { injectable } from "inversify"; import type { SidebarPrState } from "../workspace/schemas"; import type { + ApprovePrOutput, ChangedFile, CloneProgressPayload, CommitOutput, DetectRepoResult, DiscardFileChangesOutput, GetCommitConventionsOutput, + GetPrChecksOutput, + GetPrCommentsOutput, GetPrTemplateOutput, GhAuthTokenOutput, GhStatusOutput, @@ -66,10 +69,16 @@ import type { GitStatusOutput, GitSyncStatus, HandoffLocalGitState, + MergePrOutput, OpenPrOutput, PrActionType, + PrCheck, + PrCheckBucket, + PrConversationComment, PrDetailsByUrlOutput, PrDiffStats, + PrInfoByUrlOutput, + PrMergeMethod, PrReviewComment, PrReviewThread, PrStatusOutput, @@ -81,6 +90,7 @@ import type { SyncOutput, UpdatePrByUrlOutput, } from "./schemas"; +import { getPrInfoByUrlOutput, prConversationCommentSchema } from "./schemas"; const FETCH_THROTTLE_MS = 30_000; /** Max PRs per GraphQL request – stays well under GitHub's complexity ceiling. */ @@ -135,6 +145,22 @@ function normalizeGraphqlPrState( } } +/** + * Narrow `gh pr checks` bucket values to the schema enum. Unknown values fall + * back to "pending" so one odd bucket can never fail the whole checks list. + */ +function normalizeCheckBucket(bucket: string | undefined): PrCheckBucket { + switch (bucket) { + case "fail": + case "cancel": + case "pass": + case "skipping": + return bucket; + default: + return "pending"; + } +} + export function mapPrState( state: string | null, merged: boolean, @@ -965,6 +991,30 @@ export class GitService extends TypedEventEmitter { } } + async getPrInfoByUrl(prUrl: string): Promise { + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") return null; + + try { + const result = await execGh([ + "api", + `repos/${pr.owner}/${pr.repo}/pulls/${pr.number}`, + "--jq", + '{number,title,body: (.body // ""),author: .user.login,state,merged,draft,mergeable,mergeStateStatus: (.mergeable_state // "unknown"),baseRefName: .base.ref,headRefName: .head.ref,additions,deletions,changedFiles: .changed_files}', + ]); + + if (result.exitCode !== 0) { + return null; + } + + // Zod-parse rather than cast, so a GitHub response-shape change + // surfaces here (caught, -> null) instead of leaking bad data. + return getPrInfoByUrlOutput.parse(JSON.parse(result.stdout)); + } catch { + return null; + } + } + async getPrChangedFiles(prUrl: string): Promise { const pr = parseGithubUrl(prUrl); if (pr?.kind !== "pr") return []; @@ -1284,6 +1334,161 @@ export class GitService extends TypedEventEmitter { } } + async approvePr(prUrl: string): Promise { + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") { + return { success: false, message: "Invalid PR URL" }; + } + + try { + const result = await execGh([ + "pr", + "review", + String(pr.number), + "--approve", + "--repo", + `${pr.owner}/${pr.repo}`, + ]); + + if (result.exitCode !== 0) { + return { + success: false, + message: result.stderr || result.error || "Unknown error", + }; + } + + return { success: true, message: result.stdout }; + } catch (error) { + return { + success: false, + message: error instanceof Error ? error.message : "Unknown error", + }; + } + } + + async mergePr(prUrl: string, method: PrMergeMethod): Promise { + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") { + return { success: false, message: "Invalid PR URL" }; + } + + try { + const result = await execGh([ + "pr", + "merge", + String(pr.number), + `--${method}`, + "--repo", + `${pr.owner}/${pr.repo}`, + ]); + + if (result.exitCode !== 0) { + return { + success: false, + message: result.stderr || result.error || "Unknown error", + }; + } + + return { success: true, message: result.stdout }; + } catch (error) { + return { + success: false, + message: error instanceof Error ? error.message : "Unknown error", + }; + } + } + + async getPrChecks(prUrl: string): Promise { + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") return null; + + try { + // `gh pr checks` exits non-zero when checks are failing (1) or pending + // (8), so the exit code alone doesn't distinguish "couldn't fetch" from + // "fetched, some red" — parse stdout instead. + const result = await execGh([ + "pr", + "checks", + String(pr.number), + "--repo", + `${pr.owner}/${pr.repo}`, + "--json", + "name,bucket,link,workflow,description", + ]); + + if (result.stdout.trim()) { + const checks = JSON.parse(result.stdout) as Array<{ + name?: string; + bucket?: string; + link?: string; + workflow?: string; + description?: string; + }>; + return checks.map( + (check): PrCheck => ({ + name: check.name ?? "", + bucket: normalizeCheckBucket(check.bucket), + link: check.link || null, + workflow: check.workflow || null, + description: check.description || null, + }), + ); + } + + if (result.exitCode === 0) return []; + // A PR with no CI configured is not an error state. + if ((result.stderr ?? "").includes("no checks reported")) return []; + return null; + } catch { + return null; + } + } + + async getPrComments(prUrl: string): Promise { + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") return null; + + // GitHub's conversation tab is two feeds: issue comments and review + // summaries ("approved with a comment"). Inline code comments come from + // getPrReviewComments separately. + const [comments, reviewSummaries] = await Promise.all([ + this.fetchPrCommentFeed( + `repos/${pr.owner}/${pr.repo}/issues/${pr.number}/comments`, + '.[] | {id, author: (.user.login // "unknown"), avatarUrl: (.user.avatar_url // null), body: (.body // ""), createdAt: .created_at, url: (.html_url // null)}', + ), + this.fetchPrCommentFeed( + `repos/${pr.owner}/${pr.repo}/pulls/${pr.number}/reviews`, + '.[] | select(.state != "PENDING") | select((.body // "") != "") | {id, author: (.user.login // "unknown"), avatarUrl: (.user.avatar_url // null), body, createdAt: (.submitted_at // ""), url: (.html_url // null)}', + ), + ]); + return [...comments, ...reviewSummaries]; + } + + /** + * Fetch a paginated comment-shaped feed, slimmed to the schema fields in + * gh's jq (full comment objects are mostly boilerplate URLs and can blow + * past the exec buffer on busy PRs). `--jq` with `--paginate` emits one + * compact JSON object per line. Throws on failure so the renderer can show + * why (rate limit, auth, network) instead of a silent empty section. + */ + private async fetchPrCommentFeed( + endpoint: string, + jq: string, + ): Promise { + const result = await execGh(["api", endpoint, "--paginate", "--jq", jq]); + + if (result.exitCode !== 0) { + throw new Error( + `Failed to fetch PR comments: ${result.stderr || result.error || "Unknown error"}`, + ); + } + + return result.stdout + .split("\n") + .filter((line) => line.trim() !== "") + .map((line) => prConversationCommentSchema.parse(JSON.parse(line))); + } + async getPrReviewComments(prUrl: string): Promise { const pr = parseGithubUrl(prUrl); if (pr?.kind !== "pr") return []; diff --git a/packages/workspace-server/src/trpc.ts b/packages/workspace-server/src/trpc.ts index 96353907d4..ed12f56a96 100644 --- a/packages/workspace-server/src/trpc.ts +++ b/packages/workspace-server/src/trpc.ts @@ -45,6 +45,8 @@ import { } from "./services/fs/schemas"; import type { FsService } from "./services/fs/service"; import { + approvePrInput, + approvePrOutput, changedFilesOutput, checkoutBranchInput, checkoutBranchOutput, @@ -79,10 +81,16 @@ import { getHeadShaOutput, getLocalBranchChangedFilesInput, getPrChangedFilesInput, + getPrChecksInput, + getPrChecksOutput, + getPrCommentsInput, + getPrCommentsOutput, getPrDetailsByUrlInput, getPrDetailsByUrlOutput, getPrDiffStatsBatchInput, getPrDiffStatsBatchOutput, + getPrInfoByUrlInput, + getPrInfoByUrlOutput, getPrReviewCommentsInput, getPrReviewCommentsOutput, getPrTemplateInput, @@ -100,6 +108,8 @@ import { syncInput as gitSyncInput, syncOutput as gitSyncOutput, gitSyncStatusSchema, + mergePrInput, + mergePrOutput, openPrInput, openPrOutput, prStatusOutput, @@ -559,6 +569,21 @@ export function createAppRouter({ .output(getPrDetailsByUrlOutput.nullable()) .query(({ input }) => gitService().getPrDetailsByUrl(input.prUrl)), + getPrInfoByUrl: t.procedure + .input(getPrInfoByUrlInput) + .output(getPrInfoByUrlOutput.nullable()) + .query(({ input }) => gitService().getPrInfoByUrl(input.prUrl)), + + getPrChecks: t.procedure + .input(getPrChecksInput) + .output(getPrChecksOutput) + .query(({ input }) => gitService().getPrChecks(input.prUrl)), + + getPrComments: t.procedure + .input(getPrCommentsInput) + .output(getPrCommentsOutput) + .query(({ input }) => gitService().getPrComments(input.prUrl)), + getPrChangedFiles: t.procedure .input(getPrChangedFilesInput) .output(changedFilesOutput) @@ -593,6 +618,18 @@ export function createAppRouter({ gitService().updatePrByUrl(input.prUrl, input.action), ), + approvePr: t.procedure + .input(approvePrInput) + .output(approvePrOutput) + .mutation(({ input }) => gitService().approvePr(input.prUrl)), + + mergePr: t.procedure + .input(mergePrInput) + .output(mergePrOutput) + .mutation(({ input }) => + gitService().mergePr(input.prUrl, input.method), + ), + getPrReviewComments: t.procedure .input(getPrReviewCommentsInput) .output(getPrReviewCommentsOutput)