From e373c33108390e3ae1373587c8b80eff0b9f57cf Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 19 Apr 2026 13:33:46 +0700 Subject: [PATCH] feat(app,server): add pull request pane to the workspace sidebar Introduce a PR tab in the explorer sidebar that shows status checks, reviews, and the merged timeline for the current branch's pull request. Extend checkout PR status with number, draft flag, repo identity, and per-check workflow/duration; add a new pull_request_timeline request/response pair backed by a GraphQL query; enrich the workspace git snapshot to carry the fields through to the client; and extend the checkout-git-actions store so timeline queries are invalidated alongside status when mutations land. --- .../app/src/components/explorer-sidebar.tsx | 35 +- .../components/git-actions-split-button.tsx | 18 +- packages/app/src/components/pr-pane.test.tsx | 169 +++ packages/app/src/components/pr-pane.tsx | 397 +++++++ packages/app/src/hooks/use-branch-switcher.ts | 7 +- .../src/hooks/use-checkout-pr-status-query.ts | 2 +- .../app/src/hooks/use-pr-pane-data.test.ts | 583 +++++++++++ packages/app/src/hooks/use-pr-pane-data.ts | 183 ++++ .../workspace/workspace-git-actions.tsx | 6 +- .../stores/checkout-git-actions-store.test.ts | 28 + .../src/stores/checkout-git-actions-store.ts | 73 +- .../app/src/stores/explorer-tab-memory.ts | 4 +- packages/app/src/styles/theme.ts | 19 + packages/app/src/utils/pr-pane-data.test.ts | 337 ++++++ packages/app/src/utils/pr-pane-data.ts | 227 ++++ packages/app/src/utils/pr-tab-label.test.ts | 27 + packages/app/src/utils/pr-tab-label.ts | 5 + packages/server/src/client/daemon-client.ts | 20 + packages/server/src/server/session.test.ts | 288 +++++ packages/server/src/server/session.ts | 150 ++- .../src/server/workspace-git-service.ts | 13 + .../src/services/github-service.test.ts | 988 ++++++++++++++++++ .../server/src/services/github-service.ts | 638 ++++++++++- .../messages.pull-request-timeline.test.ts | 282 +++++ packages/server/src/shared/messages.ts | 106 +- .../server/src/utils/checkout-git.test.ts | 145 +-- packages/server/src/utils/checkout-git.ts | 7 +- 27 files changed, 4583 insertions(+), 174 deletions(-) create mode 100644 packages/app/src/components/pr-pane.test.tsx create mode 100644 packages/app/src/components/pr-pane.tsx create mode 100644 packages/app/src/hooks/use-pr-pane-data.test.ts create mode 100644 packages/app/src/hooks/use-pr-pane-data.ts create mode 100644 packages/app/src/utils/pr-pane-data.test.ts create mode 100644 packages/app/src/utils/pr-pane-data.ts create mode 100644 packages/app/src/utils/pr-tab-label.test.ts create mode 100644 packages/app/src/utils/pr-tab-label.ts create mode 100644 packages/server/src/server/session.test.ts create mode 100644 packages/server/src/shared/messages.pull-request-timeline.test.ts 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";