From bd161b5bf453108e6039798ba13db1205f58bc10 Mon Sep 17 00:00:00 2001 From: Andrew Maguire Date: Tue, 16 Jun 2026 10:21:32 +0100 Subject: [PATCH 1/3] feat(inbox): show live PR status on pull request list cards Extend the batched diff-stats GraphQL request to also fetch each PR's state/isDraft, so list cards render live status (open/draft/merged/closed) with no extra API calls. ReportImplementationPrLink now reads status from the batch context when present and only falls back to the per-PR usePrDetails query on the standalone detail view. --- packages/core/src/git/router-schemas.ts | 8 ++++++ .../inbox/components/PullRequestCard.tsx | 15 ++++++++--- .../utils/ReportImplementationPrLink.tsx | 27 ++++++++++++++++--- .../src/services/git/schemas.ts | 8 ++++++ .../src/services/git/service.ts | 9 ++++++- 5 files changed, 59 insertions(+), 8 deletions(-) diff --git a/packages/core/src/git/router-schemas.ts b/packages/core/src/git/router-schemas.ts index 7feadcce0..264fe7a0d 100644 --- a/packages/core/src/git/router-schemas.ts +++ b/packages/core/src/git/router-schemas.ts @@ -352,10 +352,18 @@ export const getPrChangedFilesInput = z.object({ export const getPrChangedFilesOutput = z.array(changedFileSchema); // getPrDiffStatsBatch schemas +// +// Beyond the diff numbers, the batch also carries the PR's live status +// (`state`/`merged`/`draft`) so list cards can render it from the single +// batched GraphQL request instead of firing one REST call per visible PR. export const prDiffStatsSchema = z.object({ additions: z.number(), deletions: z.number(), changedFiles: z.number(), + /** Lowercased GitHub PR state: "open" | "closed" | "merged". */ + state: z.string(), + merged: z.boolean(), + draft: z.boolean(), }); export type PrDiffStats = z.infer; diff --git a/packages/ui/src/features/inbox/components/PullRequestCard.tsx b/packages/ui/src/features/inbox/components/PullRequestCard.tsx index e0efc16b9..485738a66 100644 --- a/packages/ui/src/features/inbox/components/PullRequestCard.tsx +++ b/packages/ui/src/features/inbox/components/PullRequestCard.tsx @@ -14,6 +14,7 @@ import { InboxCardTitle } from "@posthog/ui/features/inbox/components/InboxCardT import { PrDiffStats } from "@posthog/ui/features/inbox/components/PrDiffStats"; import { PriorityMonogram } from "@posthog/ui/features/inbox/components/PriorityMonogram"; import { SuggestedReviewerAvatarStack } from "@posthog/ui/features/inbox/components/SuggestedReviewerAvatarStack"; +import { ReportImplementationPrLink } from "@posthog/ui/features/inbox/components/utils/ReportImplementationPrLink"; import { useInboxReportDetailPrefetch } from "@posthog/ui/features/inbox/hooks/useInboxReportDetailPrefetch"; import { useInboxReportArtefacts } from "@posthog/ui/features/inbox/hooks/useInboxReports"; import { Button as UiButton } from "@posthog/ui/primitives/Button"; @@ -121,10 +122,16 @@ export function PullRequestCard({ > {report.implementation_pr_url && ( - + <> + + + )} ; export const getPrChangedFilesInput = z.object({ prUrl: z.string() }); +// Also carries the PR's live status (`state`/`merged`/`draft`) so list cards +// can render it from the single batched GraphQL request instead of firing one +// REST call per visible PR. Mirrors `prDiffStatsSchema` in +// `@posthog/core/git/router-schemas`. export const prDiffStatsSchema = z.object({ additions: z.number(), deletions: z.number(), changedFiles: z.number(), + /** Lowercased GitHub PR state: "open" | "closed" | "merged". */ + state: z.string(), + merged: z.boolean(), + draft: z.boolean(), }); export type PrDiffStats = z.infer; diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index 11ba8e535..8900d1e2c 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -1054,7 +1054,7 @@ export class GitService extends TypedEventEmitter { ): Promise> { const aliasFragments = chunk .map(([, { parsed }], index) => { - return `pr${index}: repository(owner: "${escapeGraphqlString(parsed.owner)}", name: "${escapeGraphqlString(parsed.repo)}") { pullRequest(number: ${parsed.number}) { additions deletions changedFiles } }`; + return `pr${index}: repository(owner: "${escapeGraphqlString(parsed.owner)}", name: "${escapeGraphqlString(parsed.repo)}") { pullRequest(number: ${parsed.number}) { additions deletions changedFiles state isDraft } }`; }) .join("\n"); const query = `query InboxPrDiffStatsBatch {\n${aliasFragments}\n}`; @@ -1075,6 +1075,8 @@ export class GitService extends TypedEventEmitter { additions: number; deletions: number; changedFiles: number; + state: string; + isDraft: boolean; } | null; } | null >; @@ -1085,10 +1087,15 @@ export class GitService extends TypedEventEmitter { const [, { urls }] = chunk[i]; const node = parsed.data?.[`pr${i}`]?.pullRequest; if (!node) continue; + // GraphQL `PullRequestState` is OPEN | CLOSED | MERGED; normalise to the + // lowercase state + merged boolean shape the badge expects. const stats: PrDiffStats = { additions: node.additions, deletions: node.deletions, changedFiles: node.changedFiles, + state: node.state.toLowerCase(), + merged: node.state === "MERGED", + draft: node.isDraft, }; for (const url of urls) { out[url] = stats; From 23c9e0d4f575f3fc96a678fd30883778dac90644 Mon Sep 17 00:00:00 2001 From: Andrew Maguire Date: Tue, 16 Jun 2026 10:35:25 +0100 Subject: [PATCH 2/3] fix(inbox): address review on PR status badge + stack card layout - Narrow PR state to a z.enum(['open','closed','merged']) in both schema copies so unexpected GraphQL values are caught at the boundary; the workspace-server maps GitHub's enum with a safe 'open' fallback. - Harden ReportImplementationPrLink: validate the URL via parsePrUrl (no unvalidated external link), render nothing for unknown/unresolved state instead of a misleading green 'open', and guard the tooltip with isLoading. - Stack the status badge above the line-diff on the card to use horizontal space more efficiently. --- packages/core/src/git/router-schemas.ts | 4 +- .../inbox/components/PullRequestCard.tsx | 4 +- .../utils/ReportImplementationPrLink.tsx | 64 ++++++++----------- .../src/services/git/schemas.ts | 4 +- .../src/services/git/service.ts | 20 +++++- 5 files changed, 52 insertions(+), 44 deletions(-) diff --git a/packages/core/src/git/router-schemas.ts b/packages/core/src/git/router-schemas.ts index 264fe7a0d..dc9e03780 100644 --- a/packages/core/src/git/router-schemas.ts +++ b/packages/core/src/git/router-schemas.ts @@ -360,8 +360,8 @@ export const prDiffStatsSchema = z.object({ additions: z.number(), deletions: z.number(), changedFiles: z.number(), - /** Lowercased GitHub PR state: "open" | "closed" | "merged". */ - state: z.string(), + /** Lowercased GitHub PR state. */ + state: z.enum(["open", "closed", "merged"]), merged: z.boolean(), draft: z.boolean(), }); diff --git a/packages/ui/src/features/inbox/components/PullRequestCard.tsx b/packages/ui/src/features/inbox/components/PullRequestCard.tsx index 485738a66..8a1c21cf0 100644 --- a/packages/ui/src/features/inbox/components/PullRequestCard.tsx +++ b/packages/ui/src/features/inbox/components/PullRequestCard.tsx @@ -122,7 +122,7 @@ export function PullRequestCard({ > {report.implementation_pr_url && ( - <> + - + )} void; } -function parseGitHubPrReference(prUrl: string): { - reference: string; - prNumber: string; -} { - try { - const parsed = new URL(prUrl); - const match = parsed.pathname.match( - /^\/([^/]+)\/([^/]+)\/pull\/(\d+)(?:$|[/?#])/, - ); - if (match) { - return { - reference: `${match[1]}/${match[2]}#${match[3]}`, - prNumber: `#${match[3]}`, - }; - } - } catch { - // Fall through to regex fallback for non-URL-safe inputs - } - - const prMatch = prUrl.match(/\/pull\/(\d+)(?:$|[/?#])/); - const prNumber = prMatch ? `#${prMatch[1]}` : "PR"; - return { - reference: prUrl, - prNumber, - }; -} +/** The only states we render a badge for; anything else is treated as unknown. */ +const KNOWN_PR_STATES = new Set(["open", "closed", "merged"]); export function ReportImplementationPrLink({ prUrl, @@ -66,9 +43,19 @@ export function ReportImplementationPrLink({ ? batchEntry.isLoading && !batchEntry.stats : fallback.meta.isLoading; - // If neither source could resolve a status (PR 404s, gh failed, or the batch - // had no entry for this PR), don't render a misleading "open" badge. - if (!isLoading && state === null) return null; + // Only render for a canonical GitHub PR URL. This both keeps the badge in + // sync with the gated "Open in GitHub" action and avoids turning an arbitrary + // (possibly unsafe-scheme) string into a clickable external link. + const prRef = parsePrUrl(prUrl); + if (!prRef) return null; + + // `getPrDetailsByUrl` falls back to `{ state: "unknown" }` when the lookup + // fails (gh offline, private repo, unparseable URL), and a batch miss leaves + // state null. Once settled, render nothing for an unresolved state rather + // than a misleading green "open" badge. + if (!isLoading && (state === null || !KNOWN_PR_STATES.has(state))) { + return null; + } const isSm = size === "sm"; @@ -84,15 +71,18 @@ export function ReportImplementationPrLink({ ? "bg-gray-4 text-gray-11 hover:bg-gray-5" : "bg-green-4 text-green-11 hover:bg-green-5"; - const { reference: prReference, prNumber } = parseGitHubPrReference(prUrl); + const prReference = `${prRef.repoSlug}#${prRef.number}`; + const prNumber = `#${prRef.number}`; - const tooltip = merged - ? `Merged – ${prReference}` - : state === "closed" - ? `Closed – ${prReference}` - : draft - ? `Draft – ${prReference}` - : `Open – ${prReference}`; + const tooltip = isLoading + ? prReference + : merged + ? `Merged – ${prReference}` + : state === "closed" + ? `Closed – ${prReference}` + : draft + ? `Draft – ${prReference}` + : `Open – ${prReference}`; const iconSize = isSm ? 10 : 12; diff --git a/packages/workspace-server/src/services/git/schemas.ts b/packages/workspace-server/src/services/git/schemas.ts index f7e02a438..d39c56f43 100644 --- a/packages/workspace-server/src/services/git/schemas.ts +++ b/packages/workspace-server/src/services/git/schemas.ts @@ -316,8 +316,8 @@ export const prDiffStatsSchema = z.object({ additions: z.number(), deletions: z.number(), changedFiles: z.number(), - /** Lowercased GitHub PR state: "open" | "closed" | "merged". */ - state: z.string(), + /** Lowercased GitHub PR state. */ + state: z.enum(["open", "closed", "merged"]), merged: z.boolean(), draft: z.boolean(), }); diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index 8900d1e2c..211839469 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -117,6 +117,24 @@ function toUnifiedDiffPatch( return `diff --git a/${oldPath} b/${filename}\n--- ${fromPath}\n+++ ${toPath}\n${rawPatch}`; } +/** + * Narrow GitHub GraphQL's `PullRequestState` (OPEN | CLOSED | MERGED) to the + * lowercased literal the batch schema expects. Anything unexpected falls back + * to "open" so one odd value can never fail validation for the whole batch. + */ +function normalizeGraphqlPrState( + graphqlState: string, +): "open" | "closed" | "merged" { + switch (graphqlState) { + case "MERGED": + return "merged"; + case "CLOSED": + return "closed"; + default: + return "open"; + } +} + export function mapPrState( state: string | null, merged: boolean, @@ -1093,7 +1111,7 @@ export class GitService extends TypedEventEmitter { additions: node.additions, deletions: node.deletions, changedFiles: node.changedFiles, - state: node.state.toLowerCase(), + state: normalizeGraphqlPrState(node.state), merged: node.state === "MERGED", draft: node.isDraft, }; From eec066185b9b7114b83cc140d4c852c22a7ad6ff Mon Sep 17 00:00:00 2001 From: Andrew Maguire Date: Tue, 16 Jun 2026 10:42:11 +0100 Subject: [PATCH 3/3] fix(inbox): refresh PR status batch after lifecycle actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Pulls list reads status from the batched getPrDiffStatsBatch query, which usePrActions wasn't invalidating — only the per-PR getPrDetailsByUrl cache. Invalidate the batch on a successful PR action so the list badge doesn't show stale status until the 5-min cache expires. --- packages/ui/src/features/git-interaction/usePrActions.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/ui/src/features/git-interaction/usePrActions.ts b/packages/ui/src/features/git-interaction/usePrActions.ts index e26a248bd..bcb9fbab6 100644 --- a/packages/ui/src/features/git-interaction/usePrActions.ts +++ b/packages/ui/src/features/git-interaction/usePrActions.ts @@ -20,6 +20,13 @@ export function usePrActions(prUrl: string | null) { trpc.git.getPrDetailsByUrl.queryKey({ prUrl: variables.prUrl }), getOptimisticPrState(variables.action), ); + // The inbox Pulls list reads PR status from the batched + // `getPrDiffStatsBatch` query (separate cache, 5-min staleTime), so + // patching the detail cache above isn't enough — invalidate the batch + // so the list badge reflects the new state on next view. + void queryClient.invalidateQueries( + trpc.git.getPrDiffStatsBatch.pathFilter(), + ); } else { toast.error("Failed to update PR", { description: data.message }); }