diff --git a/packages/app/src/components/explorer-sidebar.tsx b/packages/app/src/components/explorer-sidebar.tsx index 99281e6a3..901999a35 100644 --- a/packages/app/src/components/explorer-sidebar.tsx +++ b/packages/app/src/components/explorer-sidebar.tsx @@ -12,6 +12,9 @@ import Animated, { useAnimatedStyle, useSharedValue, runOnJS } from "react-nativ import { Gesture, GestureDetector } from "react-native-gesture-handler"; import { StyleSheet, useUnistyles } from "react-native-unistyles"; import { X } from "lucide-react-native"; +import { GitHubIcon } from "@/components/icons/github-icon"; +import { PrPane } from "./pr-pane"; +import { usePrPaneData } from "@/hooks/use-pr-pane-data"; import { usePanelStore, MIN_EXPLORER_SIDEBAR_WIDTH, @@ -353,7 +356,19 @@ function SidebarContent({ }: SidebarContentProps) { const { theme } = useUnistyles(); const padding = useWindowControlsPadding("explorerSidebar"); - const resolvedTab: ExplorerTab = !isGit && activeTab === "changes" ? "files" : activeTab; + const canQueryPullRequest = isGit && Boolean(workspaceRoot); + const prPane = usePrPaneData({ + serverId, + cwd: workspaceRoot, + enabled: canQueryPullRequest, + timelineEnabled: activeTab === "pr" && canQueryPullRequest, + }); + const hasPullRequest = prPane.prNumber !== null; + const requestedTab: ExplorerTab = + !isGit && (activeTab === "changes" || activeTab === "pr") ? "files" : activeTab; + const resolvedTab: ExplorerTab = + requestedTab === "pr" && !hasPullRequest ? "changes" : requestedTab; + const prTabLabel = prPane.prNumber === null ? "" : `#${prPane.prNumber}`; return ( @@ -381,6 +396,23 @@ function SidebarContent({ Files + {isGit && hasPullRequest && ( + onTabPress("pr")} + > + + + {prTabLabel} + + + )} {isMobile && ( @@ -409,6 +441,7 @@ function SidebarContent({ onOpenFile={onOpenFile} /> )} + {resolvedTab === "pr" && prPane.data && } ); diff --git a/packages/app/src/components/git-actions-split-button.tsx b/packages/app/src/components/git-actions-split-button.tsx index 234d789d3..4bc6cd8cb 100644 --- a/packages/app/src/components/git-actions-split-button.tsx +++ b/packages/app/src/components/git-actions-split-button.tsx @@ -19,22 +19,16 @@ interface GitActionsSplitButtonProps { hideLabels?: boolean; } -export function GitActionsSplitButton({ - gitActions, - hideLabels, -}: GitActionsSplitButtonProps) { +export function GitActionsSplitButton({ gitActions, hideLabels }: GitActionsSplitButtonProps) { const { theme } = useUnistyles(); const toast = useToast(); const archiveShortcutKeys = useShortcutKeys("archive-worktree"); - const getActionDisplayLabel = useCallback( - (action: GitAction): string => { - if (action.status === "pending") return action.pendingLabel; - if (action.status === "success") return action.successLabel; - return action.label; - }, - [], - ); + const getActionDisplayLabel = useCallback((action: GitAction): string => { + if (action.status === "pending") return action.pendingLabel; + if (action.status === "success") return action.successLabel; + return action.label; + }, []); const handleActionSelect = useCallback( (action: GitAction) => { diff --git a/packages/app/src/components/pr-pane.test.tsx b/packages/app/src/components/pr-pane.test.tsx new file mode 100644 index 000000000..dc2908905 --- /dev/null +++ b/packages/app/src/components/pr-pane.test.tsx @@ -0,0 +1,169 @@ +import React from "react"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { JSDOM } from "jsdom"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { PrPaneData } from "@/utils/pr-pane-data"; +import { PrPane } from "./pr-pane"; + +const { theme } = vi.hoisted(() => ({ + theme: { + spacing: { 1: 4, 2: 8, 3: 12, 4: 16 }, + fontSize: { xs: 11, sm: 13, base: 15 }, + fontWeight: { normal: "400" }, + colors: { + surfaceSidebar: "#0b0b0b", + surfaceSidebarHover: "#151515", + border: "#222", + foreground: "#fff", + foregroundMuted: "#aaa", + statusSuccess: "#30d158", + statusDanger: "#ff453a", + statusWarning: "#f2c945", + statusMerged: "#a371f7", + }, + }, +})); + +vi.mock("react-native-unistyles", () => ({ + StyleSheet: { + create: (factory: unknown) => (typeof factory === "function" ? factory(theme) : factory), + }, + useUnistyles: () => ({ theme }), +})); + +vi.mock("lucide-react-native", () => { + const createIcon = (name: string) => (props: Record) => + React.createElement("span", { ...props, "data-icon": name }); + return { + ChevronDown: createIcon("ChevronDown"), + ChevronRight: createIcon("ChevronRight"), + CircleCheck: createIcon("CircleCheck"), + CircleDot: createIcon("CircleDot"), + CircleSlash: createIcon("CircleSlash"), + CircleX: createIcon("CircleX"), + ExternalLink: createIcon("ExternalLink"), + GitMerge: createIcon("GitMerge"), + GitPullRequest: createIcon("GitPullRequest"), + GitPullRequestClosed: createIcon("GitPullRequestClosed"), + GitPullRequestDraft: createIcon("GitPullRequestDraft"), + MessageSquare: createIcon("MessageSquare"), + }; +}); + +vi.mock("@/utils/open-external-url", () => ({ + openExternalUrl: vi.fn(), +})); + +let root: Root | null = null; +let container: HTMLElement | null = null; + +beforeEach(() => { + const dom = new JSDOM(""); + vi.stubGlobal("React", React); + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("window", dom.window); + vi.stubGlobal("document", dom.window.document); + vi.stubGlobal("HTMLElement", dom.window.HTMLElement); + vi.stubGlobal("Node", dom.window.Node); + vi.stubGlobal("navigator", dom.window.navigator); + + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + if (root) { + act(() => { + root?.unmount(); + }); + } + root = null; + container = null; + vi.unstubAllGlobals(); +}); + +function makeData(overrides: Partial = {}): PrPaneData { + return { + number: 1284, + title: "fix(server,app): coalesce tool call stream events", + state: "open", + url: "https://github.com/getpaseo/paseo/pull/1284", + reviewDecision: "changes_requested", + awaitingReviewers: [], + checks: [ + { name: "typecheck", workflow: "CI", status: "success", duration: "1m", url: "#" }, + { name: "test", workflow: "CI", status: "failure", duration: "2m", url: "#" }, + ], + activity: [ + { + kind: "review", + author: "alicek", + avatarColor: "#8b5cf6", + reviewState: "approved", + body: "LGTM", + age: "2h ago", + url: "#", + }, + { + kind: "comment", + author: "bmartin", + avatarColor: "#f97316", + body: "Worth a benchmark before/after.", + age: "3h ago", + url: "#", + }, + ], + ...overrides, + }; +} + +function render(data: PrPaneData) { + act(() => { + root?.render(); + }); +} + +describe("PrPane", () => { + it("renders the PR title and 'Open' state label from the data prop", () => { + render(makeData()); + + expect(container?.textContent).toContain("fix(server,app): coalesce tool call stream events"); + expect(container?.textContent).toContain("Open"); + }); + + it("renders the correct state label and icon for each PR state", () => { + render(makeData({ state: "draft" })); + expect(container?.textContent).toContain("Draft"); + expect(container?.querySelector('[data-icon="GitPullRequestDraft"]')).not.toBeNull(); + + render(makeData({ state: "merged" })); + expect(container?.textContent).toContain("Merged"); + expect(container?.querySelector('[data-icon="GitMerge"]')).not.toBeNull(); + + render(makeData({ state: "closed" })); + expect(container?.textContent).toContain("Closed"); + expect(container?.querySelector('[data-icon="GitPullRequestClosed"]')).not.toBeNull(); + }); + + it("derives check summary counts from the data prop", () => { + render(makeData()); + const text = container?.textContent ?? ""; + + expect(text).toContain("typecheck"); + expect(text).toContain("test"); + // 1 passed, 1 failed, 0 pending — so pending pill is hidden. + expect(text).toMatch(/1.*1/); + }); + + it("derives review summary counts and renders activity rows", () => { + render(makeData()); + const text = container?.textContent ?? ""; + + expect(text).toContain("alicek"); + expect(text).toContain("Approved"); + expect(text).toContain("bmartin"); + expect(text).toContain("Commented"); + }); +}); diff --git a/packages/app/src/components/pr-pane.tsx b/packages/app/src/components/pr-pane.tsx new file mode 100644 index 000000000..c3e7c3718 --- /dev/null +++ b/packages/app/src/components/pr-pane.tsx @@ -0,0 +1,397 @@ +import { useState } from "react"; +import { Pressable, ScrollView, Text, View } from "react-native"; +import { StyleSheet, useUnistyles } from "react-native-unistyles"; +import { + ChevronDown, + ChevronRight, + CircleCheck, + CircleDot, + CircleSlash, + CircleX, + ExternalLink, + GitMerge, + GitPullRequest, + GitPullRequestClosed, + GitPullRequestDraft, + MessageSquare, +} from "lucide-react-native"; +import { openExternalUrl } from "@/utils/open-external-url"; +import type { + CheckStatus, + PrPaneActivity, + PrPaneCheck, + PrPaneData, + PrState, +} from "@/utils/pr-pane-data"; + +export function PrPane({ data }: { data: PrPaneData }) { + const { theme } = useUnistyles(); + const [checksOpen, setChecksOpen] = useState(true); + const [reviewsOpen, setReviewsOpen] = useState(true); + + const passed = data.checks.filter((c) => c.status === "success").length; + const failed = data.checks.filter((c) => c.status === "failure").length; + const pending = data.checks.filter((c) => c.status === "pending").length; + + const approvals = data.activity.filter( + (a) => a.kind === "review" && a.reviewState === "approved", + ).length; + const changesRequested = data.activity.filter( + (a) => a.kind === "review" && a.reviewState === "changes_requested", + ).length; + const commentCount = data.activity.filter( + (a) => a.kind === "comment" || (a.kind === "review" && a.reviewState === "commented"), + ).length; + + const stateColor = getStateColor(data.state, theme); + const StateIcon = getStateIcon(data.state); + const stateLabel = getStateLabel(data.state); + + return ( + + void openExternalUrl(data.url)} style={styles.header}> + {({ hovered }) => ( + <> + + + {stateLabel} + + + {data.title} + {hovered ? ( + + {" "} + + + ) : null} + + + )} + + + + +
setChecksOpen((o) => !o)} + summary={ + <> + } + /> + } + /> + } + /> + + } + > + {data.checks.map((check, idx) => ( + + ))} +
+ + + +
setReviewsOpen((o) => !o)} + summary={ + <> + } + /> + } + /> + } + /> + + } + > + {data.activity.map((item, idx) => ( + + ))} +
+ + ); +} + +interface SectionProps { + title: string; + open: boolean; + onToggle: () => void; + summary: React.ReactNode; + children: React.ReactNode; +} + +function Section({ title, open, onToggle, summary, children }: SectionProps) { + const { theme } = useUnistyles(); + return ( + + + {open ? ( + + ) : ( + + )} + {title} + {summary} + + {open && ( + + {children} + + )} + + ); +} + +function SummaryPill({ + count, + color, + icon, +}: { + count: number; + color: string; + icon: React.ReactNode; +}) { + if (count === 0) return null; + return ( + + {icon} + {count} + + ); +} + +function CheckRow({ check }: { check: PrPaneCheck }) { + return ( + void openExternalUrl(check.url)} + style={({ hovered }) => [styles.row, hovered && styles.hoverable]} + > + + + {check.name} + + {check.workflow && ( + + {check.workflow} + + )} + {check.duration && {check.duration}} + + ); +} + +function CheckStatusIcon({ status }: { status: CheckStatus }) { + const { theme } = useUnistyles(); + if (status === "success") return ; + if (status === "failure") return ; + if (status === "pending") return ; + return ; +} + +function ActivityRow({ item }: { item: PrPaneActivity }) { + const verb = getActivityVerb(item); + return ( + void openExternalUrl(item.url)} + style={({ hovered }) => [styles.activityRow, hovered && styles.hoverable]} + > + + {item.author.slice(0, 1).toUpperCase()} + + + + + {item.author} + + {verb} + {item.age} + + + {item.body} + + + + ); +} + +function getActivityVerb(item: PrPaneActivity): string { + if (item.kind === "comment") return "Commented"; + if (item.reviewState === "approved") return "Approved"; + if (item.reviewState === "changes_requested") return "Requested changes"; + return "Reviewed"; +} + +function getStateColor(state: PrState, theme: ReturnType["theme"]): string { + if (state === "open") return theme.colors.statusSuccess; + if (state === "draft") return theme.colors.foregroundMuted; + if (state === "merged") return theme.colors.statusMerged; + return theme.colors.statusDanger; +} + +function getStateIcon(state: PrState) { + if (state === "draft") return GitPullRequestDraft; + if (state === "merged") return GitMerge; + if (state === "closed") return GitPullRequestClosed; + return GitPullRequest; +} + +function getStateLabel(state: PrState): string { + if (state === "draft") return "Draft"; + if (state === "merged") return "Merged"; + if (state === "closed") return "Closed"; + return "Open"; +} + +const styles = StyleSheet.create((theme) => ({ + root: { + flex: 1, + minHeight: 0, + backgroundColor: theme.colors.surfaceSidebar, + }, + hoverable: { + backgroundColor: theme.colors.surfaceSidebarHover, + }, + header: { + flexDirection: "column", + gap: theme.spacing[1], + paddingHorizontal: theme.spacing[4], + paddingVertical: theme.spacing[4], + }, + stateLine: { + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[1], + }, + stateLabel: { + fontSize: theme.fontSize.xs, + fontWeight: theme.fontWeight.normal, + }, + title: { + fontSize: theme.fontSize.sm, + fontWeight: theme.fontWeight.normal, + color: theme.colors.foreground, + lineHeight: 19, + }, + divider: { + height: 1, + backgroundColor: theme.colors.border, + }, + sectionOpen: { + flexShrink: 1, + minHeight: 0, + }, + sectionBody: { + flexShrink: 1, + minHeight: 0, + }, + sectionBodyContent: { + paddingBottom: theme.spacing[3], + }, + sectionHeader: { + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[2], + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[2], + }, + sectionTitle: { + fontSize: theme.fontSize.sm, + fontWeight: theme.fontWeight.normal, + color: theme.colors.foregroundMuted, + }, + summaryWrap: { + marginLeft: "auto", + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[2], + }, + summaryPill: { + flexDirection: "row", + alignItems: "center", + gap: 3, + }, + summaryPillText: { + fontSize: theme.fontSize.xs, + fontWeight: theme.fontWeight.normal, + }, + row: { + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[2], + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[2], + }, + activityRow: { + flexDirection: "row", + alignItems: "flex-start", + gap: theme.spacing[2], + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[2], + }, + activityMain: { flex: 1, minWidth: 0, gap: 2 }, + activityHeader: { + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[2], + }, + rowTitle: { + fontSize: theme.fontSize.sm, + fontWeight: theme.fontWeight.normal, + color: theme.colors.foreground, + flexShrink: 1, + }, + rowMeta: { + fontSize: theme.fontSize.xs, + color: theme.colors.foregroundMuted, + marginLeft: "auto", + }, + rowMetaMid: { + fontSize: theme.fontSize.xs, + color: theme.colors.foregroundMuted, + flexShrink: 1, + }, + rowBody: { + fontSize: theme.fontSize.xs, + color: theme.colors.foregroundMuted, + lineHeight: 16, + }, + avatar: { + width: 20, + height: 20, + borderRadius: 10, + alignItems: "center", + justifyContent: "center", + marginTop: 1, + }, + avatarText: { + fontSize: 10, + fontWeight: theme.fontWeight.normal, + color: "#fff", + }, +})); diff --git a/packages/app/src/hooks/use-branch-switcher.ts b/packages/app/src/hooks/use-branch-switcher.ts index ea63dd58e..0665fb86b 100644 --- a/packages/app/src/hooks/use-branch-switcher.ts +++ b/packages/app/src/hooks/use-branch-switcher.ts @@ -3,7 +3,7 @@ import { useQuery, type QueryClient } from "@tanstack/react-query"; import type { DaemonClient } from "@server/client/daemon-client"; import type { ComboboxOption } from "@/components/ui/combobox"; import type { ToastApi } from "@/components/toast-host"; -import { checkoutStatusQueryKey } from "@/hooks/use-checkout-status-query"; +import { invalidateCheckoutGitQueriesForClient } from "@/stores/checkout-git-actions-store"; import { confirmDialog } from "@/utils/confirm-dialog"; interface UseBranchSwitcherInput { @@ -70,8 +70,9 @@ export function useBranchSwitcher({ const invalidateStashAndCheckout = useCallback(async () => { await Promise.all([ queryClient.invalidateQueries({ queryKey: stashListQueryKey }), - queryClient.invalidateQueries({ - queryKey: checkoutStatusQueryKey(normalizedServerId, normalizedWorkspaceId), + invalidateCheckoutGitQueriesForClient(queryClient, { + serverId: normalizedServerId, + cwd: normalizedWorkspaceId, }), ]); }, [queryClient, stashListQueryKey, normalizedServerId, normalizedWorkspaceId]); diff --git a/packages/app/src/hooks/use-checkout-pr-status-query.ts b/packages/app/src/hooks/use-checkout-pr-status-query.ts index 801b1033a..77cd9bfcc 100644 --- a/packages/app/src/hooks/use-checkout-pr-status-query.ts +++ b/packages/app/src/hooks/use-checkout-pr-status-query.ts @@ -5,7 +5,7 @@ import type { CheckoutPrStatusResponse } from "@server/shared/messages"; const CHECKOUT_PR_STATUS_STALE_TIME = 20_000; const WORKSPACE_PR_HINT_REFETCH_INTERVAL = 60_000; -function checkoutPrStatusQueryKey(serverId: string, cwd: string) { +export function checkoutPrStatusQueryKey(serverId: string, cwd: string) { return ["checkoutPrStatus", serverId, cwd] as const; } diff --git a/packages/app/src/hooks/use-pr-pane-data.test.ts b/packages/app/src/hooks/use-pr-pane-data.test.ts new file mode 100644 index 000000000..b82f1222b --- /dev/null +++ b/packages/app/src/hooks/use-pr-pane-data.test.ts @@ -0,0 +1,583 @@ +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { JSDOM } from "jsdom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { + CheckoutPrStatusResponse, + PullRequestTimelineResponse, +} from "@server/shared/messages"; +import { + prPaneTimelineQueryKey, + usePrPaneData, + type UsePrPaneDataResult, +} from "./use-pr-pane-data"; +import { useWorkspacePrHint } from "./use-checkout-pr-status-query"; + +type CheckoutPrStatus = NonNullable; +type CheckoutPrStatusPayload = CheckoutPrStatusResponse["payload"]; +type PullRequestTimelinePayload = PullRequestTimelineResponse["payload"]; + +const { mockRuntime, mockClient } = vi.hoisted(() => { + const mockClient = { + checkoutPrStatus: vi.fn(), + pullRequestTimeline: vi.fn(), + }; + + return { + mockClient, + mockRuntime: { + client: mockClient, + isConnected: true, + }, + }; +}); + +vi.mock("@/runtime/host-runtime", () => ({ + useHostRuntimeClient: () => mockRuntime.client, + useHostRuntimeIsConnected: () => mockRuntime.isConnected, +})); + +const cwd = "/repo"; +const serverId = "server-1"; + +function status(overrides: Partial = {}): CheckoutPrStatus { + return { + number: 42, + url: "https://github.com/getpaseo/paseo/pull/42", + title: "Wire real PR pane data", + state: "open", + baseRefName: "main", + headRefName: "feature/pr-pane", + isMerged: false, + isDraft: false, + checks: [], + reviewDecision: null, + repoOwner: "getpaseo", + repoName: "paseo", + ...overrides, + }; +} + +function statusPayload(overrides: Partial = {}): CheckoutPrStatusPayload { + return { + cwd, + status: status(), + githubFeaturesEnabled: true, + error: null, + requestId: "status-1", + ...overrides, + }; +} + +function timelinePayload( + overrides: Partial = {}, +): PullRequestTimelinePayload { + return { + cwd, + prNumber: 42, + items: [], + truncated: false, + error: null, + requestId: "timeline-1", + githubFeaturesEnabled: true, + ...overrides, + }; +} + +function unsupportedTimelineError(): Error { + const error = new Error( + "Unknown request schema requestType=pull_request_timeline_request code=unknown_schema", + ) as Error & { code: string; requestType: string }; + error.name = "DaemonRpcError"; + error.code = "unknown_schema"; + error.requestType = "pull_request_timeline_request"; + return error; +} + +function createTestQueryClient(): QueryClient { + return new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); +} + +function renderPrPaneHook({ + queryClient = createTestQueryClient(), + options = { serverId, cwd }, +}: { + queryClient?: QueryClient; + options?: Parameters[0]; +} = {}) { + let latest: UsePrPaneDataResult | null = null; + let currentOptions = options; + + function Probe({ hookOptions }: { hookOptions: Parameters[0] }) { + latest = usePrPaneData(hookOptions); + return null; + } + + const container = document.getElementById("root"); + if (!container) { + throw new Error("Missing root container"); + } + + const root = createRoot(container); + + function render() { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(Probe, { hookOptions: currentOptions }), + ), + ); + } + + return { + get latest() { + if (!latest) { + throw new Error("Expected hook result"); + } + return latest; + }, + queryClient, + async mount() { + await act(async () => { + render(); + }); + }, + async rerender(nextOptions: Parameters[0]) { + currentOptions = nextOptions; + await act(async () => { + render(); + }); + }, + async unmount() { + await act(async () => { + root.unmount(); + }); + }, + }; +} + +function renderSharedStatusConsumers({ queryClient = createTestQueryClient() } = {}) { + let latest: UsePrPaneDataResult | null = null; + + function Probe() { + latest = usePrPaneData({ serverId, cwd }); + useWorkspacePrHint({ serverId, cwd }); + return null; + } + + const container = document.getElementById("root"); + if (!container) { + throw new Error("Missing root container"); + } + + const root = createRoot(container); + + return { + get latest() { + if (!latest) { + throw new Error("Expected hook result"); + } + return latest; + }, + async mount() { + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(Probe), + ), + ); + }); + }, + }; +} + +async function waitForExpectation(assertion: () => void): Promise { + let lastError: unknown; + + for (let attempt = 0; attempt < 60; attempt += 1) { + try { + assertion(); + return; + } catch (error) { + lastError = error; + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + }); + } + } + + throw lastError; +} + +describe("usePrPaneData", () => { + beforeEach(() => { + const dom = new JSDOM("
", { + url: "http://localhost", + }); + + Object.defineProperty(globalThis, "document", { + value: dom.window.document, + configurable: true, + }); + Object.defineProperty(globalThis, "window", { + value: dom.window, + configurable: true, + }); + Object.defineProperty(globalThis, "navigator", { + value: dom.window.navigator, + configurable: true, + }); + Object.defineProperty(globalThis, "IS_REACT_ACT_ENVIRONMENT", { + value: true, + configurable: true, + }); + + mockRuntime.client = mockClient; + mockRuntime.isConnected = true; + mockClient.checkoutPrStatus.mockReset(); + mockClient.pullRequestTimeline.mockReset(); + }); + + it("returns null when status has no PR number", async () => { + const statusWithoutNumber = status(); + delete statusWithoutNumber.number; + mockClient.checkoutPrStatus.mockResolvedValue(statusPayload({ status: statusWithoutNumber })); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.isLoading).toBe(false); + }); + + expect(hook.latest.data).toBeNull(); + expect(mockClient.pullRequestTimeline).not.toHaveBeenCalled(); + }); + + it("wires the timeline query only when a PR number is known", async () => { + mockClient.checkoutPrStatus.mockResolvedValue(statusPayload({ status: null })); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.isLoading).toBe(false); + }); + + expect(mockClient.pullRequestTimeline).not.toHaveBeenCalled(); + expect( + hook.queryClient.getQueryState(prPaneTimelineQueryKey({ serverId, cwd, prNumber: 42 })), + ).toBeUndefined(); + + mockClient.checkoutPrStatus.mockResolvedValue(statusPayload()); + hook.queryClient.invalidateQueries({ queryKey: ["checkoutPrStatus", serverId, cwd] }); + + await waitForExpectation(() => { + expect(mockClient.pullRequestTimeline).toHaveBeenCalledWith({ + cwd, + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + }); + }); + }); + + it("checks PR status while timeline activity is disabled", async () => { + mockClient.checkoutPrStatus.mockResolvedValue(statusPayload()); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderPrPaneHook({ + options: { serverId, cwd, enabled: true, timelineEnabled: false }, + }); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.prNumber).toBe(42); + }); + + expect(hook.latest.data).toBeNull(); + expect(mockClient.checkoutPrStatus).toHaveBeenCalledTimes(1); + expect(mockClient.pullRequestTimeline).not.toHaveBeenCalled(); + }); + + it("does not request timeline activity until the PR repo identity is known", async () => { + mockClient.checkoutPrStatus.mockResolvedValue( + statusPayload({ + status: status({ + repoOwner: undefined, + repoName: undefined, + }), + }), + ); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.isLoading).toBe(false); + }); + + expect(hook.latest.data?.number).toBe(42); + expect(hook.latest.data?.activity).toEqual([]); + expect(mockClient.pullRequestTimeline).not.toHaveBeenCalled(); + }); + + it("shares the checkout PR status query with workspace hint consumers", async () => { + mockClient.checkoutPrStatus.mockResolvedValue(statusPayload()); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderSharedStatusConsumers(); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.data?.number).toBe(42); + }); + + expect(mockClient.checkoutPrStatus).toHaveBeenCalledTimes(1); + }); + + it("passes repoOwner and repoName to the timeline request when present", async () => { + mockClient.checkoutPrStatus.mockResolvedValue( + statusPayload({ + status: { + ...status(), + repoOwner: "fork-parent", + repoName: "paseo", + }, + }), + ); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(mockClient.pullRequestTimeline).toHaveBeenCalledWith({ + cwd, + prNumber: 42, + repoOwner: "fork-parent", + repoName: "paseo", + }); + }); + }); + + it("lets the mapper reject stale timeline activity for a mismatched PR number", async () => { + mockClient.checkoutPrStatus.mockResolvedValue(statusPayload()); + mockClient.pullRequestTimeline.mockResolvedValue( + timelinePayload({ + prNumber: 41, + items: [ + { + id: "comment-1", + kind: "comment", + author: "octocat", + body: "This belongs to another PR", + createdAt: Date.now(), + url: "https://github.com/getpaseo/paseo/pull/41#issuecomment-1", + }, + ], + }), + ); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.data?.number).toBe(42); + }); + + expect(hook.latest.data?.activity).toEqual([]); + }); + + it("suppresses old-daemon errors and prevents future timeline requests for that tuple", async () => { + const unsupportedError = unsupportedTimelineError(); + mockClient.checkoutPrStatus.mockResolvedValue( + statusPayload({ status: status({ number: 99 }) }), + ); + mockClient.pullRequestTimeline.mockRejectedValue(unsupportedError); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(mockClient.pullRequestTimeline).toHaveBeenCalledTimes(1); + }); + + await waitForExpectation(() => { + expect(hook.latest.error).toBeNull(); + }); + + await hook.rerender({ serverId, cwd }); + await hook.queryClient.invalidateQueries({ + queryKey: prPaneTimelineQueryKey({ serverId, cwd, prNumber: 99 }), + }); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + + expect(mockClient.pullRequestTimeline).toHaveBeenCalledTimes(1); + expect(hook.latest.data?.number).toBe(99); + }); + + it("surfaces checkout PR status payload errors", async () => { + mockClient.checkoutPrStatus.mockResolvedValue( + statusPayload({ + error: { + code: "UNKNOWN", + message: "bad daemon payload", + }, + }), + ); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.error).not.toBeNull(); + expect(hook.latest.error?.message).toContain("bad daemon payload"); + }); + }); + + it("retries timeline requests when the unsupported timeline tuple changes", async () => { + const cwdA = "/repo-a"; + const cwdB = "/repo-b"; + const prNumberA = 123; + const prNumberB = 124; + const unsupportedError = unsupportedTimelineError(); + + mockClient.checkoutPrStatus.mockImplementation(async (requestedCwd: string) => + statusPayload({ + cwd: requestedCwd, + status: status({ number: requestedCwd === cwdA ? prNumberA : prNumberB }), + }), + ); + mockClient.pullRequestTimeline.mockImplementation(async (input: { cwd: string }) => { + if (input.cwd === cwdA) { + throw unsupportedError; + } + return timelinePayload({ cwd: input.cwd, prNumber: prNumberB }); + }); + + const hook = renderPrPaneHook({ options: { serverId, cwd: cwdA } }); + await hook.mount(); + + await waitForExpectation(() => { + expect(countTimelineCalls({ cwd: cwdA, prNumber: prNumberA })).toBe(1); + }); + + await hook.rerender({ serverId, cwd: cwdA }); + await hook.queryClient.invalidateQueries({ + queryKey: prPaneTimelineQueryKey({ serverId, cwd: cwdA, prNumber: prNumberA }), + }); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + + expect(countTimelineCalls({ cwd: cwdA, prNumber: prNumberA })).toBe(1); + + await hook.rerender({ serverId, cwd: cwdB }); + + await waitForExpectation(() => { + expect(countTimelineCalls({ cwd: cwdB, prNumber: prNumberB })).toBe(1); + }); + expect(countTimelineCalls({ cwd: cwdA, prNumber: prNumberA })).toBe(1); + }); + + it("disables the timeline query when githubFeaturesEnabled is false", async () => { + mockClient.checkoutPrStatus.mockResolvedValue( + statusPayload({ githubFeaturesEnabled: false, status: status() }), + ); + mockClient.pullRequestTimeline.mockResolvedValue(timelinePayload()); + + const hook = renderPrPaneHook(); + await hook.mount(); + + await waitForExpectation(() => { + expect(hook.latest.data?.number).toBe(42); + }); + + expect(hook.latest.githubFeaturesEnabled).toBe(false); + expect(hook.latest.data?.activity).toEqual([]); + expect(mockClient.pullRequestTimeline).not.toHaveBeenCalled(); + }); + + it("reports first-load, background refresh, and non-suppressed errors", async () => { + const statusDeferred = createDeferred(); + mockClient.checkoutPrStatus.mockReturnValue(statusDeferred.promise); + mockClient.pullRequestTimeline.mockResolvedValue( + timelinePayload({ + error: { + kind: "unknown", + message: "rate limited", + }, + }), + ); + + const hook = renderPrPaneHook(); + await hook.mount(); + + expect(hook.latest.isLoading).toBe(true); + expect(hook.latest.isRefreshing).toBe(false); + + await act(async () => { + statusDeferred.resolve(statusPayload()); + }); + + await waitForExpectation(() => { + expect(hook.latest.data?.number).toBe(42); + expect(hook.latest.error?.message).toBe("rate limited"); + }); + + const refreshDeferred = createDeferred(); + mockClient.checkoutPrStatus.mockReturnValue(refreshDeferred.promise); + hook.queryClient.invalidateQueries({ queryKey: ["checkoutPrStatus", serverId, cwd] }); + + await waitForExpectation(() => { + expect(hook.latest.isLoading).toBe(false); + expect(hook.latest.isRefreshing).toBe(true); + }); + + await act(async () => { + refreshDeferred.resolve(statusPayload()); + }); + + await waitForExpectation(() => { + expect(hook.latest.isRefreshing).toBe(false); + }); + }); +}); + +function createDeferred(): { + promise: Promise; + resolve: (value: T) => void; +} { + let resolve: (value: T) => void = () => undefined; + const promise = new Promise((promiseResolve) => { + resolve = promiseResolve; + }); + + return { promise, resolve }; +} + +function countTimelineCalls({ cwd, prNumber }: { cwd: string; prNumber: number }): number { + return mockClient.pullRequestTimeline.mock.calls.filter(([input]) => { + const request = input as { cwd?: string; prNumber?: number | null }; + return request.cwd === cwd && request.prNumber === prNumber; + }).length; +} diff --git a/packages/app/src/hooks/use-pr-pane-data.ts b/packages/app/src/hooks/use-pr-pane-data.ts new file mode 100644 index 000000000..a51de0e51 --- /dev/null +++ b/packages/app/src/hooks/use-pr-pane-data.ts @@ -0,0 +1,183 @@ +import { keepPreviousData, useQuery } from "@tanstack/react-query"; +import { useHostRuntimeClient, useHostRuntimeIsConnected } from "@/runtime/host-runtime"; +import type { + CheckoutPrStatusResponse, + PullRequestTimelineResponse, +} from "@server/shared/messages"; +import { mapPrPaneData, type PrPaneData } from "../utils/pr-pane-data"; +import { useCheckoutPrStatusQuery } from "./use-checkout-pr-status-query"; + +const PR_PANE_TIMELINE_STALE_TIME = 15_000; +const PR_PANE_TIMELINE_REFETCH_INTERVAL = 30_000; + +type CheckoutPrStatusPayloadError = CheckoutPrStatusResponse["payload"]["error"]; +type PullRequestTimeline = PullRequestTimelineResponse["payload"]; + +const unsupportedTimelineKeys = new Set(); + +export interface UsePrPaneDataOptions { + serverId: string; + cwd: string; + enabled?: boolean; + timelineEnabled?: boolean; +} + +export interface UsePrPaneDataResult { + data: PrPaneData | null; + prNumber: number | null; + isLoading: boolean; + isRefreshing: boolean; + error: Error | null; + githubFeaturesEnabled: boolean; +} + +export function usePrPaneData({ + serverId, + cwd, + enabled = true, + timelineEnabled = enabled, +}: UsePrPaneDataOptions): UsePrPaneDataResult { + const daemonClient = useHostRuntimeClient(serverId); + const isConnected = useHostRuntimeIsConnected(serverId); + const checkoutPrStatus = useCheckoutPrStatusQuery({ serverId, cwd, enabled }); + const status = checkoutPrStatus.status; + const prNumber = status?.number ?? null; + const repoOwner = status?.repoOwner && status.repoOwner.length > 0 ? status.repoOwner : null; + const repoName = status?.repoName && status.repoName.length > 0 ? status.repoName : null; + const githubFeaturesEnabled = checkoutPrStatus.githubFeaturesEnabled !== false; + const unsupportedKey = + prNumber === null ? null : timelineUnsupportedKey({ serverId, cwd, prNumber }); + const timelineUnsupported = unsupportedKey ? unsupportedTimelineKeys.has(unsupportedKey) : false; + const shouldFetchTimeline = + !!daemonClient && + isConnected && + timelineEnabled && + githubFeaturesEnabled && + !!cwd && + prNumber !== null && + repoOwner !== null && + repoName !== null && + !timelineUnsupported; + + const timelineQuery = useQuery({ + queryKey: prPaneTimelineQueryKey({ serverId, cwd, prNumber }), + queryFn: async () => { + if (!daemonClient || prNumber === null || repoOwner === null || repoName === null) { + throw new Error("Daemon client not available"); + } + + try { + return await daemonClient.pullRequestTimeline({ + cwd, + prNumber, + repoOwner, + repoName, + }); + } catch (error) { + if (unsupportedKey && isUnsupportedTimelineError(error)) { + unsupportedTimelineKeys.add(unsupportedKey); + } + throw error; + } + }, + enabled: shouldFetchTimeline, + staleTime: PR_PANE_TIMELINE_STALE_TIME, + refetchInterval: PR_PANE_TIMELINE_REFETCH_INTERVAL, + placeholderData: keepPreviousData, + retry: (failureCount, error) => !isUnsupportedTimelineError(error) && failureCount < 3, + }); + + const data = + prNumber === null || !timelineEnabled ? null : mapPrPaneData(status, timelineQuery.data); + const statusRefreshing = checkoutPrStatus.isFetching && !checkoutPrStatus.isLoading; + const timelineRefreshing = timelineQuery.isFetching && !timelineQuery.isLoading; + + return { + data, + prNumber, + isLoading: + checkoutPrStatus.isLoading || + (shouldFetchTimeline && timelineQuery.isLoading && timelineQuery.data === undefined), + isRefreshing: statusRefreshing || timelineRefreshing, + error: firstNonSuppressedError({ + statusPayloadError: checkoutPrStatus.payloadError, + statusError: checkoutPrStatus.error, + timelineError: timelineQuery.error, + timelinePayloadError: timelineQuery.data?.error ?? null, + }), + githubFeaturesEnabled, + }; +} + +export function prPaneTimelineQueryKey({ + serverId, + cwd, + prNumber, +}: { + serverId: string; + cwd: string; + prNumber: number | null; +}) { + return ["prPaneTimeline", serverId, cwd, prNumber] as const; +} + +function firstNonSuppressedError({ + statusPayloadError, + statusError, + timelineError, + timelinePayloadError, +}: { + statusPayloadError: CheckoutPrStatusPayloadError; + statusError: Error | null; + timelineError: Error | null; + timelinePayloadError: PullRequestTimeline["error"]; +}): Error | null { + if (statusPayloadError) { + return new Error(statusPayloadError.message || "Unable to load pull request status"); + } + + if (statusError) { + return statusError; + } + + if (timelineError && !isUnsupportedTimelineError(timelineError)) { + return timelineError; + } + + if (timelinePayloadError) { + return new Error(timelinePayloadError.message || "Unable to load pull request activity"); + } + + return null; +} + +function timelineUnsupportedKey({ + serverId, + cwd, + prNumber, +}: { + serverId: string; + cwd: string; + prNumber: number; +}): string { + return `${serverId}\0${cwd}\0${prNumber}`; +} + +function isUnsupportedTimelineError(error: unknown): boolean { + if (!(error instanceof Error)) { + return false; + } + + const name = error.name.toLowerCase(); + const rpcError = error as Error & { code?: unknown; requestType?: unknown }; + + if ( + name === "daemonrpcerror" && + rpcError.code === "unknown_schema" && + rpcError.requestType === "pull_request_timeline_request" + ) { + return true; + } + + return false; +} diff --git a/packages/app/src/screens/workspace/workspace-git-actions.tsx b/packages/app/src/screens/workspace/workspace-git-actions.tsx index b30fe7bf4..1b5f65b10 100644 --- a/packages/app/src/screens/workspace/workspace-git-actions.tsx +++ b/packages/app/src/screens/workspace/workspace-git-actions.tsx @@ -18,11 +18,7 @@ interface WorkspaceGitActionsProps { hideLabels?: boolean; } -export function WorkspaceGitActions({ - serverId, - cwd, - hideLabels, -}: WorkspaceGitActionsProps) { +export function WorkspaceGitActions({ serverId, cwd, hideLabels }: WorkspaceGitActionsProps) { const { theme } = useUnistyles(); const icons = useMemo( diff --git a/packages/app/src/stores/checkout-git-actions-store.test.ts b/packages/app/src/stores/checkout-git-actions-store.test.ts index 7de1558e9..24c04d634 100644 --- a/packages/app/src/stores/checkout-git-actions-store.test.ts +++ b/packages/app/src/stores/checkout-git-actions-store.test.ts @@ -1,7 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; import { useSessionStore } from "@/stores/session-store"; import { __resetCheckoutGitActionsStoreForTests, + invalidateCheckoutGitQueriesForClient, useCheckoutGitActionsStore, } from "@/stores/checkout-git-actions-store"; @@ -61,4 +63,30 @@ describe("checkout-git-actions-store", () => { vi.advanceTimersByTime(1000); expect(store.getStatus({ serverId, cwd, actionId: "commit" })).toBe("idle"); }); + + it("invalidates checkout PR status and every PR pane timeline for a checkout", async () => { + const queryClient = new QueryClient(); + + queryClient.setQueryData(["checkoutPrStatus", serverId, cwd], { status: { number: 12 } }); + queryClient.setQueryData(["prPaneTimeline", serverId, cwd, 12], { items: [] }); + queryClient.setQueryData(["prPaneTimeline", serverId, cwd, 13], { items: [] }); + queryClient.setQueryData(["prPaneTimeline", serverId, "/tmp/other", 12], { items: [] }); + + await invalidateCheckoutGitQueriesForClient(queryClient, { serverId, cwd }); + + expect(queryClient.getQueryState(["checkoutPrStatus", serverId, cwd])?.isInvalidated).toBe( + true, + ); + expect(queryClient.getQueryState(["prPaneTimeline", serverId, cwd, 12])?.isInvalidated).toBe( + true, + ); + expect(queryClient.getQueryState(["prPaneTimeline", serverId, cwd, 13])?.isInvalidated).toBe( + true, + ); + expect( + queryClient.getQueryState(["prPaneTimeline", serverId, "/tmp/other", 12])?.isInvalidated, + ).toBe(false); + + queryClient.clear(); + }); }); diff --git a/packages/app/src/stores/checkout-git-actions-store.ts b/packages/app/src/stores/checkout-git-actions-store.ts index c49ce2fd1..b964602e5 100644 --- a/packages/app/src/stores/checkout-git-actions-store.ts +++ b/packages/app/src/stores/checkout-git-actions-store.ts @@ -1,10 +1,11 @@ +import type { QueryClient } from "@tanstack/react-query"; import { create } from "zustand"; -import { useSessionStore } from "@/stores/session-store"; -import { queryClient } from "@/query/query-client"; +import { queryClient as appQueryClient } from "@/query/query-client"; import { buildWorkspaceTabPersistenceKey, useWorkspaceLayoutStore, } from "@/stores/workspace-layout-store"; +import { useSessionStore } from "@/stores/session-store"; import { useWorkspaceTabsStore } from "@/stores/workspace-tabs-store"; const SUCCESS_DISPLAY_MS = 1000; @@ -59,34 +60,54 @@ function setStatus( }); } +export async function invalidateCheckoutGitQueriesForClient( + queryClient: QueryClient, + { serverId, cwd }: { serverId: string; cwd: string }, +) { + await Promise.all([ + queryClient.invalidateQueries({ + queryKey: ["checkoutStatus", serverId, cwd], + }), + queryClient.invalidateQueries({ + predicate: (query) => { + const key = query.queryKey; + return ( + Array.isArray(key) && key[0] === "checkoutDiff" && key[1] === serverId && key[2] === cwd + ); + }, + }), + queryClient.invalidateQueries({ + predicate: (query) => { + const key = query.queryKey; + return ( + Array.isArray(key) && + key[0] === "checkoutPrStatus" && + key[1] === serverId && + key[2] === cwd + ); + }, + }), + queryClient.invalidateQueries({ + predicate: (query) => { + const key = query.queryKey; + return ( + Array.isArray(key) && key[0] === "prPaneTimeline" && key[1] === serverId && key[2] === cwd + ); + }, + }), + ]); +} + function invalidateCheckoutGitQueries(serverId: string, cwd: string) { - void queryClient.invalidateQueries({ - queryKey: ["checkoutStatus", serverId, cwd], - }); - void queryClient.invalidateQueries({ - predicate: (query) => { - const key = query.queryKey; - return ( - Array.isArray(key) && key[0] === "checkoutDiff" && key[1] === serverId && key[2] === cwd - ); - }, - }); - void queryClient.invalidateQueries({ - predicate: (query) => { - const key = query.queryKey; - return ( - Array.isArray(key) && key[0] === "checkoutPrStatus" && key[1] === serverId && key[2] === cwd - ); - }, - }); + return invalidateCheckoutGitQueriesForClient(appQueryClient, { serverId, cwd }); } function invalidateWorktreeList() { - void queryClient.invalidateQueries({ + void appQueryClient.invalidateQueries({ predicate: (query) => Array.isArray(query.queryKey) && query.queryKey[0] === "paseoWorktreeList", }); - void queryClient.invalidateQueries({ + void appQueryClient.invalidateQueries({ predicate: (query) => Array.isArray(query.queryKey) && query.queryKey[0] === "sidebarPaseoWorktreeList", }); @@ -107,7 +128,7 @@ function removeWorktreeFromCachedLists(input: { serverId: string; worktreePath: return filtered.length === current.length ? current : filtered; }; - queryClient.setQueriesData( + appQueryClient.setQueriesData( { predicate: (query) => Array.isArray(query.queryKey) && @@ -117,7 +138,7 @@ function removeWorktreeFromCachedLists(input: { serverId: string; worktreePath: removeFromList, ); - queryClient.setQueriesData( + appQueryClient.setQueriesData( { predicate: (query) => Array.isArray(query.queryKey) && @@ -201,7 +222,7 @@ async function runCheckoutAction({ const promise = (async () => { try { await run(); - invalidateCheckoutGitQueries(serverId, cwd); + await invalidateCheckoutGitQueries(serverId, cwd); setStatus(key, actionId, "success"); const timer = setTimeout(() => { setStatus(key, actionId, "idle"); diff --git a/packages/app/src/stores/explorer-tab-memory.ts b/packages/app/src/stores/explorer-tab-memory.ts index 7a5e92225..3e26c0e94 100644 --- a/packages/app/src/stores/explorer-tab-memory.ts +++ b/packages/app/src/stores/explorer-tab-memory.ts @@ -1,7 +1,7 @@ -export type ExplorerTab = "changes" | "files"; +export type ExplorerTab = "changes" | "files" | "pr"; export function isExplorerTab(value: unknown): value is ExplorerTab { - return value === "changes" || value === "files"; + return value === "changes" || value === "files" || value === "pr"; } export function buildExplorerCheckoutKey(serverId: string, cwd: string): string | null { diff --git a/packages/app/src/styles/theme.ts b/packages/app/src/styles/theme.ts index 48a9addd4..d597d3cd5 100644 --- a/packages/app/src/styles/theme.ts +++ b/packages/app/src/styles/theme.ts @@ -117,6 +117,23 @@ const darkDiffColors = { diffDeletion: "#ef4444", // red-500 }; +// Status colors — semantic signals for success/danger/warning/merged. Used by +// check statuses, PR states, and review decisions. Kept a step darker than the +// raw palette so they read as signals, not neon. +const lightStatusColors = { + statusSuccess: "#15803d", // green-700 + statusDanger: "#b91c1c", // red-700 + statusWarning: "#d97706", // amber-600 + statusMerged: "#7c3aed", // purple-600 +}; + +const darkStatusColors = { + statusSuccess: "#16a34a", // green-600 + statusDanger: "#dc2626", // red-600 + statusWarning: "#f59e0b", // amber-500 + statusMerged: "#9333ea", // purple-600 +}; + // Semantic color tokens - Layer-based system const lightSemanticColors = { // Surfaces (layers) - shifted one step lighter @@ -167,6 +184,7 @@ const lightSemanticColors = { ring: "#18181b", ...lightDiffColors, + ...lightStatusColors, terminal: { background: "#ffffff", @@ -278,6 +296,7 @@ function buildDarkSemanticColors(tint: DarkThemeConfig) { ring: "#d4d4d8", ...darkDiffColors, + ...darkStatusColors, terminal: { background: tint.surface0, diff --git a/packages/app/src/utils/pr-pane-data.test.ts b/packages/app/src/utils/pr-pane-data.test.ts new file mode 100644 index 000000000..e16d69462 --- /dev/null +++ b/packages/app/src/utils/pr-pane-data.test.ts @@ -0,0 +1,337 @@ +import { describe, expect, it } from "vitest"; +import type { + CheckoutPrStatusResponse, + PullRequestTimelineResponse, +} from "@server/shared/messages"; +import { deriveAvatarColor, formatAge, mapPrPaneData } from "./pr-pane-data"; + +type CheckoutPrStatus = NonNullable; +type PullRequestTimeline = PullRequestTimelineResponse["payload"]; + +const baseStatus: CheckoutPrStatus = { + number: 42, + url: "https://github.com/getpaseo/paseo/pull/42", + title: "Wire PR pane data", + state: "open", + baseRefName: "main", + headRefName: "feature/pr-pane", + isMerged: false, + isDraft: false, + checks: [], + reviewDecision: null, +}; + +const baseTimeline: PullRequestTimeline = { + cwd: "/repo", + prNumber: 42, + items: [], + truncated: false, + error: null, + requestId: "timeline-1", + githubFeaturesEnabled: true, +}; + +function status(overrides: Partial = {}): CheckoutPrStatus { + return { ...baseStatus, ...overrides }; +} + +function timeline(overrides: Partial = {}): PullRequestTimeline { + return { ...baseTimeline, ...overrides }; +} + +describe("mapPrPaneData", () => { + it("returns null when no PR status exists", () => { + expect(mapPrPaneData(null, baseTimeline)).toBeNull(); + }); + + it("derives the PR number from the status URL when the status number is absent", () => { + const data = mapPrPaneData( + status({ + number: undefined, + url: "https://github.com/getpaseo/paseo/pull/1284", + }), + timeline({ prNumber: 1284 }), + ); + + expect(data?.number).toBe(1284); + }); + + it("returns null when status has no number and no parseable PR URL", () => { + expect( + mapPrPaneData(status({ number: undefined, url: "https://github.com/getpaseo/paseo" }), null), + ).toBeNull(); + }); + + it("derives PR state with merged taking precedence over closed, draft, and open", () => { + expect( + mapPrPaneData(status({ isMerged: true, isDraft: true, state: "closed" }), baseTimeline) + ?.state, + ).toBe("merged"); + expect( + mapPrPaneData(status({ isMerged: false, isDraft: true, state: "closed" }), baseTimeline) + ?.state, + ).toBe("closed"); + expect(mapPrPaneData(status({ isDraft: true, state: "open" }), baseTimeline)?.state).toBe( + "draft", + ); + expect(mapPrPaneData(status({ isDraft: false, state: "open" }), baseTimeline)?.state).toBe( + "open", + ); + }); + + it("drops checks with null URLs to preserve the pressable check contract", () => { + const data = mapPrPaneData( + status({ + checks: [ + { name: "typecheck", status: "success", url: "https://example.com/checks/1" }, + { name: "legacy status", status: "pending", url: null }, + ], + }), + baseTimeline, + ); + + expect(data?.checks).toEqual([ + { + name: "typecheck", + status: "success", + url: "https://example.com/checks/1", + }, + ]); + }); + + it("maps server check statuses into the frozen check status union", () => { + const data = mapPrPaneData( + status({ + checks: [ + { + name: "success", + status: "success", + url: "https://example.com/1", + workflow: "CI", + duration: "1m", + }, + { name: "failure", status: "failure", url: "https://example.com/2" }, + { name: "pending", status: "pending", url: "https://example.com/3" }, + { name: "skipped", status: "skipped", url: "https://example.com/4" }, + { name: "cancelled", status: "cancelled", url: "https://example.com/5" }, + ], + }), + baseTimeline, + ); + + expect(data?.checks).toEqual([ + { + name: "success", + workflow: "CI", + status: "success", + duration: "1m", + url: "https://example.com/1", + }, + { name: "failure", status: "failure", url: "https://example.com/2" }, + { name: "pending", status: "pending", url: "https://example.com/3" }, + { name: "skipped", status: "skipped", url: "https://example.com/4" }, + { name: "cancelled", status: "skipped", url: "https://example.com/5" }, + ]); + }); + + it("preserves timeline item order while mapping mixed reviews and comments", () => { + const data = mapPrPaneData( + baseStatus, + timeline({ + items: [ + { + id: "review-1", + kind: "review", + author: "alice", + reviewState: "approved", + body: "Looks good.", + createdAt: Date.UTC(2026, 0, 1, 10, 0, 0), + url: "https://example.com/review-1", + }, + { + id: "comment-1", + kind: "comment", + author: "bob", + body: "One thought.", + createdAt: Date.UTC(2026, 0, 1, 11, 0, 0), + url: "https://example.com/comment-1", + }, + ], + }), + Date.UTC(2026, 0, 1, 12, 0, 0), + ); + + expect(data?.activity.map((item) => item.kind)).toEqual(["review", "comment"]); + expect(data?.activity.map((item) => item.author)).toEqual(["alice", "bob"]); + }); + + it("filters empty commented reviews but keeps blocking review states", () => { + const data = mapPrPaneData( + baseStatus, + timeline({ + items: [ + { + id: "commented-empty", + kind: "review", + author: "alice", + reviewState: "commented", + body: " ", + createdAt: 1000, + url: "https://example.com/commented-empty", + }, + { + id: "approved-empty", + kind: "review", + author: "bob", + reviewState: "approved", + body: "", + createdAt: 2000, + url: "https://example.com/approved-empty", + }, + { + id: "changes-empty", + kind: "review", + author: "cam", + reviewState: "changes_requested", + body: "", + createdAt: 3000, + url: "https://example.com/changes-empty", + }, + ], + }), + 4000, + ); + + expect(data?.activity).toMatchObject([ + { kind: "review", author: "bob", reviewState: "approved", body: "" }, + { kind: "review", author: "cam", reviewState: "changes_requested", body: "" }, + ]); + }); + + it("filters empty issue comments while preserving review activity with empty bodies", () => { + const data = mapPrPaneData( + baseStatus, + timeline({ + items: [ + { + id: "comment-empty", + kind: "comment", + author: "alice", + body: " ", + createdAt: 1000, + url: "https://example.com/comment-empty", + }, + { + id: "approved-empty", + kind: "review", + author: "bob", + reviewState: "approved", + body: "", + createdAt: 2000, + url: "https://example.com/approved-empty", + }, + { + id: "approved-body", + kind: "review", + author: "cam", + reviewState: "approved", + body: "Looks good.", + createdAt: 3000, + url: "https://example.com/approved-body", + }, + ], + }), + 4000, + ); + + expect(data?.activity).toMatchObject([ + { kind: "review", author: "bob", reviewState: "approved", body: "" }, + { kind: "review", author: "cam", reviewState: "approved", body: "Looks good." }, + ]); + }); + + it("maps review decisions into the frozen pending fallback contract", () => { + expect( + mapPrPaneData(status({ reviewDecision: "approved" }), baseTimeline)?.reviewDecision, + ).toBe("approved"); + expect( + mapPrPaneData(status({ reviewDecision: "changes_requested" }), baseTimeline)?.reviewDecision, + ).toBe("changes_requested"); + expect( + mapPrPaneData(status({ reviewDecision: "review_required" }), baseTimeline)?.reviewDecision, + ).toBe("pending"); + expect(mapPrPaneData(status({ reviewDecision: null }), baseTimeline)?.reviewDecision).toBe( + "pending", + ); + expect( + mapPrPaneData( + status({ reviewDecision: undefined as CheckoutPrStatus["reviewDecision"] }), + baseTimeline, + )?.reviewDecision, + ).toBe("pending"); + expect( + mapPrPaneData( + status({ reviewDecision: "surprising" as CheckoutPrStatus["reviewDecision"] }), + baseTimeline, + )?.reviewDecision, + ).toBe("pending"); + }); + + it("leaves awaiting reviewers intentionally unwired", () => { + expect(mapPrPaneData(baseStatus, baseTimeline)?.awaitingReviewers).toEqual([]); + }); + + it("rejects stale timeline activity when the timeline PR number differs from status", () => { + const data = mapPrPaneData( + baseStatus, + timeline({ + prNumber: 99, + items: [ + { + id: "comment-1", + kind: "comment", + author: "alice", + body: "This belongs to another PR.", + createdAt: 1000, + url: "https://example.com/comment-1", + }, + ], + }), + 2000, + ); + + expect(data?.activity).toEqual([]); + }); +}); + +describe("deriveAvatarColor", () => { + it("returns a deterministic color from the PR pane avatar palette", () => { + const palette = [ + "#8b5cf6", + "#f97316", + "#0ea5e9", + "#10b981", + "#ef4444", + "#eab308", + "#ec4899", + "#6366f1", + ]; + + expect(deriveAvatarColor("alice")).toBe(deriveAvatarColor("alice")); + expect(palette).toContain(deriveAvatarColor("alice")); + expect(palette).toContain(deriveAvatarColor("Alice")); + }); +}); + +describe("formatAge", () => { + it("emits PR pane age labels", () => { + const now = Date.UTC(2026, 0, 1, 12, 0, 0); + + expect(formatAge(now - 20_000, now)).toBe("just now"); + expect(formatAge(now - 5 * 60_000, now)).toBe("5m ago"); + expect(formatAge(now - 2 * 60 * 60_000, now)).toBe("2h ago"); + expect(formatAge(now - 3 * 24 * 60 * 60_000, now)).toBe("3d ago"); + expect(formatAge(now - 90 * 24 * 60 * 60_000, now)).toBe("3mo ago"); + expect(formatAge(now - 365 * 24 * 60 * 60_000, now)).toBe("1y ago"); + }); +}); diff --git a/packages/app/src/utils/pr-pane-data.ts b/packages/app/src/utils/pr-pane-data.ts new file mode 100644 index 000000000..7fcf424ef --- /dev/null +++ b/packages/app/src/utils/pr-pane-data.ts @@ -0,0 +1,227 @@ +import type { + CheckoutPrStatusResponse, + PullRequestTimelineResponse, +} from "@server/shared/messages"; + +export type PrState = "open" | "draft" | "merged" | "closed"; +export type CheckStatus = "success" | "failure" | "pending" | "skipped"; +export type ReviewState = "approved" | "changes_requested" | "commented"; +export type ActivityKind = "review" | "comment"; + +export interface PrPaneCheck { + name: string; + workflow?: string; + status: CheckStatus; + duration?: string; + url: string; +} + +export interface PrPaneActivity { + kind: ActivityKind; + author: string; + avatarColor: string; + reviewState?: ReviewState; + body: string; + age: string; + url: string; +} + +export interface PrPaneData { + number: number; + title: string; + state: PrState; + url: string; + reviewDecision: "approved" | "changes_requested" | "pending"; + awaitingReviewers: string[]; + checks: PrPaneCheck[]; + activity: PrPaneActivity[]; +} + +type CheckoutPrStatus = CheckoutPrStatusResponse["payload"]["status"]; +type PullRequestTimeline = PullRequestTimelineResponse["payload"]; +type PullRequestTimelineItem = PullRequestTimeline["items"][number]; + +const AVATAR_COLORS = [ + "#8b5cf6", + "#f97316", + "#0ea5e9", + "#10b981", + "#ef4444", + "#eab308", + "#ec4899", + "#6366f1", +]; + +export function mapPrPaneData( + status: CheckoutPrStatus, + timeline: PullRequestTimeline | null | undefined, + nowMs = Date.now(), +): PrPaneData | null { + if (!status) { + return null; + } + + const number = status.number ?? parsePullRequestNumber(status.url); + if (number === null) { + return null; + } + + const timelineMatchesStatus = timeline?.prNumber === number; + + return { + number, + title: status.title, + state: derivePrState(status), + url: status.url, + reviewDecision: mapReviewDecision(status.reviewDecision), + // Requested reviewers are intentionally unwired until the server exposes them. + awaitingReviewers: [], + checks: (status.checks ?? []).flatMap(mapCheck), + activity: timelineMatchesStatus + ? timeline.items.flatMap((item) => mapActivity(item, nowMs)) + : [], + }; +} + +export function deriveAvatarColor(login: string): string { + return AVATAR_COLORS[hashLogin(login) % AVATAR_COLORS.length]; +} + +export function formatAge(createdAtMs: number, nowMs = Date.now()): string { + const elapsedMs = Math.max(0, nowMs - createdAtMs); + const elapsedSeconds = Math.floor(elapsedMs / 1000); + + if (elapsedSeconds < 60) { + return "just now"; + } + + const elapsedMinutes = Math.floor(elapsedSeconds / 60); + if (elapsedMinutes < 60) { + return `${elapsedMinutes}m ago`; + } + + const elapsedHours = Math.floor(elapsedMinutes / 60); + if (elapsedHours < 24) { + return `${elapsedHours}h ago`; + } + + const elapsedDays = Math.floor(elapsedHours / 24); + if (elapsedDays < 30) { + return `${elapsedDays}d ago`; + } + + if (elapsedDays < 365) { + return `${Math.floor(elapsedDays / 30)}mo ago`; + } + + return `${Math.floor(elapsedDays / 365)}y ago`; +} + +function derivePrState(status: NonNullable): PrState { + if (status.isMerged || status.state === "merged") { + return "merged"; + } + if (status.state !== "open") { + return "closed"; + } + if (status.isDraft) { + return "draft"; + } + return "open"; +} + +function mapCheck(check: NonNullable["checks"][number]): PrPaneCheck[] { + if (check.url === null) { + return []; + } + + return [ + { + name: check.name, + workflow: check.workflow, + status: mapCheckStatus(check.status), + duration: check.duration, + url: check.url, + }, + ]; +} + +function mapCheckStatus(status: string): CheckStatus { + if ( + status === "success" || + status === "failure" || + status === "pending" || + status === "skipped" + ) { + return status; + } + if (status === "cancelled") { + return "skipped"; + } + return "pending"; +} + +function mapActivity(item: PullRequestTimelineItem, nowMs: number): PrPaneActivity[] { + if (item.kind === "comment") { + if (item.body.trim() === "") { + return []; + } + return [ + { + kind: "comment", + author: item.author, + avatarColor: deriveAvatarColor(item.author), + body: item.body, + age: formatAge(item.createdAt, nowMs), + url: item.url, + }, + ]; + } + + if (item.reviewState === "commented" && item.body.trim() === "") { + return []; + } + + return [ + { + kind: "review", + author: item.author, + avatarColor: deriveAvatarColor(item.author), + reviewState: item.reviewState, + body: item.body, + age: formatAge(item.createdAt, nowMs), + url: item.url, + }, + ]; +} + +function mapReviewDecision( + reviewDecision: NonNullable["reviewDecision"], +): PrPaneData["reviewDecision"] { + if (reviewDecision === "approved" || reviewDecision === "changes_requested") { + return reviewDecision; + } + return "pending"; +} + +function parsePullRequestNumber(url: string): number | null { + try { + const match = new URL(url).pathname.match(/\/pull\/(\d+)(?:\/|$)/); + if (!match) { + return null; + } + + const number = Number.parseInt(match[1], 10); + return Number.isFinite(number) ? number : null; + } catch { + return null; + } +} + +function hashLogin(login: string): number { + let hash = 0; + for (const character of login.toLowerCase()) { + hash = (hash * 31 + character.charCodeAt(0)) >>> 0; + } + return hash; +} diff --git a/packages/app/src/utils/pr-tab-label.test.ts b/packages/app/src/utils/pr-tab-label.test.ts new file mode 100644 index 000000000..8150fc198 --- /dev/null +++ b/packages/app/src/utils/pr-tab-label.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import type { PrPaneData } from "./pr-pane-data"; +import { formatPrTabLabel } from "./pr-tab-label"; + +function makePrPaneData(overrides: Partial = {}): PrPaneData { + return { + number: 42, + title: "Sample PR", + state: "open", + url: "https://github.com/getpaseo/paseo/pull/42", + reviewDecision: "pending", + awaitingReviewers: [], + checks: [], + activity: [], + ...overrides, + }; +} + +describe("formatPrTabLabel", () => { + it("returns # when data is present", () => { + expect(formatPrTabLabel(makePrPaneData({ number: 42 }))).toBe("#42"); + }); + + it("returns #— fallback when data is null", () => { + expect(formatPrTabLabel(null)).toBe("#—"); + }); +}); diff --git a/packages/app/src/utils/pr-tab-label.ts b/packages/app/src/utils/pr-tab-label.ts new file mode 100644 index 000000000..9bc079ffd --- /dev/null +++ b/packages/app/src/utils/pr-tab-label.ts @@ -0,0 +1,5 @@ +import type { PrPaneData } from "./pr-pane-data"; + +export function formatPrTabLabel(data: PrPaneData | null): string { + return data ? `#${data.number}` : "#—"; +} diff --git a/packages/server/src/client/daemon-client.ts b/packages/server/src/client/daemon-client.ts index 54e3ba304..98f07cb9f 100644 --- a/packages/server/src/client/daemon-client.ts +++ b/packages/server/src/client/daemon-client.ts @@ -30,6 +30,7 @@ import type { CheckoutPushResponse, CheckoutPrCreateResponse, CheckoutPrStatusResponse, + PullRequestTimelineResponse, CheckoutSwitchBranchResponse, StashSaveResponse, StashPopResponse, @@ -242,6 +243,7 @@ type CheckoutPullPayload = CheckoutPullResponse["payload"]; type CheckoutPushPayload = CheckoutPushResponse["payload"]; type CheckoutPrCreatePayload = CheckoutPrCreateResponse["payload"]; type CheckoutPrStatusPayload = CheckoutPrStatusResponse["payload"]; +type PullRequestTimelinePayload = PullRequestTimelineResponse["payload"]; type CheckoutSwitchBranchPayload = CheckoutSwitchBranchResponse["payload"]; type StashSavePayload = StashSaveResponse["payload"]; type StashPopPayload = StashPopResponse["payload"]; @@ -2485,6 +2487,24 @@ export class DaemonClient { }); } + async pullRequestTimeline( + input: { cwd: string; prNumber: number; repoOwner: string; repoName: string }, + requestId?: string, + ): Promise { + return this.sendCorrelatedSessionRequest({ + requestId, + message: { + type: "pull_request_timeline_request", + cwd: input.cwd, + prNumber: input.prNumber, + repoOwner: input.repoOwner, + repoName: input.repoName, + }, + responseType: "pull_request_timeline_response", + timeout: 60000, + }); + } + async checkoutSwitchBranch( cwd: string, branch: string, diff --git a/packages/server/src/server/session.test.ts b/packages/server/src/server/session.test.ts new file mode 100644 index 000000000..53a18b69d --- /dev/null +++ b/packages/server/src/server/session.test.ts @@ -0,0 +1,288 @@ +import pino from "pino"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { CheckoutPrStatusSchema } from "../shared/messages.js"; +import { normalizeCheckoutPrStatusPayload, Session } from "./session.js"; + +const checkoutGitMocks = vi.hoisted(() => ({ + getCheckoutStatus: vi.fn(), + mergeToBase: vi.fn(), +})); + +vi.mock("../utils/checkout-git.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getCheckoutStatus: checkoutGitMocks.getCheckoutStatus, + mergeToBase: checkoutGitMocks.mergeToBase, + }; +}); + +function createSessionForTest(options?: { + github?: { + invalidate: ReturnType; + isAuthenticated?: ReturnType; + getPullRequestTimeline?: ReturnType; + }; + checkoutDiffManager?: { scheduleRefreshForCwd: ReturnType }; + messages?: unknown[]; +}): Session { + const logger = pino({ level: "silent" }); + const github = options?.github ?? { invalidate: vi.fn() }; + const checkoutDiffManager = options?.checkoutDiffManager ?? { + scheduleRefreshForCwd: vi.fn(), + }; + const messages = options?.messages ?? []; + + return new Session({ + clientId: "test-client", + onMessage: (message) => messages.push(message), + logger, + downloadTokenStore: {} as any, + pushTokenStore: {} as any, + paseoHome: "/tmp/paseo-home", + agentManager: { + subscribe: vi.fn(() => () => {}), + } as any, + agentStorage: {} as any, + projectRegistry: {} as any, + workspaceRegistry: {} as any, + chatService: {} as any, + scheduleService: {} as any, + loopService: {} as any, + checkoutDiffManager: checkoutDiffManager as any, + github: github as any, + workspaceGitService: {} as any, + daemonConfigStore: {} as any, + stt: null, + tts: null, + terminalManager: null, + }); +} + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe("session PR status payload normalization", () => { + test("includes repository identity fields on the wire", () => { + const payload = normalizeCheckoutPrStatusPayload({ + number: 123, + repoOwner: "internal-owner", + repoName: "internal-repo", + url: "https://github.com/getpaseo/paseo/pull/123", + title: "Ship PR pane", + state: "open", + baseRefName: "main", + headRefName: "feature/pr-pane", + isMerged: false, + isDraft: true, + checks: [ + { + name: "typecheck", + status: "success", + url: "https://github.com/getpaseo/paseo/actions/runs/1", + workflow: "CI", + duration: "1m 20s", + }, + ], + checksStatus: "success", + reviewDecision: "approved", + }); + + expect(payload).toHaveProperty("repoOwner", "internal-owner"); + expect(payload).toHaveProperty("repoName", "internal-repo"); + expect(CheckoutPrStatusSchema.parse(payload)).toEqual(payload); + }); +}); + +describe("session checkout merge handling", () => { + test("invalidates the cwd mutated by merge-to-base", async () => { + const messages: unknown[] = []; + const github = { invalidate: vi.fn() }; + const checkoutDiffManager = { scheduleRefreshForCwd: vi.fn() }; + const session = createSessionForTest({ github, checkoutDiffManager, messages }); + + checkoutGitMocks.getCheckoutStatus.mockResolvedValue({ + isGit: true, + baseRef: "main", + }); + checkoutGitMocks.mergeToBase.mockResolvedValue("/tmp/base-worktree"); + + await (session as any).handleCheckoutMergeRequest({ + type: "checkout_merge_request", + cwd: "/tmp/request-worktree", + baseRef: "main", + requestId: "request-1", + }); + + expect(checkoutGitMocks.mergeToBase).toHaveBeenCalledWith( + "/tmp/request-worktree", + { + baseRef: "main", + mode: "merge", + }, + { paseoHome: "/tmp/paseo-home" }, + ); + expect(github.invalidate).toHaveBeenCalledTimes(1); + expect(github.invalidate).toHaveBeenCalledWith({ cwd: "/tmp/base-worktree" }); + expect(checkoutDiffManager.scheduleRefreshForCwd).toHaveBeenCalledWith("/tmp/request-worktree"); + expect(messages).toContainEqual({ + type: "checkout_merge_response", + payload: { + cwd: "/tmp/request-worktree", + success: true, + error: null, + requestId: "request-1", + }, + }); + }); +}); + +describe("session pull request timeline handling", () => { + test("passes request identity to GitHubService and emits timeline items", async () => { + const messages: unknown[] = []; + const github = { + invalidate: vi.fn(), + isAuthenticated: vi.fn().mockResolvedValue(true), + getPullRequestTimeline: vi.fn().mockResolvedValue({ + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + items: [ + { + id: "review-1", + kind: "review", + author: "octocat", + authorUrl: "https://github.com/octocat", + body: "Looks good", + createdAt: 1710000000000, + url: "https://github.com/getpaseo/paseo/pull/42#pullrequestreview-1", + reviewState: "approved", + }, + ], + truncated: false, + error: null, + }), + }; + const session = createSessionForTest({ github, messages }); + + await session.handleMessage({ + type: "pull_request_timeline_request", + cwd: "/tmp/repo", + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + requestId: "request-1", + }); + + expect(github.getPullRequestTimeline).toHaveBeenCalledWith({ + cwd: "/tmp/repo", + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + }); + expect(messages).toContainEqual({ + type: "pull_request_timeline_response", + payload: { + cwd: "/tmp/repo", + prNumber: 42, + items: [ + { + id: "review-1", + kind: "review", + author: "octocat", + body: "Looks good", + createdAt: 1710000000000, + url: "https://github.com/getpaseo/paseo/pull/42#pullrequestreview-1", + reviewState: "approved", + }, + ], + truncated: false, + error: null, + requestId: "request-1", + githubFeaturesEnabled: true, + }, + }); + }); + + test.each([ + { prNumber: 0, repoOwner: "getpaseo", repoName: "paseo" }, + { prNumber: -1, repoOwner: "getpaseo", repoName: "paseo" }, + { prNumber: 42, repoOwner: "get paseo", repoName: "paseo" }, + { prNumber: 42, repoOwner: "getpaseo/cli", repoName: "paseo" }, + { prNumber: 42, repoOwner: "get$paseo", repoName: "paseo" }, + { prNumber: 42, repoOwner: "getpaseo", repoName: "pa seo" }, + { prNumber: 42, repoOwner: "getpaseo", repoName: "paseo/app" }, + { prNumber: 42, repoOwner: "getpaseo", repoName: "paseo!" }, + ])("returns an unknown error when request identity is invalid: %j", async (identity) => { + const messages: unknown[] = []; + const github = { + invalidate: vi.fn(), + isAuthenticated: vi.fn().mockResolvedValue(true), + getPullRequestTimeline: vi.fn(), + }; + const session = createSessionForTest({ github, messages }); + + await session.handleMessage({ + type: "pull_request_timeline_request", + cwd: "/tmp/repo", + ...identity, + requestId: "request-invalid", + }); + + expect(github.isAuthenticated).not.toHaveBeenCalled(); + expect(github.getPullRequestTimeline).not.toHaveBeenCalled(); + expect(messages).toContainEqual({ + type: "pull_request_timeline_response", + payload: { + cwd: "/tmp/repo", + prNumber: identity.prNumber, + items: [], + truncated: false, + error: { + kind: "unknown", + message: "Pull request timeline request has invalid PR identity", + }, + requestId: "request-invalid", + githubFeaturesEnabled: true, + }, + }); + }); + + test("disables GitHub features when gh auth is unavailable", async () => { + const messages: unknown[] = []; + const github = { + invalidate: vi.fn(), + isAuthenticated: vi.fn().mockResolvedValue(false), + getPullRequestTimeline: vi.fn(), + }; + const session = createSessionForTest({ github, messages }); + + await session.handleMessage({ + type: "pull_request_timeline_request", + cwd: "/tmp/repo", + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + requestId: "request-3", + }); + + expect(github.getPullRequestTimeline).not.toHaveBeenCalled(); + expect(messages).toContainEqual({ + type: "pull_request_timeline_response", + payload: { + cwd: "/tmp/repo", + prNumber: 42, + items: [], + truncated: false, + error: { + kind: "unknown", + message: "GitHub CLI is unavailable or not authenticated", + }, + requestId: "request-3", + githubFeaturesEnabled: false, + }, + }); + }); +}); diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index c4d51b2b4..ee3e5dc5a 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -191,7 +191,11 @@ import { notifyChatMentions } from "./chat/chat-mentions.js"; import { LoopService } from "./loop-service.js"; import { ScheduleService } from "./schedule/service.js"; import { execCommand } from "../utils/spawn.js"; -import { createGitHubService, type GitHubService } from "../services/github-service.js"; +import { + createGitHubService, + type GitHubService, + type PullRequestTimelineItem, +} from "../services/github-service.js"; import { createPaseoWorktree, type CreatePaseoWorktreeInput, @@ -530,6 +534,17 @@ export type SessionLifecycleIntent = reason?: string; }; +type CheckoutPrStatusPayload = Extract< + SessionOutboundMessage, + { type: "checkout_pr_status_response" } +>["payload"]; +type CheckoutPrStatusPayloadStatus = NonNullable; +type PullRequestTimelinePayload = Extract< + SessionOutboundMessage, + { type: "pull_request_timeline_response" } +>["payload"]; +type PullRequestTimelinePayloadItem = PullRequestTimelinePayload["items"][number]; + type VoiceFeatureUnavailableContext = { reasonCode: SpeechReadinessSnapshot["voiceFeature"]["reasonCode"]; message: string; @@ -1654,6 +1669,10 @@ export class Session { await this.handleCheckoutPrStatusRequest(msg); break; + case "pull_request_timeline_request": + await this.handlePullRequestTimelineRequest(msg); + break; + case "github_search_request": await this.handleGitHubSearchRequest(msg); break; @@ -4753,7 +4772,7 @@ export class Session { type: "checkout_pr_status_response", payload: { cwd, - status: snapshot.github.pullRequest, + status: normalizeCheckoutPrStatusPayload(snapshot.github.pullRequest), githubFeaturesEnabled: snapshot.github.featuresEnabled, error: snapshot.github.error ? { @@ -4778,6 +4797,88 @@ export class Session { } } + private async handlePullRequestTimelineRequest( + msg: Extract, + ): Promise { + const { cwd, prNumber, repoOwner, repoName, requestId } = msg; + + if (!isValidPullRequestTimelineIdentity({ prNumber, repoOwner, repoName })) { + this.emit({ + type: "pull_request_timeline_response", + payload: { + cwd, + prNumber, + items: [], + truncated: false, + error: { + kind: "unknown", + message: "Pull request timeline request has invalid PR identity", + }, + requestId, + githubFeaturesEnabled: true, + }, + }); + return; + } + + const githubFeaturesEnabled = await this.github.isAuthenticated({ cwd }); + if (!githubFeaturesEnabled) { + this.emit({ + type: "pull_request_timeline_response", + payload: { + cwd, + prNumber, + items: [], + truncated: false, + error: { + kind: "unknown", + message: "GitHub CLI is unavailable or not authenticated", + }, + requestId, + githubFeaturesEnabled: false, + }, + }); + return; + } + + try { + const timeline = await this.github.getPullRequestTimeline({ + cwd, + prNumber, + repoOwner, + repoName, + }); + this.emit({ + type: "pull_request_timeline_response", + payload: { + cwd, + prNumber: timeline.prNumber, + items: timeline.items.map(toPullRequestTimelinePayloadItem), + truncated: timeline.truncated, + error: timeline.error, + requestId, + githubFeaturesEnabled: true, + }, + }); + } catch (error) { + this.emit({ + type: "pull_request_timeline_response", + payload: { + cwd, + prNumber, + items: [], + truncated: false, + error: { + kind: "unknown", + message: error instanceof Error ? error.message : String(error), + }, + requestId, + githubFeaturesEnabled: true, + }, + }); + } + } + private async handlePaseoWorktreeListRequest( msg: Extract, ): Promise { @@ -8748,3 +8849,48 @@ export class Session { } } } + +export function normalizeCheckoutPrStatusPayload( + status: WorkspaceGitRuntimeSnapshot["github"]["pullRequest"], +): CheckoutPrStatusPayloadStatus | null { + if (!status) { + return null; + } + return { + number: status.number, + url: status.url, + title: status.title, + state: status.state, + repoOwner: status.repoOwner, + repoName: status.repoName, + baseRefName: status.baseRefName, + headRefName: status.headRefName, + isMerged: status.isMerged, + isDraft: status.isDraft ?? false, + checks: status.checks ?? [], + checksStatus: status.checksStatus, + reviewDecision: status.reviewDecision, + }; +} + +function isValidPullRequestTimelineIdentity(options: { + prNumber: number; + repoOwner: string; + repoName: string; +}): boolean { + if (!Number.isInteger(options.prNumber) || options.prNumber <= 0) { + return false; + } + return isValidGitHubRepoSegment(options.repoOwner) && isValidGitHubRepoSegment(options.repoName); +} + +function isValidGitHubRepoSegment(value: string): boolean { + return /^[A-Za-z0-9._-]+$/.test(value); +} + +function toPullRequestTimelinePayloadItem( + item: PullRequestTimelineItem, +): PullRequestTimelinePayloadItem { + const { authorUrl: _authorUrl, ...payload } = item; + return payload; +} diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index fae8bd9bd..469f890d4 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -38,12 +38,25 @@ export type WorkspaceGitRuntimeSnapshot = { github: { featuresEnabled: boolean; pullRequest: { + number?: number; + repoOwner?: string; + repoName?: string; url: string; title: string; state: string; baseRefName: string; headRefName: string; isMerged: boolean; + isDraft?: boolean; + checks?: Array<{ + name: string; + status: "success" | "failure" | "pending" | "skipped" | "cancelled"; + url: string | null; + workflow?: string; + duration?: string; + }>; + checksStatus?: "none" | "pending" | "success" | "failure"; + reviewDecision?: "approved" | "changes_requested" | "pending" | null; } | null; error: { message: string } | null; refreshedAt: string | null; diff --git a/packages/server/src/services/github-service.test.ts b/packages/server/src/services/github-service.test.ts index 431e01e76..2b0f92160 100644 --- a/packages/server/src/services/github-service.test.ts +++ b/packages/server/src/services/github-service.test.ts @@ -7,6 +7,7 @@ import { type GitHubCommandRunner, type GitHubCommandRunnerOptions, } from "./github-service.js"; +import { CheckoutPrStatusResponseSchema } from "../shared/messages.js"; interface RunnerCall { args: string[]; @@ -19,6 +20,14 @@ interface TestRunner { resolveNext: (stdout: string) => void; } +type RunnerStep = + | string + | { + stdout?: string; + stderr?: string; + error?: Error; + }; + function createRunner(stdoutByCall: string[]): TestRunner { const calls: RunnerCall[] = []; @@ -33,6 +42,26 @@ function createRunner(stdoutByCall: string[]): TestRunner { }; } +function createScriptedRunner(steps: RunnerStep[]): TestRunner { + const calls: RunnerCall[] = []; + + return { + calls, + runner: async (args: string[], options: GitHubCommandRunnerOptions) => { + calls.push({ args, cwd: options.cwd }); + const step = steps.shift() ?? ""; + if (typeof step === "string") { + return { stdout: step, stderr: "" }; + } + if (step.error) { + throw step.error; + } + return { stdout: step.stdout ?? "", stderr: step.stderr ?? "" }; + }, + resolveNext: () => {}, + }; +} + function createDeferredRunner(): TestRunner { const calls: RunnerCall[] = []; let resolveNext: ((stdout: string) => void) | null = null; @@ -54,6 +83,31 @@ function createDeferredRunner(): TestRunner { }; } +function currentPullRequestJson(overrides: Record = {}): string { + return JSON.stringify({ + number: 42, + url: "https://github.com/parentOwner/parentRepo/pull/42", + title: "Fork PR", + state: "OPEN", + isDraft: false, + baseRefName: "main", + headRefName: "feature/fork", + mergedAt: null, + statusCheckRollup: [], + reviewDecision: "REVIEW_REQUIRED", + ...overrides, + }); +} + +function noPullRequestError(args: string[] = ["pr", "view"]): GitHubCommandError { + return new GitHubCommandError({ + args, + cwd: "/repo", + exitCode: 1, + stderr: "no pull requests found for branch", + }); +} + function pullRequestJson(title: string): string { return JSON.stringify([ { @@ -68,7 +122,939 @@ function pullRequestJson(title: string): string { ]); } +function pullRequestTimelineJson(overrides: Record = {}): string { + return JSON.stringify({ + data: { + repository: { + pullRequest: { + number: 42, + reviews: { + nodes: [ + { + id: "PRR_approved", + state: "APPROVED", + body: "Looks good to me.", + url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-1", + submittedAt: "2026-04-02T13:52:14Z", + author: { + login: "reviewer", + url: "https://github.com/reviewer", + }, + }, + { + id: "PRR_empty_commented", + state: "COMMENTED", + body: "", + url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-2", + submittedAt: "2026-04-02T13:50:00Z", + author: null, + }, + ], + pageInfo: { hasNextPage: false }, + }, + comments: { + nodes: [ + { + id: "IC_later", + body: "Can we add a regression test?", + url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-3", + createdAt: "2026-04-02T13:55:00Z", + author: { + login: "commenter", + url: "https://github.com/commenter", + }, + }, + ], + pageInfo: { hasNextPage: false }, + }, + ...overrides, + }, + }, + }, + }); +} + describe("GitHubService", () => { + it("fetches PR reviews and issue comments with one GraphQL call sorted chronologically", async () => { + const runner = createRunner([pullRequestTimelineJson()]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(runner.calls).toHaveLength(1); + expect(runner.calls[0]).toMatchObject({ + cwd: "/repo", + args: [ + "api", + "graphql", + "-f", + expect.stringContaining("query="), + "-F", + "owner=parentOwner", + "-F", + "name=parentRepo", + "-F", + "number=42", + ], + }); + expect(runner.calls[0]?.args[3]).toContain("reviews(first: 100)"); + expect(runner.calls[0]?.args[3]).toContain("comments(first: 100)"); + expect(timeline).toEqual({ + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + truncated: false, + error: null, + items: [ + { + kind: "review", + id: "PRR_empty_commented", + author: "unknown", + authorUrl: null, + body: "", + createdAt: Date.parse("2026-04-02T13:50:00Z"), + url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-2", + reviewState: "commented", + }, + { + kind: "review", + id: "PRR_approved", + author: "reviewer", + authorUrl: "https://github.com/reviewer", + body: "Looks good to me.", + createdAt: Date.parse("2026-04-02T13:52:14Z"), + url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-1", + reviewState: "approved", + }, + { + kind: "comment", + id: "IC_later", + author: "commenter", + authorUrl: "https://github.com/commenter", + body: "Can we add a regression test?", + createdAt: Date.parse("2026-04-02T13:55:00Z"), + url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-3", + }, + ], + }); + }); + + it("uses the passed parent repository identity for fork PR timelines", async () => { + const runner = createRunner([pullRequestTimelineJson()]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + await service.getPullRequestTimeline({ + cwd: "/local/fork", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(runner.calls[0]?.args).toEqual([ + "api", + "graphql", + "-f", + expect.stringContaining("repository(owner: $owner, name: $name)"), + "-F", + "owner=parentOwner", + "-F", + "name=parentRepo", + "-F", + "number=42", + ]); + }); + + it("marks PR timeline results truncated when reviews or comments hit the pagination cap", async () => { + const runner = createRunner([ + pullRequestTimelineJson({ + reviews: { + nodes: [], + pageInfo: { hasNextPage: true }, + }, + comments: { + nodes: [], + pageInfo: { hasNextPage: false }, + }, + }), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline).toMatchObject({ + items: [], + truncated: true, + error: null, + }); + }); + + it("maps PR timeline GraphQL access failures to typed internal errors", async () => { + const runner = createScriptedRunner([ + { + error: new GitHubCommandError({ + args: ["api", "graphql"], + cwd: "/repo", + exitCode: 1, + stderr: "GraphQL: Resource not accessible by integration", + }), + }, + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline).toEqual({ + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + items: [], + truncated: false, + error: { + kind: "forbidden", + message: "GraphQL: Resource not accessible by integration", + }, + }); + }); + + it("maps PR timeline missing PR failures to typed internal errors", async () => { + const runner = createScriptedRunner([ + { + error: new GitHubCommandError({ + args: ["api", "graphql"], + cwd: "/repo", + exitCode: 1, + stderr: "GraphQL: Could not resolve to a PullRequest", + }), + }, + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 404, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline.error).toEqual({ + kind: "not_found", + message: "GraphQL: Could not resolve to a PullRequest", + }); + }); + + it("does not classify unrelated not found failures as missing PR timeline errors", async () => { + const runner = createScriptedRunner([ + { + error: new GitHubCommandError({ + args: ["api", "graphql"], + cwd: "/repo", + exitCode: 1, + stderr: "fatal: remote not found", + }), + }, + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline.error).toEqual({ + kind: "unknown", + message: "fatal: remote not found", + }); + }); + + it("maps a missing gh binary during PR timeline fetch to an unknown timeline error", async () => { + const runner = createRunner([]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => null, + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(runner.calls).toHaveLength(0); + expect(timeline.error).toEqual({ + kind: "unknown", + message: "GitHub CLI (gh) is not installed or not in PATH", + }); + }); + + it("maps PR timeline authentication failures to forbidden timeline errors", async () => { + const runner = createScriptedRunner([ + { + error: new GitHubCommandError({ + args: ["api", "graphql"], + cwd: "/repo", + exitCode: 1, + stderr: "To authenticate, run: gh auth login", + }), + }, + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline.error).toEqual({ + kind: "forbidden", + message: "To authenticate, run: gh auth login", + }); + }); + + it("maps PR timeline rate limits to unknown timeline errors", async () => { + const runner = createScriptedRunner([ + { + error: new GitHubCommandError({ + args: ["api", "graphql"], + cwd: "/repo", + exitCode: 1, + stderr: "GraphQL: API rate limit exceeded for user ID 123", + }), + }, + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline.error).toEqual({ + kind: "unknown", + message: "GraphQL: API rate limit exceeded for user ID 123", + }); + }); + + it("maps PR timeline network timeouts to unknown timeline errors with runner details", async () => { + const runner = createScriptedRunner([ + { + error: Object.assign(new Error("request timed out after 10000ms"), { + code: "ETIMEDOUT", + }), + }, + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline.error).toEqual({ + kind: "unknown", + message: "request timed out after 10000ms", + }); + }); + + it("maps PR timeline JSON parse failures to unknown timeline errors", async () => { + const runner = createRunner(["{"]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const timeline = await service.getPullRequestTimeline({ + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }); + + expect(timeline.error).toMatchObject({ + kind: "unknown", + }); + expect(timeline.error?.message).toContain("JSON"); + }); + + it("caches PR timelines by cwd and PR number until invalidated", async () => { + const runner = createRunner([ + pullRequestTimelineJson({ + comments: { + nodes: [ + { + id: "IC_first", + body: "First cached result", + url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-1", + createdAt: "2026-04-02T13:55:00Z", + author: { login: "commenter", url: "https://github.com/commenter" }, + }, + ], + pageInfo: { hasNextPage: false }, + }, + }), + pullRequestTimelineJson({ + comments: { + nodes: [ + { + id: "IC_refreshed", + body: "Refreshed result", + url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-2", + createdAt: "2026-04-02T13:56:00Z", + author: { login: "commenter", url: "https://github.com/commenter" }, + }, + ], + pageInfo: { hasNextPage: false }, + }, + }), + ]); + const service = createGitHubService({ + ttlMs: 1_000, + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + const request = { + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }; + + const first = await service.getPullRequestTimeline(request); + const second = await service.getPullRequestTimeline(request); + service.invalidate({ cwd: "/repo" }); + const refreshed = await service.getPullRequestTimeline(request); + + expect(first.items.at(-1)?.body).toBe("First cached result"); + expect(second.items.at(-1)?.body).toBe("First cached result"); + expect(refreshed.items.at(-1)?.body).toBe("Refreshed result"); + expect(runner.calls).toHaveLength(2); + }); + + it("does not cache a PR timeline result that resolves after cwd invalidation", async () => { + const runner = createDeferredRunner(); + const service = createGitHubService({ + ttlMs: 1_000, + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + const request = { + cwd: "/repo", + prNumber: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + }; + + const staleRequest = service.getPullRequestTimeline(request); + await Promise.resolve(); + expect(runner.calls).toHaveLength(1); + + service.invalidate({ cwd: "/repo" }); + runner.resolveNext( + pullRequestTimelineJson({ + comments: { + nodes: [ + { + id: "IC_stale", + body: "Stale pre-invalidation result", + url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-1", + createdAt: "2026-04-02T13:55:00Z", + author: { login: "commenter", url: "https://github.com/commenter" }, + }, + ], + pageInfo: { hasNextPage: false }, + }, + }), + ); + const stale = await staleRequest; + expect(stale.items.at(-1)?.body).toBe("Stale pre-invalidation result"); + + const freshRequest = service.getPullRequestTimeline(request); + await Promise.resolve(); + expect(runner.calls).toHaveLength(2); + runner.resolveNext( + pullRequestTimelineJson({ + comments: { + nodes: [ + { + id: "IC_fresh", + body: "Fresh post-invalidation result", + url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-2", + createdAt: "2026-04-02T13:56:00Z", + author: { login: "commenter", url: "https://github.com/commenter" }, + }, + ], + pageInfo: { hasNextPage: false }, + }, + }), + ); + + const fresh = await freshRequest; + expect(fresh.items.at(-1)?.body).toBe("Fresh post-invalidation result"); + expect(runner.calls).toHaveLength(2); + }); + + it("requests and surfaces current PR number, draft state, workflow names, and formatted check durations", async () => { + const runner = createRunner([ + JSON.stringify({ + number: 42, + url: "https://github.com/acme/repo/pull/42", + title: "Wire real PR pane data", + state: "OPEN", + isDraft: true, + baseRefName: "main", + headRefName: "feature/pr-pane", + mergedAt: null, + reviewDecision: "REVIEW_REQUIRED", + statusCheckRollup: [ + { + __typename: "CheckRun", + completedAt: "2026-04-02T13:52:14Z", + conclusion: "SUCCESS", + detailsUrl: "https://github.com/acme/repo/actions/runs/123", + name: "server-tests", + startedAt: "2026-04-02T13:50:00Z", + status: "COMPLETED", + workflowName: "Server CI", + checkSuite: { + workflowRun: { + databaseId: 123, + }, + }, + }, + { + __typename: "StatusContext", + context: "deploy/preview", + state: "SUCCESS", + targetUrl: "https://github.com/acme/repo/status/preview", + createdAt: "2026-04-02T13:51:00Z", + }, + ], + }), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/pr-pane", + }); + + expect(runner.calls[0]?.args).toEqual([ + "pr", + "view", + "--json", + "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,headRepositoryOwner", + ]); + expect(status).toEqual({ + number: 42, + repoOwner: "acme", + repoName: "repo", + url: "https://github.com/acme/repo/pull/42", + title: "Wire real PR pane data", + state: "open", + baseRefName: "main", + headRefName: "feature/pr-pane", + isMerged: false, + isDraft: true, + checks: [ + { + name: "server-tests", + status: "success", + url: "https://github.com/acme/repo/actions/runs/123", + workflow: "Server CI", + duration: "2m 14s", + }, + { + name: "deploy/preview", + status: "success", + url: "https://github.com/acme/repo/status/preview", + }, + ], + checksStatus: "success", + reviewDecision: "pending", + }); + }); + + it("resolves fork PR heads to the parent repository when gh pr view returns a stale branch match", async () => { + const runner = createScriptedRunner([ + currentPullRequestJson({ + number: 7, + url: "https://github.com/parentOwner/parentRepo/pull/7", + title: "Stale tracking PR", + headRefName: "old-branch", + }), + JSON.stringify({ + owner: { login: "forkOwner" }, + name: "parentRepo", + parent: { owner: { login: "parentOwner" }, name: "parentRepo" }, + }), + JSON.stringify([ + { + number: 41, + url: "https://github.com/parentOwner/parentRepo/pull/41", + title: "Wrong fork owner", + state: "OPEN", + isDraft: false, + baseRefName: "main", + headRefName: "feature/fork", + mergedAt: null, + statusCheckRollup: [], + reviewDecision: "REVIEW_REQUIRED", + headRepositoryOwner: { login: "otherFork" }, + }, + { + number: 42, + url: "https://github.com/parentOwner/parentRepo/pull/42", + title: "Real fork PR", + state: "OPEN", + isDraft: false, + baseRefName: "main", + headRefName: "feature/fork", + mergedAt: null, + statusCheckRollup: [], + reviewDecision: "REVIEW_REQUIRED", + headRepositoryOwner: { login: "forkOwner" }, + }, + ]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + }); + + expect(status).toMatchObject({ + number: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + title: "Real fork PR", + headRefName: "feature/fork", + }); + expect(runner.calls.map((call) => call.args)).toEqual([ + [ + "pr", + "view", + "--json", + "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,headRepositoryOwner", + ], + ["repo", "view", "--json", "owner,name,parent"], + [ + "pr", + "list", + "--repo", + "parentOwner/parentRepo", + "--state", + "all", + "--head", + "forkOwner:feature/fork", + "--json", + "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,headRepositoryOwner", + "--limit", + "10", + ], + ]); + }); + + it("does not match another fork owner's PR when the current branch is main", async () => { + const calls: RunnerCall[] = []; + const runner: GitHubCommandRunner = async (args, options) => { + calls.push({ args, cwd: options.cwd }); + if (args[0] === "pr" && args[1] === "view") { + throw noPullRequestError(args); + } + if (args[0] === "pr" && args[1] === "list" && args.includes("--head")) { + return { + stdout: JSON.stringify([ + { + number: 77, + url: "https://github.com/parentOwner/parentRepo/pull/77", + title: "Unrelated fork main branch", + state: "OPEN", + isDraft: false, + baseRefName: "main", + headRefName: "main", + mergedAt: null, + statusCheckRollup: [], + reviewDecision: "REVIEW_REQUIRED", + headRepositoryOwner: { login: "otherForkOwner" }, + }, + ]), + stderr: "", + }; + } + return { + stdout: JSON.stringify({ + owner: { login: "repoOwner" }, + name: "repo", + parent: null, + }), + stderr: "", + }; + }; + const service = createGitHubService({ + runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + await expect( + service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "main", + }), + ).resolves.toBeNull(); + expect(calls.map((call) => call.args)).toEqual([ + [ + "pr", + "view", + "--json", + "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,headRepositoryOwner", + ], + ["repo", "view", "--json", "owner,name,parent"], + ]); + }); + + it("finds a fork PR in the parent repo when the direct current branch view is unavailable", async () => { + const runner = createScriptedRunner([ + { error: noPullRequestError() }, + JSON.stringify({ + owner: { login: "forkOwner" }, + name: "repo", + parent: { owner: { login: "parentOwner" }, name: "repo" }, + }), + JSON.stringify([ + { + number: 88, + url: "https://github.com/parentOwner/repo/pull/88", + title: "Fork-only PR", + state: "OPEN", + isDraft: false, + baseRefName: "main", + headRefName: "feature/fork", + mergedAt: null, + statusCheckRollup: [], + reviewDecision: null, + headRepositoryOwner: { login: "forkOwner" }, + }, + ]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + }); + + expect(status).toMatchObject({ + number: 88, + repoOwner: "parentOwner", + repoName: "repo", + headRefName: "feature/fork", + }); + expect(runner.calls[2]?.args).toContain("forkOwner:feature/fork"); + }); + + it("propagates DNS errors while resolving the current PR view", async () => { + const dnsError = new GitHubCommandError({ + args: ["pr", "view"], + cwd: "/repo", + exitCode: 1, + stderr: "could not resolve host: github.com", + }); + const runner = createScriptedRunner([{ error: dnsError }]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + await expect( + service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/pr-pane", + }), + ).rejects.toBe(dnsError); + }); + + it("returns null when no current branch PR is matched by view or qualified fork lookup", async () => { + const runner = createScriptedRunner([ + { error: noPullRequestError() }, + JSON.stringify({ + owner: { login: "forkOwner" }, + name: "repo", + parent: { owner: { login: "parentOwner" }, name: "repo" }, + }), + "[]", + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + await expect( + service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/missing", + }), + ).resolves.toBeNull(); + }); + + it("keeps S1 PR status schema additions optional and strips internal check timestamps", () => { + const oldDaemonResponse = CheckoutPrStatusResponseSchema.parse({ + type: "checkout_pr_status_response", + payload: { + cwd: "/repo", + status: { + url: "https://github.com/acme/repo/pull/42", + title: "Old daemon payload", + state: "open", + baseRefName: "main", + headRefName: "feature", + isMerged: false, + }, + githubFeaturesEnabled: true, + error: null, + requestId: "req-old", + }, + }); + expect(oldDaemonResponse.payload.status).toMatchObject({ + isDraft: false, + checks: [], + }); + + const newDaemonResponse = CheckoutPrStatusResponseSchema.parse({ + type: "checkout_pr_status_response", + payload: { + cwd: "/repo", + status: { + number: 42, + url: "https://github.com/acme/repo/pull/42", + title: "New daemon payload", + state: "open", + baseRefName: "main", + headRefName: "feature", + isMerged: false, + isDraft: true, + checks: [ + { + name: "server-tests", + status: "success", + url: "https://github.com/acme/repo/actions/runs/123", + workflow: "Server CI", + duration: "2m 14s", + startedAt: "2026-04-02T13:50:00Z", + completedAt: "2026-04-02T13:52:14Z", + workflowRunDatabaseId: 123, + }, + ], + checksStatus: "success", + reviewDecision: "pending", + }, + githubFeaturesEnabled: true, + error: null, + requestId: "req-new", + }, + }); + + expect(newDaemonResponse.payload.status).toEqual({ + number: 42, + url: "https://github.com/acme/repo/pull/42", + title: "New daemon payload", + state: "open", + baseRefName: "main", + headRefName: "feature", + isMerged: false, + isDraft: true, + checks: [ + { + name: "server-tests", + status: "success", + url: "https://github.com/acme/repo/actions/runs/123", + workflow: "Server CI", + duration: "2m 14s", + }, + ], + checksStatus: "success", + reviewDecision: "pending", + }); + }); + it("returns cached results for identical calls within the TTL", async () => { const runner = createRunner([pullRequestJson("First result")]); const service = createGitHubService({ @@ -133,6 +1119,7 @@ describe("GitHubService", () => { baseRefName: "main", headRefName: "feature", labels: ["bug"], + updatedAt: "", }, ], [ @@ -145,6 +1132,7 @@ describe("GitHubService", () => { baseRefName: "main", headRefName: "feature", labels: ["bug"], + updatedAt: "", }, ], ]); diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 5d6a42f63..8e44aad81 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -38,6 +38,7 @@ const GitHubPullRequestSummarySchema = z.object({ const PullRequestCheckRunNodeSchema = z.object({ __typename: z.literal("CheckRun"), name: z.string(), + workflowName: z.string().nullable().optional(), conclusion: z.string().nullable().optional(), status: z.string().nullable().optional(), detailsUrl: z.string().nullable().optional(), @@ -79,17 +80,151 @@ const PullRequestReviewDecisionSchema = z .nullable() .catch(null); +const HeadRepositoryOwnerSchema = z + .object({ + login: z.string().optional(), + }) + .nullable() + .optional(); + const CurrentPullRequestStatusSchema = z.object({ + number: z.number().optional(), url: z.string().catch(""), title: z.string().catch(""), state: z.string().catch(""), + isDraft: z.boolean().optional().catch(false), baseRefName: z.string().catch(""), headRefName: z.string().catch(""), mergedAt: z.string().nullable().optional(), statusCheckRollup: z.unknown().optional(), reviewDecision: z.unknown().optional(), + headRepositoryOwner: HeadRepositoryOwnerSchema, }); +const TimelineAuthorSchema = z + .object({ + login: z.string().optional(), + url: z.string().nullable().optional(), + }) + .nullable() + .optional(); + +const PullRequestTimelineReviewNodeSchema = z.object({ + id: z.string().catch(""), + state: z.string().catch(""), + body: z.string().nullable().catch(null), + url: z.string().catch(""), + submittedAt: z.string().nullable().catch(null), + author: TimelineAuthorSchema, +}); + +const PullRequestTimelineCommentNodeSchema = z.object({ + id: z.string().catch(""), + body: z.string().nullable().catch(null), + url: z.string().catch(""), + createdAt: z.string().nullable().catch(null), + author: TimelineAuthorSchema, +}); + +const PullRequestTimelinePageInfoSchema = z.object({ + hasNextPage: z.boolean().catch(false), +}); + +const PullRequestTimelineGraphqlSchema = z.object({ + data: z + .object({ + repository: z + .object({ + pullRequest: z + .object({ + number: z.number().optional(), + reviews: z + .object({ + nodes: z.array(PullRequestTimelineReviewNodeSchema).catch([]), + pageInfo: PullRequestTimelinePageInfoSchema.catch({ hasNextPage: false }), + }) + .catch({ nodes: [], pageInfo: { hasNextPage: false } }), + comments: z + .object({ + nodes: z.array(PullRequestTimelineCommentNodeSchema).catch([]), + pageInfo: PullRequestTimelinePageInfoSchema.catch({ hasNextPage: false }), + }) + .catch({ nodes: [], pageInfo: { hasNextPage: false } }), + }) + .nullable() + .optional(), + }) + .nullable() + .optional(), + }) + .optional(), +}); + +const GitHubRepoViewSchema = z.object({ + owner: z + .object({ + login: z.string().optional(), + }) + .nullable() + .optional(), + name: z.string().optional(), + parent: z + .object({ + owner: z + .object({ + login: z.string().optional(), + }) + .nullable() + .optional(), + name: z.string().optional(), + }) + .nullable() + .optional(), +}); + +const CURRENT_PR_STATUS_FIELDS = + "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,headRepositoryOwner"; + +const PULL_REQUEST_TIMELINE_QUERY = ` +query PullRequestTimeline($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + number + reviews(first: 100) { + nodes { + id + state + body + url + submittedAt + author { + login + url + } + } + pageInfo { + hasNextPage + } + } + comments(first: 100) { + nodes { + id + body + url + createdAt + author { + login + url + } + } + pageInfo { + hasNextPage + } + } + } + } +}`; + interface CacheEntry { value: unknown; expiresAt: number; @@ -144,23 +279,65 @@ export interface PullRequestCheck { name: string; status: PullRequestCheckStatus; url: string | null; + workflow?: string; + duration?: string; } export type PullRequestChecksStatus = "none" | "pending" | "success" | "failure"; export type PullRequestReviewDecision = "approved" | "changes_requested" | "pending" | null; export interface GitHubCurrentPullRequestStatus { + number?: number; + repoOwner?: string; + repoName?: string; url: string; title: string; state: string; baseRefName: string; headRefName: string; isMerged: boolean; + isDraft?: boolean; checks: PullRequestCheck[]; checksStatus: PullRequestChecksStatus; reviewDecision: PullRequestReviewDecision; } +export type PullRequestTimelineReviewState = "approved" | "changes_requested" | "commented"; + +interface PullRequestTimelineItemBase { + id: string; + author: string; + authorUrl: string | null; + body: string; + createdAt: number; + url: string; +} + +export type PullRequestTimelineItem = + | (PullRequestTimelineItemBase & { + kind: "review"; + reviewState: PullRequestTimelineReviewState; + }) + | (PullRequestTimelineItemBase & { + kind: "comment"; + }); + +export type GitHubPullRequestTimelineErrorKind = "not_found" | "forbidden" | "unknown"; + +export interface GitHubPullRequestTimelineError { + kind: GitHubPullRequestTimelineErrorKind; + message: string; +} + +export interface GitHubPullRequestTimeline { + prNumber: number; + repoOwner: string; + repoName: string; + items: PullRequestTimelineItem[]; + truncated: boolean; + error: GitHubPullRequestTimelineError | null; +} + export interface GitHubPullRequestCreateResult { url: string; number: number; @@ -183,6 +360,13 @@ export interface GetGitHubPullRequestOptions { number: number; } +export interface GetGitHubPullRequestTimelineOptions { + cwd: string; + prNumber: number; + repoOwner: string; + repoName: string; +} + export interface CreateGitHubPullRequestOptions { cwd: string; repo: string; @@ -201,6 +385,9 @@ export interface GitHubService { cwd: string; headRef: string; }): Promise; + getPullRequestTimeline( + options: GetGitHubPullRequestTimelineOptions, + ): Promise; createPullRequest( options: CreateGitHubPullRequestOptions, ): Promise; @@ -261,6 +448,17 @@ interface CommandFailureLike { type PullRequestCheckRunNode = z.infer; type PullRequestStatusContextNode = z.infer; +type CurrentPullRequestStatusItem = z.infer; + +interface InFlightCacheEntry { + cwd: string; + promise: Promise; +} + +interface ResolvedPullRequestCandidate { + status: GitHubCurrentPullRequestStatus; + headRepositoryOwner?: string; +} export function createGitHubService(options: CreateGitHubServiceOptions = {}): GitHubService { const ttlMs = options.ttlMs ?? DEFAULT_GITHUB_CACHE_TTL_MS; @@ -270,7 +468,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G now: options.now ?? Date.now, }; const cache = new Map(); - const inFlight = new Map>(); + const inFlight = new Map(); async function cached(params: { cwd: string; @@ -291,23 +489,27 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G const existing = inFlight.get(key); if (existing) { - return existing as Promise; + return existing.promise as Promise; } const request = params .load() .then((value) => { - cache.set(key, { - value, - cwd: params.cwd, - expiresAt: deps.now() + ttlMs, - }); + if (inFlight.get(key)?.promise === request) { + cache.set(key, { + value, + cwd: params.cwd, + expiresAt: deps.now() + ttlMs, + }); + } return value; }) .finally(() => { - inFlight.delete(key); + if (inFlight.get(key)?.promise === request) { + inFlight.delete(key); + } }); - inFlight.set(key, request); + inFlight.set(key, { cwd: params.cwd, promise: request }); return request; } @@ -407,23 +609,52 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G cwd: options.cwd, method: "getCurrentPullRequestStatus", args: { headRef: options.headRef }, + load: async () => { + return resolveCurrentPullRequestView({ + cwd: options.cwd, + headRef: options.headRef, + run, + }); + }, + }); + }, + + getPullRequestTimeline(options) { + return cached({ + cwd: options.cwd, + method: "getPullRequestTimeline", + args: { prNumber: options.prNumber }, load: async () => { try { const stdout = await run( [ - "pr", - "view", - "--json", - "url,title,state,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision", + "api", + "graphql", + "-f", + `query=${PULL_REQUEST_TIMELINE_QUERY}`, + "-F", + `owner=${options.repoOwner}`, + "-F", + `name=${options.repoName}`, + "-F", + `number=${options.prNumber}`, ], { cwd: options.cwd }, ); - return parseCurrentPullRequestStatus(stdout, options.headRef); + return parsePullRequestTimeline(stdout, { + prNumber: options.prNumber, + repoOwner: options.repoOwner, + repoName: options.repoName, + }); } catch (error) { - if (isNoPullRequestFoundError(error)) { - return null; - } - throw error; + return { + prNumber: options.prNumber, + repoOwner: options.repoOwner, + repoName: options.repoName, + items: [], + truncated: false, + error: mapPullRequestTimelineError(error), + }; } }, }); @@ -476,11 +707,18 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G }, invalidate(options) { + // Local checkout mutations that can alter the current PR identity or PR status + // must call this with the affected cwd before broadcasting fresh git state. for (const [key, entry] of cache.entries()) { if (entry.cwd === options.cwd) { cache.delete(key); } } + for (const [key, entry] of inFlight.entries()) { + if (entry.cwd === options.cwd) { + inFlight.delete(key); + } + } }, }; } @@ -541,14 +779,15 @@ function normalizeGitHubCommandError( return new GitHubCliMissingError(); } const stderr = bufferOrStringToString(failure.stderr); - if (isAuthFailureText(stderr) || isAuthFailureText(failure.message ?? "")) { + const message = failure.message ?? ""; + if (isAuthFailureText(stderr) || isAuthFailureText(message)) { return new GitHubAuthenticationError({ stderr }); } return new GitHubCommandError({ args: context.args, cwd: context.cwd, exitCode: typeof failure.code === "number" ? failure.code : null, - stderr, + stderr: stderr || message, }); } @@ -602,7 +841,178 @@ function isNoPullRequestFoundError(error: unknown): boolean { return false; } const text = error.stderr.toLowerCase(); - return text.includes("no pull requests found") || text.includes("could not resolve"); + return text.includes("no pull requests found"); +} + +async function resolveCurrentPullRequestView(options: { + cwd: string; + headRef: string; + run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; +}): Promise { + const viewCandidate = await tryCurrentPullRequestView(options); + if (viewCandidate && isCandidateForHeadRef(viewCandidate, options.headRef)) { + return viewCandidate.status; + } + + const repo = await getGitHubRepoView(options); + const forkOwner = repo?.owner?.login; + const parentOwner = repo?.parent?.owner?.login; + const parentName = repo?.parent?.name; + if (!forkOwner || !parentOwner || !parentName) { + return null; + } + + const parentCandidates = await listCurrentPullRequestCandidates({ + cwd: options.cwd, + headRef: `${forkOwner}:${options.headRef}`, + run: options.run, + repo: `${parentOwner}/${parentName}`, + }); + const parentMatch = pickPullRequestCandidate({ + candidates: parentCandidates, + headRef: options.headRef, + headRepositoryOwner: forkOwner, + }); + return parentMatch?.status ?? null; +} + +async function tryCurrentPullRequestView(options: { + cwd: string; + headRef: string; + run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; +}): Promise { + try { + const stdout = await options.run(["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], { + cwd: options.cwd, + }); + return parseCurrentPullRequestCandidate(stdout, options.headRef); + } catch (error) { + if (isNoPullRequestFoundError(error)) { + return null; + } + throw error; + } +} + +async function listCurrentPullRequestCandidates(options: { + cwd: string; + headRef: string; + run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; + repo?: string; +}): Promise { + const args = ["pr", "list"]; + if (options.repo) { + args.push("--repo", options.repo); + } + args.push( + "--state", + "all", + "--head", + options.headRef, + "--json", + CURRENT_PR_STATUS_FIELDS, + "--limit", + "10", + ); + try { + const stdout = await options.run(args, { cwd: options.cwd }); + return parseCurrentPullRequestCandidateList(stdout, options.headRef); + } catch (error) { + if (isNoPullRequestFoundError(error)) { + return []; + } + throw error; + } +} + +async function getGitHubRepoView(options: { + cwd: string; + run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; +}): Promise | null> { + try { + const stdout = await options.run(["repo", "view", "--json", "owner,name,parent"], { + cwd: options.cwd, + }); + return GitHubRepoViewSchema.parse(JSON.parse(stdout || "{}")); + } catch { + return null; + } +} + +function parseCurrentPullRequestCandidate( + stdout: string, + fallbackHeadRefName: string, +): ResolvedPullRequestCandidate | null { + const item = CurrentPullRequestStatusSchema.parse(JSON.parse(stdout || "{}")); + return toCurrentPullRequestCandidate(item, fallbackHeadRefName); +} + +function parseCurrentPullRequestCandidateList( + stdout: string, + fallbackHeadRefName: string, +): ResolvedPullRequestCandidate[] { + const items = z.array(CurrentPullRequestStatusSchema).parse(JSON.parse(stdout || "[]")); + return items + .map((item) => toCurrentPullRequestCandidate(item, fallbackHeadRefName)) + .filter((candidate): candidate is ResolvedPullRequestCandidate => candidate !== null); +} + +function toCurrentPullRequestCandidate( + item: CurrentPullRequestStatusItem, + fallbackHeadRefName: string, +): ResolvedPullRequestCandidate | null { + const status = toCurrentPullRequestStatus(item, fallbackHeadRefName); + if (!status) { + return null; + } + const headRepositoryOwner = item.headRepositoryOwner?.login; + return { + status, + ...(headRepositoryOwner ? { headRepositoryOwner } : {}), + }; +} + +function isCandidateForHeadRef(candidate: ResolvedPullRequestCandidate, headRef: string): boolean { + return candidate.status.headRefName === headRef && hasResolvedRepoIdentity(candidate.status); +} + +function hasResolvedRepoIdentity(status: GitHubCurrentPullRequestStatus): boolean { + return Boolean(status.repoOwner && status.repoName); +} + +function pickPullRequestCandidate(options: { + candidates: ResolvedPullRequestCandidate[]; + headRef: string; + headRepositoryOwner?: string; +}): ResolvedPullRequestCandidate | null { + const matching = options.candidates.filter((candidate) => { + if (!isCandidateForHeadRef(candidate, options.headRef)) { + return false; + } + if (!options.headRepositoryOwner) { + return true; + } + return candidate.headRepositoryOwner === options.headRepositoryOwner; + }); + matching.sort(comparePullRequestCandidatePreference); + return matching[0] ?? null; +} + +function comparePullRequestCandidatePreference( + left: ResolvedPullRequestCandidate, + right: ResolvedPullRequestCandidate, +): number { + return getPullRequestStateRank(left.status) - getPullRequestStateRank(right.status); +} + +function getPullRequestStateRank(status: GitHubCurrentPullRequestStatus): number { + if (status.state === "open" || status.isDraft) { + return 0; + } + if (status.state === "merged") { + return 1; + } + return 2; } function parsePullRequestSummaries(stdout: string): GitHubPullRequestSummary[] { @@ -626,6 +1036,7 @@ function toPullRequestSummary( baseRefName: item.baseRefName, headRefName: item.headRefName, labels: item.labels.map((label) => label.name ?? "").filter((name) => name.length > 0), + updatedAt: item.updatedAt, }; } @@ -638,35 +1049,185 @@ function parseIssueSummaries(stdout: string): GitHubIssueSummary[] { state: item.state, body: item.body, labels: item.labels.map((label) => label.name ?? "").filter((name) => name.length > 0), + updatedAt: item.updatedAt, })); } -function parseCurrentPullRequestStatus( +function parsePullRequestTimeline( stdout: string, + identity: { prNumber: number; repoOwner: string; repoName: string }, +): GitHubPullRequestTimeline { + const parsed = PullRequestTimelineGraphqlSchema.parse(JSON.parse(stdout || "{}")); + const pullRequest = parsed.data?.repository?.pullRequest; + const items = pullRequest + ? [ + ...pullRequest.reviews.nodes.flatMap(toPullRequestTimelineReviewItem), + ...pullRequest.comments.nodes.map(toPullRequestTimelineCommentItem), + ].sort(compareTimelineItems) + : []; + return { + prNumber: pullRequest?.number ?? identity.prNumber, + repoOwner: identity.repoOwner, + repoName: identity.repoName, + items, + // S3 deliberately caps timeline fetches at the first 100 reviews and first 100 comments. + truncated: Boolean( + pullRequest?.reviews.pageInfo.hasNextPage || pullRequest?.comments.pageInfo.hasNextPage, + ), + error: pullRequest ? null : { kind: "not_found", message: "Pull request not found" }, + }; +} + +function toPullRequestTimelineReviewItem( + review: z.infer, +): PullRequestTimelineItem[] { + const reviewState = mapTimelineReviewState(review.state, review.body ?? ""); + if (!reviewState) { + return []; + } + return [ + { + kind: "review", + id: review.id, + author: review.author?.login ?? "unknown", + authorUrl: review.author?.url ?? null, + body: review.body ?? "", + createdAt: parseOptionalTime(review.submittedAt ?? null), + url: review.url, + reviewState, + }, + ]; +} + +function toPullRequestTimelineCommentItem( + comment: z.infer, +): PullRequestTimelineItem { + return { + kind: "comment", + id: comment.id, + author: comment.author?.login ?? "unknown", + authorUrl: comment.author?.url ?? null, + body: comment.body ?? "", + createdAt: parseOptionalTime(comment.createdAt ?? null), + url: comment.url, + }; +} + +function mapTimelineReviewState( + state: string, + body: string, +): PullRequestTimelineReviewState | null { + switch (state) { + case "APPROVED": + return "approved"; + case "CHANGES_REQUESTED": + return "changes_requested"; + case "COMMENTED": + return "commented"; + case "DISMISSED": + case "PENDING": + return body.trim().length > 0 ? "commented" : null; + default: + return body.trim().length > 0 ? "commented" : null; + } +} + +function compareTimelineItems( + left: PullRequestTimelineItem, + right: PullRequestTimelineItem, +): number { + if (left.createdAt !== right.createdAt) { + return left.createdAt - right.createdAt; + } + return left.id.localeCompare(right.id); +} + +function mapPullRequestTimelineError(error: unknown): GitHubPullRequestTimelineError { + if (error instanceof GitHubCommandError) { + return { + kind: classifyPullRequestTimelineError(error.stderr), + message: error.stderr || error.message, + }; + } + if (error instanceof GitHubAuthenticationError) { + return { + kind: "forbidden", + message: error.stderr || error.message, + }; + } + return { + kind: "unknown", + message: error instanceof Error ? error.message : String(error), + }; +} + +function classifyPullRequestTimelineError(stderr: string): GitHubPullRequestTimelineErrorKind { + const normalized = stderr.toLowerCase(); + if ( + normalized.includes("could not resolve to a pullrequest") || + normalized.includes("pull request not found") || + normalized.includes("pullrequest not found") + ) { + return "not_found"; + } + if ( + normalized.includes("forbidden") || + normalized.includes("resource not accessible") || + normalized.includes("permission") || + normalized.includes("access denied") || + normalized.includes("requires authentication") || + normalized.includes("http 403") + ) { + return "forbidden"; + } + return "unknown"; +} + +function toCurrentPullRequestStatus( + item: CurrentPullRequestStatusItem, fallbackHeadRefName: string, ): GitHubCurrentPullRequestStatus | null { - const item = CurrentPullRequestStatusSchema.parse(JSON.parse(stdout || "{}")); if (!item.url || !item.title) { return null; } + const repoIdentity = parseGitHubPullRequestRepo(item.url); const mergedAt = typeof item.mergedAt === "string" && item.mergedAt.trim().length > 0 ? item.mergedAt : null; const state = mergedAt !== null ? "merged" : item.state.trim().length > 0 ? item.state.toLowerCase() : ""; const checks = parseStatusCheckRollup(item.statusCheckRollup); return { + ...(typeof item.number === "number" ? { number: item.number } : {}), + ...(repoIdentity ? { repoOwner: repoIdentity.owner, repoName: repoIdentity.name } : {}), url: item.url, title: item.title, state, baseRefName: item.baseRefName, headRefName: item.headRefName || fallbackHeadRefName, isMerged: mergedAt !== null, + isDraft: item.isDraft ?? false, checks, checksStatus: computeChecksStatus(checks), reviewDecision: mapReviewDecision(item.reviewDecision), }; } +function parseGitHubPullRequestRepo(url: string): { owner: string; name: string } | null { + try { + const parsed = new URL(url); + if (parsed.hostname !== "github.com") { + return null; + } + const [owner, name, kind] = parsed.pathname.split("/").filter(Boolean); + if (!owner || !name || kind !== "pull") { + return null; + } + return { owner, name }; + } catch { + return null; + } +} + export function parseStatusCheckRollup(value: unknown): PullRequestCheck[] { const directContexts = PullRequestStatusCheckRollupArraySchema.safeParse(value); if (!directContexts.success) { @@ -704,6 +1265,10 @@ function buildPullRequestCheck( name: context.name, status: mapCheckRunStatus(context.status, context.conclusion), url: typeof context.detailsUrl === "string" ? context.detailsUrl : null, + ...(typeof context.workflowName === "string" && context.workflowName.trim().length > 0 + ? { workflow: context.workflowName } + : {}), + ...formatCheckRunDuration(context), recency: getCheckRunRecency(context), }; } @@ -762,6 +1327,33 @@ function getCheckRunRecency(context: PullRequestCheckRunNode): number { return parseOptionalTime(context.completedAt ?? context.startedAt ?? null); } +function formatCheckRunDuration(context: PullRequestCheckRunNode): { duration?: string } { + const startedAt = parseOptionalTime(context.startedAt ?? null); + const completedAt = parseOptionalTime(context.completedAt ?? null); + if (startedAt <= 0 || completedAt <= 0 || completedAt < startedAt) { + return {}; + } + const durationSeconds = Math.floor((completedAt - startedAt) / 1_000); + return { duration: formatDurationSeconds(durationSeconds) }; +} + +function formatDurationSeconds(totalSeconds: number): string { + const hours = Math.floor(totalSeconds / 3_600); + const minutes = Math.floor((totalSeconds % 3_600) / 60); + const seconds = totalSeconds % 60; + const parts: string[] = []; + if (hours > 0) { + parts.push(`${hours}h`); + } + if (minutes > 0) { + parts.push(`${minutes}m`); + } + if (seconds > 0 || parts.length === 0) { + parts.push(`${seconds}s`); + } + return parts.join(" "); +} + function getStatusContextRecency(context: PullRequestStatusContextNode): number { return parseOptionalTime(context.createdAt ?? null); } diff --git a/packages/server/src/shared/messages.pull-request-timeline.test.ts b/packages/server/src/shared/messages.pull-request-timeline.test.ts new file mode 100644 index 000000000..804f4ea2f --- /dev/null +++ b/packages/server/src/shared/messages.pull-request-timeline.test.ts @@ -0,0 +1,282 @@ +import { describe, expect, test } from "vitest"; +import { z } from "zod"; +import { + CheckoutPrStatusResponseSchema, + PullRequestTimelineRequestSchema, + PullRequestTimelineResponseSchema, + SessionInboundMessageSchema, + SessionOutboundMessageSchema, +} from "./messages.js"; + +describe("pull request timeline message schemas", () => { + test("requires request identity fields", () => { + expect(() => + PullRequestTimelineRequestSchema.parse({ + type: "pull_request_timeline_request", + }), + ).toThrow(); + }); + + test("parses request fields", () => { + const parsed = PullRequestTimelineRequestSchema.parse({ + type: "pull_request_timeline_request", + cwd: "/tmp/repo", + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + requestId: "request-1", + }); + + expect(parsed).toEqual({ + type: "pull_request_timeline_request", + cwd: "/tmp/repo", + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + requestId: "request-1", + }); + }); + + test("parses request through the inbound message union", () => { + const parsed = SessionInboundMessageSchema.parse({ + type: "pull_request_timeline_request", + cwd: "/tmp/repo", + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + requestId: "request-1", + }); + + expect(parsed).toEqual({ + type: "pull_request_timeline_request", + cwd: "/tmp/repo", + prNumber: 42, + repoOwner: "getpaseo", + repoName: "paseo", + requestId: "request-1", + }); + }); + + test("defaults optional response payload and malformed timeline item fields", () => { + const parsed = PullRequestTimelineResponseSchema.parse({ + type: "pull_request_timeline_response", + payload: { + items: [{}, { kind: "review" }], + }, + }); + + expect(parsed).toEqual({ + type: "pull_request_timeline_response", + payload: { + cwd: "", + prNumber: null, + items: [ + { + id: "", + kind: "comment", + author: "unknown", + body: "", + createdAt: 0, + url: "", + }, + { + id: "", + kind: "review", + author: "unknown", + body: "", + createdAt: 0, + url: "", + reviewState: "commented", + }, + ], + truncated: false, + error: null, + requestId: "", + githubFeaturesEnabled: true, + }, + }); + }); + + test("normalizes unknown timeline item kinds to comments", () => { + const parsed = PullRequestTimelineResponseSchema.parse({ + type: "pull_request_timeline_response", + payload: { + items: [ + { + id: "future-1", + kind: "unknown_future_kind", + author: "octocat", + body: "Future daemon item", + createdAt: 1710000000000, + url: "https://github.com/getpaseo/paseo/pull/42#future-1", + }, + ], + }, + }); + + expect(parsed.payload.items).toEqual([ + { + id: "future-1", + kind: "comment", + author: "octocat", + body: "Future daemon item", + createdAt: 1710000000000, + url: "https://github.com/getpaseo/paseo/pull/42#future-1", + }, + ]); + }); + + test("parses response through the outbound message union", () => { + const parsed = SessionOutboundMessageSchema.parse({ + type: "pull_request_timeline_response", + payload: { + cwd: "/tmp/repo", + prNumber: 42, + items: [ + { + id: "review-1", + kind: "review", + author: "octocat", + body: "Looks good", + createdAt: 1710000000000, + url: "https://github.com/getpaseo/paseo/pull/42#pullrequestreview-1", + reviewState: "approved", + }, + { + id: "comment-1", + kind: "comment", + author: "hubot", + body: "Left a note", + createdAt: 1710000001000, + url: "https://github.com/getpaseo/paseo/pull/42#issuecomment-1", + }, + ], + truncated: true, + error: { kind: "unknown", message: "rate limited" }, + requestId: "request-1", + githubFeaturesEnabled: true, + }, + }); + + expect(parsed.type).toBe("pull_request_timeline_response"); + }); + + test("normalizes future error kinds to unknown", () => { + const parsed = PullRequestTimelineResponseSchema.parse({ + type: "pull_request_timeline_response", + payload: { + error: { kind: "future_kind", message: "x" }, + }, + }); + + expect(parsed.payload.error).toEqual({ kind: "unknown", message: "x" }); + }); + + test.each([ + "rate_limited", + "", + undefined, + ])("normalizes malformed error kind %j to unknown", (kind) => { + const parsed = PullRequestTimelineResponseSchema.parse({ + type: "pull_request_timeline_response", + payload: { + error: { kind }, + }, + }); + + expect(parsed.payload.error).toEqual({ kind: "unknown", message: "" }); + }); +}); + +describe("checkout PR status compatibility", () => { + test("an old client schema parses a new daemon checkout PR status response", () => { + const oldClientCheckoutPrStatusResponseSchema = z.object({ + type: z.literal("checkout_pr_status_response"), + payload: z.object({ + cwd: z.string(), + status: z + .object({ + url: z.string(), + title: z.string(), + state: z.string(), + baseRefName: z.string(), + headRefName: z.string(), + isMerged: z.boolean(), + checks: z + .array( + z.object({ + name: z.string(), + status: z.string(), + url: z.string().nullable(), + }), + ) + .optional() + .default([]), + checksStatus: z.string().optional(), + reviewDecision: z.string().nullable().optional(), + }) + .nullable(), + githubFeaturesEnabled: z.boolean(), + error: z.unknown().nullable(), + requestId: z.string(), + }), + }); + const newDaemonPayload = CheckoutPrStatusResponseSchema.parse({ + type: "checkout_pr_status_response", + payload: { + cwd: "/tmp/repo", + status: { + number: 42, + url: "https://github.com/getpaseo/paseo/pull/42", + title: "Wire real PR pane data", + state: "OPEN", + baseRefName: "main", + headRefName: "feature/pr-pane", + isMerged: false, + isDraft: true, + checks: [ + { + name: "server-tests", + status: "success", + url: "https://github.com/getpaseo/paseo/actions/runs/123", + workflow: "Server CI", + duration: "2m 14s", + }, + ], + checksStatus: "success", + reviewDecision: "APPROVED", + }, + githubFeaturesEnabled: true, + error: null, + requestId: "request-compat", + }, + }); + + expect(oldClientCheckoutPrStatusResponseSchema.parse(newDaemonPayload)).toEqual({ + type: "checkout_pr_status_response", + payload: { + cwd: "/tmp/repo", + status: { + url: "https://github.com/getpaseo/paseo/pull/42", + title: "Wire real PR pane data", + state: "OPEN", + baseRefName: "main", + headRefName: "feature/pr-pane", + isMerged: false, + checks: [ + { + name: "server-tests", + status: "success", + url: "https://github.com/getpaseo/paseo/actions/runs/123", + }, + ], + checksStatus: "success", + reviewDecision: "APPROVED", + }, + githubFeaturesEnabled: true, + error: null, + requestId: "request-compat", + }, + }); + }); +}); diff --git a/packages/server/src/shared/messages.ts b/packages/server/src/shared/messages.ts index faa788577..edb6bdeeb 100644 --- a/packages/server/src/shared/messages.ts +++ b/packages/server/src/shared/messages.ts @@ -1133,6 +1133,15 @@ export const CheckoutPrStatusRequestSchema = z.object({ requestId: z.string(), }); +export const PullRequestTimelineRequestSchema = z.object({ + type: z.literal("pull_request_timeline_request"), + cwd: z.string(), + prNumber: z.number(), + repoOwner: z.string(), + repoName: z.string(), + requestId: z.string(), +}); + export const ValidateBranchRequestSchema = z.object({ type: z.literal("validate_branch_request"), cwd: z.string(), @@ -1554,6 +1563,7 @@ export const SessionInboundMessageSchema = z.discriminatedUnion("type", [ CheckoutPushRequestSchema, CheckoutPrCreateRequestSchema, CheckoutPrStatusRequestSchema, + PullRequestTimelineRequestSchema, CheckoutSwitchBranchRequestSchema, StashSaveRequestSchema, StashPopRequestSchema, @@ -2505,24 +2515,31 @@ export const CheckoutPrCreateResponseSchema = z.object({ }), }); -const CheckoutPrStatusSchema = z.object({ +export const CheckoutPrStatusSchema = z.object({ + number: z.number().optional(), url: z.string(), title: z.string(), state: z.string(), baseRefName: z.string(), headRefName: z.string(), isMerged: z.boolean(), + isDraft: z.boolean().optional().default(false), checks: z .array( z.object({ name: z.string(), status: z.string(), url: z.string().nullable(), + workflow: z.string().optional(), + duration: z.string().optional(), }), ) - .optional(), + .optional() + .default([]), checksStatus: z.string().optional(), reviewDecision: z.string().nullable().optional(), + repoOwner: z.string().optional(), + repoName: z.string().optional(), }); export const CheckoutPrStatusResponseSchema = z.object({ @@ -2536,6 +2553,87 @@ export const CheckoutPrStatusResponseSchema = z.object({ }), }); +const PullRequestTimelineKnownErrorSchema = z.discriminatedUnion("kind", [ + z.object({ + kind: z.literal("not_found"), + message: z.string().optional().default(""), + }), + z.object({ + kind: z.literal("forbidden"), + message: z.string().optional().default(""), + }), + z.object({ + kind: z.literal("unknown"), + message: z.string().optional().default(""), + }), +]); + +const PullRequestTimelineErrorSchema = z.preprocess((value) => { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return { kind: "unknown", message: "" }; + } + const error = value as Record; + if (error.kind === "not_found" || error.kind === "forbidden" || error.kind === "unknown") { + return error; + } + return { ...error, kind: "unknown" }; +}, PullRequestTimelineKnownErrorSchema); + +const PullRequestTimelineReviewItemSchema = z.object({ + id: z.string().optional().default(""), + kind: z.literal("review"), + author: z.string().optional().default("unknown"), + body: z.string().optional().default(""), + createdAt: z.number().optional().default(0), + url: z.string().optional().default(""), + reviewState: z + .enum(["approved", "changes_requested", "commented"]) + .optional() + .default("commented"), +}); + +const PullRequestTimelineCommentItemSchema = z.object({ + id: z.string().optional().default(""), + kind: z.literal("comment"), + author: z.string().optional().default("unknown"), + body: z.string().optional().default(""), + createdAt: z.number().optional().default(0), + url: z.string().optional().default(""), +}); + +export const PullRequestTimelineItemSchema = z.preprocess( + (value) => { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return value; + } + const item = value as Record; + if (item.kind === "review" || item.kind === "comment") { + return item; + } + return { ...item, kind: "comment" }; + }, + z.discriminatedUnion("kind", [ + PullRequestTimelineReviewItemSchema, + PullRequestTimelineCommentItemSchema, + ]), +); + +export const PullRequestTimelineResponseSchema = z.object({ + type: z.literal("pull_request_timeline_response"), + payload: z + .object({ + cwd: z.string().optional().default(""), + prNumber: z.number().nullable().optional().default(null), + items: z.array(PullRequestTimelineItemSchema).optional().default([]), + truncated: z.boolean().optional().default(false), + error: PullRequestTimelineErrorSchema.nullable().optional().default(null), + requestId: z.string().optional().default(""), + githubFeaturesEnabled: z.boolean().optional().default(true), + }) + .optional() + .default({}), +}); + export const CheckoutSwitchBranchResponseSchema = z.object({ type: z.literal("checkout_switch_branch_response"), payload: z.object({ @@ -3000,6 +3098,7 @@ export const SessionOutboundMessageSchema = z.discriminatedUnion("type", [ CheckoutPushResponseSchema, CheckoutPrCreateResponseSchema, CheckoutPrStatusResponseSchema, + PullRequestTimelineResponseSchema, CheckoutSwitchBranchResponseSchema, StashSaveResponseSchema, StashPopResponseSchema, @@ -3234,6 +3333,9 @@ export type CheckoutPrCreateRequest = z.infer; export type CheckoutPrStatusRequest = z.infer; export type CheckoutPrStatusResponse = z.infer; +export type PullRequestTimelineRequest = z.infer; +export type PullRequestTimelineItem = z.infer; +export type PullRequestTimelineResponse = z.infer; export type CheckoutSwitchBranchRequest = z.infer; export type CheckoutSwitchBranchResponse = z.infer; export type StashSaveRequest = z.infer; diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index da83fec69..23a27e050 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -24,7 +24,6 @@ import { pushCurrentBranch, resolveRepositoryDefaultBranch, parseWorktreeList, - parseStatusCheckRollup, isPaseoWorktreePath, isDescendantPath, searchGitHubIssuesAndPrs, @@ -981,101 +980,6 @@ const x = 1; }); }); - it("parses real gh status check rollup output and dedupes by latest check run", () => { - expect( - parseStatusCheckRollup([ - { - __typename: "CheckRun", - completedAt: "2026-04-02T13:53:59Z", - conclusion: "SUCCESS", - detailsUrl: "https://github.com/org/repo/actions/runs/123", - name: "review_app", - startedAt: "2026-04-02T13:49:31Z", - status: "COMPLETED", - workflowName: "Deploy PR Preview", - }, - { - __typename: "CheckRun", - completedAt: "2026-04-02T13:58:59Z", - conclusion: "FAILURE", - detailsUrl: "https://github.com/org/repo/actions/runs/124", - name: "review_app", - startedAt: "2026-04-02T13:55:31Z", - status: "COMPLETED", - }, - ]), - ).toEqual([ - { - name: "review_app", - status: "failure", - url: "https://github.com/org/repo/actions/runs/124", - }, - ]); - }); - - it("parses mixed check run and status context entries", () => { - expect( - parseStatusCheckRollup([ - { - __typename: "CheckRun", - name: "unit-tests", - status: "IN_PROGRESS", - conclusion: null, - detailsUrl: "https://github.com/org/repo/actions/runs/200", - startedAt: "2026-04-02T13:49:31Z", - }, - { - __typename: "StatusContext", - context: "lint", - state: "SUCCESS", - targetUrl: "https://github.com/org/repo/status/300", - createdAt: "2026-04-02T13:48:00Z", - }, - ]), - ).toEqual([ - { - name: "unit-tests", - status: "pending", - url: "https://github.com/org/repo/actions/runs/200", - }, - { - name: "lint", - status: "success", - url: "https://github.com/org/repo/status/300", - }, - ]); - }); - - it("returns an empty list for nullish or empty status check rollups", () => { - expect(parseStatusCheckRollup(undefined)).toEqual([]); - expect(parseStatusCheckRollup(null)).toEqual([]); - expect(parseStatusCheckRollup([])).toEqual([]); - }); - - it("ignores unknown status check rollup node types", () => { - expect( - parseStatusCheckRollup([ - { - __typename: "Commit", - oid: "abc123", - }, - { - __typename: "CheckRun", - name: "build", - status: "COMPLETED", - conclusion: "SUCCESS", - detailsUrl: "https://github.com/org/repo/actions/runs/500", - }, - ]), - ).toEqual([ - { - name: "build", - status: "success", - url: "https://github.com/org/repo/actions/runs/500", - }, - ]); - }); - it("returns merged PR status when no open PR exists for the current branch", async () => { execSync("git checkout -b feature", { cwd: repoDir }); execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir }); @@ -1098,6 +1002,55 @@ const x = 1; expect(status.status?.state).toBe("merged"); }); + it("propagates S1 PR metadata and check display fields through checkout PR status", async () => { + execSync("git checkout -b feature", { cwd: repoDir }); + execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir }); + + const status = await getPullRequestStatus( + repoDir, + createGitHubServiceForStatus( + createPullRequestStatus({ + number: 123, + isDraft: true, + checks: [ + { + name: "server-tests", + status: "success", + url: "https://github.com/getpaseo/paseo/actions/runs/123", + workflow: "Server CI", + duration: "2m 14s", + }, + ], + }), + ), + ); + + expect(status).toEqual({ + githubFeaturesEnabled: true, + status: { + number: 123, + url: "https://github.com/getpaseo/paseo/pull/123", + title: "Ship feature", + state: "open", + baseRefName: "main", + headRefName: "feature", + isMerged: false, + isDraft: true, + checks: [ + { + name: "server-tests", + status: "success", + url: "https://github.com/getpaseo/paseo/actions/runs/123", + workflow: "Server CI", + duration: "2m 14s", + }, + ], + checksStatus: "none", + reviewDecision: null, + }, + }); + }); + it("returns closed-unmerged PR status without marking it as merged", async () => { execSync("git checkout -b feature", { cwd: repoDir }); execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir }); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 56e8e6476..cadc61258 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -12,7 +12,6 @@ import { resolveGitHubRepo, type GitHubService, } from "../services/github-service.js"; -export { parseStatusCheckRollup } from "../services/github-service.js"; import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-path.js"; import { runGitCommand } from "./run-git-command.js"; import { isPaseoOwnedWorktreeCwd } from "./worktree.js"; @@ -1819,12 +1818,16 @@ export interface CreatePullRequestOptions { } export interface PullRequestStatus { + number?: number; + repoOwner?: string; + repoName?: string; url: string; title: string; state: string; baseRefName: string; headRefName: string; isMerged: boolean; + isDraft?: boolean; checks?: PullRequestCheck[]; checksStatus?: ChecksStatus; reviewDecision?: ReviewDecision; @@ -1839,6 +1842,8 @@ export type PullRequestCheck = { name: string; status: "success" | "failure" | "pending" | "skipped" | "cancelled"; url: string | null; + workflow?: string; + duration?: string; }; export type ChecksStatus = "none" | "pending" | "success" | "failure";