diff --git a/apps/web/src/components/ChatMarkdown.browser.tsx b/apps/web/src/components/ChatMarkdown.browser.tsx index 764fa5ed1..f02b9ab68 100644 --- a/apps/web/src/components/ChatMarkdown.browser.tsx +++ b/apps/web/src/components/ChatMarkdown.browser.tsx @@ -572,9 +572,11 @@ describe("ChatMarkdown", () => { window.addEventListener("click", observeClick); try { + // Both links are chips now, so their text is the number the chip prints + // followed by the words the author wrote. const click = (name: string) => { - const anchor = Array.from(document.querySelectorAll("a")).find( - (candidate) => candidate.textContent === name, + const anchor = Array.from(document.querySelectorAll("a")).find((candidate) => + candidate.textContent?.includes(name), ); if (!anchor) { throw new Error(`No link named ${name}`); @@ -595,6 +597,42 @@ describe("ChatMarkdown", () => { } }); + it("renders a pull request address as a numbered chip that opens its tab", async () => { + const { url: pullRequestUrl, open } = THREAD_PULL_REQUEST_LINK; + open.mockClear(); + const screen = await render( + + + , + ); + + try { + const chips = Array.from(document.querySelectorAll("a")).filter((anchor) => + anchor.getAttribute("href")?.startsWith(pullRequestUrl), + ); + expect(chips).toHaveLength(3); + // The bare address and text that only restates the number both give way + // to the chip; anything the author actually wrote is kept beside it. + expect(chips.map((chip) => chip.textContent)).toEqual([ + "#223", + "#223", + "#223the migration fix", + ]); + // A chip, not a URL: the state glyph plus the number. + expect(chips[0]?.querySelector("svg")).toBeTruthy(); + + chips[0]?.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true })); + expect(open).toHaveBeenCalledTimes(1); + } finally { + await screen.unmount(); + } + }); + it("keeps normal web links unchanged", async () => { const screen = await render( , diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 26b84e02b..340b75fc2 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -64,7 +64,12 @@ import { rewriteMarkdownFileUriHref, } from "../markdown-links"; import { readLocalApi } from "../localApi"; +import { GitPullRequestIcon } from "lucide-react"; + import { ChatWebLink } from "./chat/ChatWebLink"; +import { PullRequestHoverCard, usePullRequestChip } from "./pull-requests/PullRequestHoverCard"; +import { pullRequestBadgeTone } from "./pull-requests/pullRequests.logic"; +import { parsePullRequestUrl } from "../pullRequestReference"; import { copyTextWithToast } from "./chat/copyTextWithToast"; import { isBrowserPanelHref } from "./browser/openInBrowserPanel"; import { @@ -564,6 +569,72 @@ const renderBareImagePath: NonNullable ); +/** The visible words of a link, flattened out of whatever markdown made them. */ +function markdownChildrenText(children: ReactNode): string { + let text = ""; + Children.forEach(children, (child) => { + if (typeof child === "string" || typeof child === "number") { + text += String(child); + return; + } + if (isValidElement<{ children?: ReactNode }>(child)) { + text += markdownChildrenText(child.props.children); + } + }); + return text; +} + +/** A link whose words only repeat the number the chip already prints. */ +const REDUNDANT_PULL_REQUEST_LINK_TEXT = /^(?:pr\s*)?#?\d+$/i; + +/** + * A pull request address in a transcript, as a chip rather than a URL: the + * state glyph and `#number`, which is how the sidebar, the composer's row and + * the pull requests page all name one. The words the author wrote are kept + * after the number unless they only repeat it. + * + * The click behaviour is unchanged -- {@link ChatWebLink} still decides whether + * this opens the thread's own Pull request tab or the browser panel. + */ +function MarkdownPullRequestChip({ + href, + repository, + number, + label, + threadRef, + title, +}: { + readonly href: string; + readonly repository: string; + readonly number: number; + /** Empty when the link's own text said nothing the number does not. */ + readonly label: string; + readonly threadRef: ScopedThreadRef | null; + readonly title?: string | undefined; +}) { + const chip = usePullRequestChip(repository, number); + const tone = chip.state + ? pullRequestBadgeTone(chip.state.state, chip.state.isDraft) + : // Nothing here has listed this repository, so the glyph says "a pull + // request" without claiming to know how it is going. + { Icon: GitPullRequestIcon, className: "text-muted-foreground", label: "Pull request" }; + + return ( + + + + #{number} + {label ? {label} : null} + + + ); +} + function MarkdownAnchor({ node: _node, href, children, ...props }: MarkdownRendererProps<"a">) { const { cwd, @@ -580,6 +651,21 @@ function MarkdownAnchor({ node: _node, href, children, ...props }: MarkdownRende resolveMarkdownFileLinkMeta(normalizedHref, cwd)) : null; if (!fileLinkMeta) { + const pullRequest = href ? parsePullRequestUrl(href) : null; + if (href && pullRequest) { + const text = markdownChildrenText(children).trim(); + const label = text === href.trim() || REDUNDANT_PULL_REQUEST_LINK_TEXT.test(text) ? "" : text; + return ( + + ); + } if (href && isBrowserPanelHref(href)) { return ( ({ + ...project, + repositoryIdentity: { + canonicalKey: `github.com/${PULL_REQUEST_REPOSITORY}`.toLowerCase(), + locator: { + source: "git-remote" as const, + remoteName: "origin", + remoteUrl: `https://github.com/${PULL_REQUEST_REPOSITORY}.git`, + }, + displayName: PULL_REQUEST_REPOSITORY, + provider: "github", + owner: "Threadlines", + name: "threadlines", + }, + })), + }; +} + +/** The listing row and the detail behind the composer's docked pull request. */ +function resolvePullRequestRpc(body: NormalizedWsRpcRequestBody): unknown | undefined { + if (body._tag === WS_METHODS.pullRequestsList) { + const state = (body as { state?: string }).state; + return { + viewer: "badcuban", + errors: [], + entries: + state === "open" + ? [ + { + provider: "github", + projectId: PROJECT_ID, + projectTitle: "threadlines", + repository: PULL_REQUEST_REPOSITORY, + number: PULL_REQUEST_NUMBER, + title: "fix(server): migration 050 no longer stalls startup", + url: PULL_REQUEST_URL, + author: { login: "badcuban", isBot: false, avatarUrl: null }, + headBranch: PULL_REQUEST_HEAD_BRANCH, + baseBranch: "main", + state: "open", + isDraft: false, + additions: 26, + deletions: 25, + createdAt: NOW_ISO, + updatedAt: NOW_ISO, + viewerIsAuthor: true, + viewerReviewRequested: false, + labels: [], + origin: "workspace", + }, + ] + : [], + }; + } + if (body._tag === WS_METHODS.pullRequestsDetail) { + return { + provider: "github", + projectId: PROJECT_ID, + projectTitle: "threadlines", + workspaceRoot: "/repo/project", + repository: PULL_REQUEST_REPOSITORY, + number: PULL_REQUEST_NUMBER, + title: "fix(server): migration 050 no longer stalls startup", + body: "", + url: PULL_REQUEST_URL, + author: { login: "badcuban", isBot: false, avatarUrl: null }, + state: "open", + isDraft: false, + mergeability: "mergeable", + additions: 26, + deletions: 25, + changedFiles: 2, + headBranch: PULL_REQUEST_HEAD_BRANCH, + baseBranch: "main", + createdAt: NOW_ISO, + updatedAt: NOW_ISO, + mergedAt: null, + closedAt: null, + viewerIsAuthor: true, + reviewers: [], + labels: [], + checks: [ + { name: "build", status: "pending", description: null, url: null }, + { name: "test", status: "pending", description: null, url: null }, + { name: "lint", status: "success", description: null, url: null }, + ], + checksState: "pending", + viewer: { canWrite: true, canReview: false, canManage: true }, + mergeMethods: ["squash"], + capabilities: { + diff: true, + comment: true, + actions: ["merge", "close", "enable-auto-merge", "disable-auto-merge"], + mergeMethods: ["squash"], + updateMethods: ["merge"], + reactions: true, + review: { + inlineComment: true, + reply: true, + resolve: true, + verdicts: ["approve", "comment"], + }, + reviewers: { request: true, listCandidates: true }, + edit: { pullRequest: true, comment: true }, + }, + baseComparison: "up-to-date", + behindBy: 0, + autoMergeEnabled: false, + isStacked: false, + defaultBranch: "main", + }; + } + return undefined; +} + function addThreadToSnapshot( snapshot: OrchestrationReadModel, threadId: ThreadId, @@ -2775,6 +2918,63 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + it("docks the thread's pull request above the notices in one frame", async () => { + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: createSnapshotForTargetUser({ + targetMessageId: "msg-user-pull-request-dock" as MessageId, + targetText: "pull request dock", + }), + configureFixture: withPullRequestFixture, + resolveRpc: resolvePullRequestRpc, + }); + + try { + // The branch, the project and the size only exist on the detail, so + // waiting for the branch is waiting for the shared read to reach the row. + const row = await waitForElement(() => { + const candidate = document.querySelector( + '[data-composer-pull-request-row="true"]', + ); + return candidate?.textContent?.includes(PULL_REQUEST_HEAD_BRANCH) ? candidate : null; + }, "Unable to find the composer pull request row with its branch."); + expect(row.textContent).toContain(`#${PULL_REQUEST_NUMBER}`); + expect(row.textContent).toContain("+26"); + // Two checks are still running, which is what the chip's word covers and + // its dot colours. + expect(row.textContent).toContain("CI"); + + // The state glyph is the open one the sidebar badge and the pull + // requests page use, in the same tone. + const stateIcon = row.querySelector("svg"); + expect(stateIcon?.getAttribute("class")).toContain("text-emerald-600"); + + // One frame, not two: the notice the version skew raises sits inside the + // same dock, under the pull request row, and the dock's bottom edge is + // the composer's top edge. + const dock = document.querySelector('[data-composer-notice-dock="true"]'); + expect(dock).toBeTruthy(); + expect(dock!.contains(row)).toBe(true); + const notice = dock!.querySelector("[data-composer-notice-severity]"); + expect(notice).toBeTruthy(); + expect(row.getBoundingClientRect().bottom).toBeLessThanOrEqual( + notice!.getBoundingClientRect().top + 1, + ); + + const composerSurface = document.querySelector( + "[data-chat-composer-mobile-collapsed]", + ); + expect(composerSurface).toBeTruthy(); + expect( + Math.abs( + dock!.getBoundingClientRect().bottom - composerSurface!.getBoundingClientRect().top, + ), + ).toBeLessThan(2); + } finally { + await mounted.cleanup(); + } + }); + it("re-expands the bootstrap project using its logical key", async () => { useUiStateStore.setState({ projectExpandedById: { diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 3a868288c..9e8ea6c98 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -207,6 +207,7 @@ import { useTerminalStateStore, } from "../terminalStateStore"; import { ChatComposer, type ChatComposerHandle } from "./chat/ChatComposer"; +import type { ComposerPullRequest } from "./chat/ComposerPullRequestRow"; import { type ComposerGoalSetInput } from "./chat/ComposerGoalBar"; import { getComposerProviderState } from "./chat/composerProviderState"; import { ExpandedImageDialog } from "./chat/ExpandedImageDialog"; @@ -793,6 +794,11 @@ type ChatViewProps = onDiffPanelOpen?: () => void; reserveTitleBarControlInset?: boolean; composerFocusRequest?: number; + /** + * The thread's pull request, resolved by the route that owns the Pull + * request tab. The composer docks a row for it; a draft has none. + */ + composerPullRequest?: ComposerPullRequest | null; routeKind: "server"; draftId?: never; } @@ -1088,6 +1094,7 @@ export default function ChatView(props: ChatViewProps) { reserveTitleBarControlInset = true, composerFocusRequest = 0, } = props; + const composerPullRequest = routeKind === "server" ? (props.composerPullRequest ?? null) : null; const draftId = routeKind === "draft" ? props.draftId : null; const routeThreadRef = useMemo( () => scopeThreadRef(environmentId, threadId), @@ -6856,6 +6863,7 @@ export default function ChatView(props: ChatViewProps) { } activeThreadActivities={activeThread?.activities} notices={composerNotices} + pullRequest={composerPullRequest} resolvedTheme={resolvedTheme} settings={settings} keybindings={keybindings} diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index 24a68db0b..e7d0b11a7 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -116,7 +116,8 @@ import { ComposerPendingUserInputPanel } from "./ComposerPendingUserInputPanel"; import { ComposerGoalBar, type ComposerGoalSetInput } from "./ComposerGoalBar"; import { ComposerPlanFollowUpBanner } from "./ComposerPlanFollowUpBanner"; import type { ComposerNotice } from "./composerNotices"; -import { ComposerNoticeDock } from "./ComposerNoticeDock"; +import { ComposerDock, hasComposerDockContent } from "./ComposerDock"; +import type { ComposerPullRequest } from "./ComposerPullRequestRow"; import { ComposerPendingDrawingContexts } from "./ComposerPendingDrawingContexts"; import { ComposerPendingPickedElementContexts } from "./ComposerPendingPickedElementContexts"; import { ComposerPendingTranscriptHighlightContexts } from "./ComposerPendingTranscriptHighlightContexts"; @@ -557,6 +558,13 @@ export interface ChatComposerProps { */ notices: ReadonlyArray; + /** + * The pull request the thread is working on, docked above the notices. Null + * where the thread has none, or where the surface has no route to resolve + * one (a draft, a general chat). + */ + pullRequest: ComposerPullRequest | null; + // Misc resolvedTheme: "light" | "dark"; settings: UnifiedSettings; @@ -654,6 +662,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) activeThreadModelSelection, activeThreadActivities, notices, + pullRequest, resolvedTheme, settings, keybindings, @@ -3081,13 +3090,20 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) onDragLeave={onComposerDragLeave} onDrop={onComposerDrop} > - + {/* A collapsed composer drops the standing context, never a notice + about sending: those stay in front of the send button. */} +
0 && "rounded-t-none", + !isComposerCollapsedMobile && + hasComposerDockContent({ pullRequest, notices }) && + "rounded-t-none", isDragOverComposer ? "border-primary/70 bg-accent/30" : "border-border", environmentUnavailable ? "opacity-75" : null, composerProviderState.composerSurfaceClassName, diff --git a/apps/web/src/components/chat/ComposerDock.tsx b/apps/web/src/components/chat/ComposerDock.tsx new file mode 100644 index 000000000..6fc931411 --- /dev/null +++ b/apps/web/src/components/chat/ComposerDock.tsx @@ -0,0 +1,49 @@ +/** + * The rows docked to the top of the composer. + * + * One frame, not two: the pull request row and the notice rows share the + * composer's left and right edges and square off its top corners, so they read + * as one statement about sending rather than a stack of separate cards. The + * pull request comes first because it is standing context, and a notice is + * news that has to land closest to the thing it blocks. + * + * @module ComposerDock + */ +import { ComposerNoticeDock } from "./ComposerNoticeDock"; +import { ComposerPullRequestRow, type ComposerPullRequest } from "./ComposerPullRequestRow"; +import type { ComposerNotice } from "./composerNotices"; + +/** Whether the dock would draw anything, which is what squares the composer's top. */ +export function hasComposerDockContent(input: { + readonly pullRequest: ComposerPullRequest | null; + readonly notices: ReadonlyArray; +}): boolean { + return input.pullRequest !== null || input.notices.length > 0; +} + +export function ComposerDock({ + pullRequest, + notices, +}: { + readonly pullRequest: ComposerPullRequest | null; + readonly notices: ReadonlyArray; +}) { + if (!hasComposerDockContent({ pullRequest, notices })) { + return null; + } + return ( + // The dock is always exactly as wide as the composer it docks to, so its + // inline size is contained: without that, a row's fixed chrome raises the + // composer's minimum width and can hold its footer out of the compact + // layout that narrow widths depend on. +
+ {pullRequest ? ( + 0} /> + ) : null} + +
+ ); +} diff --git a/apps/web/src/components/chat/ComposerNoticeDock.tsx b/apps/web/src/components/chat/ComposerNoticeDock.tsx index 0b813e7fd..420d4df2b 100644 --- a/apps/web/src/components/chat/ComposerNoticeDock.tsx +++ b/apps/web/src/components/chat/ComposerNoticeDock.tsx @@ -1,10 +1,9 @@ /** - * The notice row docked to the top of the composer. + * The notice rows inside the composer's dock. * - * It shares the composer's left and right edges and squares off its top - * corners, so it reads as a statement about sending rather than as another - * piece of chat content. Only the worst active notice is on screen; the rest - * sit behind a count that expands them in place. + * Only the worst active notice is on screen; the rest sit behind a count that + * expands them in place. The frame around them belongs to {@link ComposerDock}, + * which the pull request row shares. * * @module ComposerNoticeDock */ @@ -55,15 +54,7 @@ export function ComposerNoticeDock({ notices }: { notices: ReadonlyArray +
{stackedNotices.map((notice) => ( ))} diff --git a/apps/web/src/components/chat/ComposerPullRequestRow.tsx b/apps/web/src/components/chat/ComposerPullRequestRow.tsx new file mode 100644 index 000000000..d9a775290 --- /dev/null +++ b/apps/web/src/components/chat/ComposerPullRequestRow.tsx @@ -0,0 +1,301 @@ +/** + * The pull request the thread is working on, docked to the top of the + * composer. + * + * The sidebar badge says a pull request exists and the Pull request tab says + * everything about it; neither is in view while you are writing the next + * message, which is exactly when "did the checks pass" decides what you type. + * The row is one line: which pull request, on which branch, how big, and how + * its checks are going, with the two switches that decide what happens when + * they pass. + * + * @module ComposerPullRequestRow + */ +import type { + EnvironmentId, + PullRequestAction, + PullRequestDetail, + PullRequestRef, +} from "@threadlines/contracts"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { ChevronDownIcon, ExternalLinkIcon } from "lucide-react"; +import { useState } from "react"; + +import { isElectron } from "../../env"; +import { useSettings, updateSettings } from "../../hooks/useSettings"; +import { readLocalApi } from "../../localApi"; +import { pullRequestActionMutationOptions } from "../../lib/pullRequestsReactQuery"; +import { cn } from "../../lib/utils"; +import { + PullRequestHoverCard, + type PullRequestHoverCardPayload, +} from "../pull-requests/PullRequestHoverCard"; +import { CHECK_TONES } from "../pull-requests/pullRequestPresentation"; +import { pullRequestBadgeTone, type ThreadPullRequest } from "../pull-requests/pullRequests.logic"; +import { Checkbox } from "../ui/checkbox"; +import { Popover, PopoverPopup, PopoverTrigger } from "../ui/popover"; +import { DiffStatLabel } from "./DiffStatLabel"; +import { + canToggleComposerAutoMerge, + composerPullRequestCheckBuckets, + composerPullRequestRow, + pullRequestChecksUrl, + type ComposerPullRequestChipTone, +} from "./composerPullRequest.logic"; + +/** Everything the thread route hands the composer about its pull request. */ +export interface ComposerPullRequest { + readonly environmentId: EnvironmentId; + readonly reference: PullRequestRef; + /** What the sidebar badge and the tab already resolved. */ + readonly pullRequest: ThreadPullRequest; + /** The shared read behind the Pull request tab; absent until it lands. */ + readonly detail: PullRequestDetail | undefined; + /** Opens the Pull request tab, the same place the sidebar badge goes. */ + readonly onOpen: () => void; +} + +const CHIP_TONE_CLASS: Readonly< + Record +> = { + unknown: { chip: "", dot: "border-[1.5px] border-muted-foreground/70" }, + pending: { chip: "", dot: "border-[1.5px] border-muted-foreground/70" }, + success: { chip: "", dot: "bg-success" }, + failure: { chip: "", dot: "bg-destructive" }, + none: { chip: "text-muted-foreground", dot: "border-[1.5px] border-muted-foreground/70" }, + queued: { chip: "text-amber-600/90 dark:text-amber-400/80", dot: "bg-current" }, + merged: { chip: "text-violet-600 dark:text-violet-300/90", dot: "bg-current" }, + closed: { chip: "text-zinc-500 dark:text-zinc-400/80", dot: "bg-current" }, +}; + +export function ComposerPullRequestRow({ + pullRequest, + divided, +}: { + readonly pullRequest: ComposerPullRequest; + /** A notice sits under this row, so the two are ruled apart. */ + readonly divided: boolean; +}) { + const row = composerPullRequestRow({ + pullRequest: pullRequest.pullRequest, + detail: pullRequest.detail, + }); + const tone = pullRequestBadgeTone(row.state, row.isDraft); + const hoverCardPayload: PullRequestHoverCardPayload = { + environmentId: pullRequest.environmentId, + reference: pullRequest.reference, + state: row.state, + isDraft: row.isDraft, + }; + + return ( +
+ + + + {row.projectTitle ? ( + {row.projectTitle} + ) : null} + {row.headBranch ? ( + + {row.headBranch} + + ) : ( + + )} + {row.diffStat ? ( + + + + ) : null} + +
+ ); +} + +/** The dot every chip and popover row leads with, filled or hollow. */ +function ChipDot({ className }: { readonly className: string }) { + return ; +} + +function ComposerPullRequestChecksChip({ + pullRequest, + chip, + checksUrl, +}: { + readonly pullRequest: ComposerPullRequest; + readonly chip: ReturnType["chip"]; + readonly checksUrl: string; +}) { + const [open, setOpen] = useState(false); + const toneClass = CHIP_TONE_CLASS[chip.tone]; + const chipClass = cn( + "inline-flex h-[22px] shrink-0 items-center gap-1.5 rounded-md border border-border px-1.5 text-xs", + toneClass.chip, + ); + + // A settled pull request has no checks left to wait on and nothing to arm, + // so its chip states the fact and is not a control. + if (!chip.interactive) { + return ( + + + {chip.label} + + ); + } + + return ( + + + + {chip.label} + + + } + /> + + setOpen(false)} + /> + + + ); +} + +function ComposerPullRequestChecksPopover({ + pullRequest, + checksUrl, + onOpenExternal, +}: { + readonly pullRequest: ComposerPullRequest; + readonly checksUrl: string; + readonly onOpenExternal: () => void; +}) { + const queryClient = useQueryClient(); + const detail = pullRequest.detail; + const buckets = composerPullRequestCheckBuckets(detail?.checks ?? []); + const wrapUpOnSettled = useSettings((settings) => settings.wrapUpThreadsOnPullRequestSettled); + const autoMergeEnabled = detail?.autoMergeEnabled === true; + const canToggleAutoMerge = canToggleComposerAutoMerge(detail); + const action = useMutation( + pullRequestActionMutationOptions({ + environmentId: pullRequest.environmentId, + reference: pullRequest.reference, + queryClient, + }), + ); + + return ( +
+
+ Checks + {/* Outside Electron there is no shell to hand the address to, so the + same affordance is an ordinary link the browser opens itself. */} + {isElectron ? ( + + ) : ( + + + + )} +
+ {buckets.length === 0 ? ( +

No checks on this pull request

+ ) : ( + buckets.map((bucket) => { + const checkTone = CHECK_TONES[bucket.id]; + return ( +
+ + {bucket.label} + {bucket.count} +
+ ); + }) + )} +
+ {canToggleAutoMerge ? ( + + ) : null} + +
+ ); +} diff --git a/apps/web/src/components/chat/DiffStatLabel.tsx b/apps/web/src/components/chat/DiffStatLabel.tsx index 2dda06fd9..873ae58a3 100644 --- a/apps/web/src/components/chat/DiffStatLabel.tsx +++ b/apps/web/src/components/chat/DiffStatLabel.tsx @@ -8,13 +8,23 @@ export const DiffStatLabel = memo(function DiffStatLabel(props: { additions: number; deletions: number; showParentheses?: boolean; + /** + * What sits between the two counts. The slash reads as one figure and suits + * a line of meta; a plain space reads as two facts and suits a row that is + * already ruled into columns. + */ + separator?: "slash" | "space"; }) { - const { additions, deletions, showParentheses = false } = props; + const { additions, deletions, showParentheses = false, separator = "slash" } = props; return ( <> {showParentheses && (} +{additions} - / + {separator === "slash" ? ( + / + ) : ( + + )} -{deletions} {showParentheses && )} diff --git a/apps/web/src/components/chat/composerPullRequest.logic.test.ts b/apps/web/src/components/chat/composerPullRequest.logic.test.ts new file mode 100644 index 000000000..401b11f90 --- /dev/null +++ b/apps/web/src/components/chat/composerPullRequest.logic.test.ts @@ -0,0 +1,180 @@ +import type { PullRequestCheck, PullRequestDetail } from "@threadlines/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import type { ThreadPullRequest } from "../pull-requests/pullRequests.logic"; +import { + canToggleComposerAutoMerge, + composerPullRequestCheckBuckets, + composerPullRequestChip, + composerPullRequestRow, +} from "./composerPullRequest.logic"; + +const THREAD_PULL_REQUEST: ThreadPullRequest = { + number: 234, + state: "open", + isDraft: false, + title: "fix(server): migration 050 no longer stalls startup", + url: "https://github.com/Threadlines/threadlines/pull/234", + repository: "Threadlines/threadlines", + settledAt: null, +}; + +function check(status: PullRequestCheck["status"], name: string): PullRequestCheck { + return { name, status, description: null, url: null } as PullRequestCheck; +} + +function detail(overrides: Partial = {}): PullRequestDetail { + return { + provider: "github", + projectId: "project-1", + projectTitle: "threadlines", + workspaceRoot: "/repo/project", + repository: "Threadlines/threadlines", + number: 234, + title: "fix(server): migration 050 no longer stalls startup on large databases", + body: "", + url: "https://github.com/Threadlines/threadlines/pull/234", + author: { login: "badcuban", isBot: false, avatarUrl: null }, + state: "open", + isDraft: false, + mergeability: "mergeable", + additions: 26, + deletions: 25, + changedFiles: 2, + headBranch: "fix/migration-050-backfill-speed", + baseBranch: "main", + createdAt: "2026-09-01T00:00:00.000Z", + updatedAt: "2026-09-01T01:00:00.000Z", + mergedAt: null, + closedAt: null, + viewerIsAuthor: true, + reviewers: [], + labels: [], + checks: [], + viewer: { canWrite: true, canReview: false, canManage: true }, + mergeMethods: ["squash"], + capabilities: { + diff: true, + comment: true, + actions: ["merge", "close", "enable-auto-merge", "disable-auto-merge"], + mergeMethods: ["squash"], + updateMethods: ["merge"], + reactions: true, + review: { + inlineComment: true, + reply: true, + resolve: true, + verdicts: ["approve", "comment"], + }, + reviewers: { request: true, listCandidates: true }, + edit: { pullRequest: true, comment: true }, + }, + baseComparison: "up-to-date", + behindBy: 0, + autoMergeEnabled: false, + isStacked: false, + defaultBranch: "main", + ...overrides, + } as PullRequestDetail; +} + +describe("composerPullRequestRow", () => { + it("renders from the thread's own pull request before the detail arrives", () => { + const row = composerPullRequestRow({ pullRequest: THREAD_PULL_REQUEST, detail: undefined }); + + expect(row.number).toBe(234); + expect(row.state).toBe("open"); + expect(row.title).toBe(THREAD_PULL_REQUEST.title); + // Nothing but the detail knows these, and a guessed branch or diff stat is + // worse than a row that simply has not filled in yet. + expect(row.headBranch).toBeNull(); + expect(row.projectTitle).toBeNull(); + expect(row.diffStat).toBeNull(); + expect(row.chip).toEqual({ label: "CI", tone: "unknown", interactive: true }); + }); + + it("takes the branch, project, size and state from the detail once it lands", () => { + const row = composerPullRequestRow({ + pullRequest: THREAD_PULL_REQUEST, + // The listing behind the thread's resolution polls slowly, so a merge + // shows up on the detail first and the row has to follow it. + detail: detail({ state: "merged", checks: [check("success", "build")] }), + }); + + expect(row.state).toBe("merged"); + expect(row.headBranch).toBe("fix/migration-050-backfill-speed"); + expect(row.projectTitle).toBe("threadlines"); + expect(row.diffStat).toEqual({ additions: 26, deletions: 25 }); + expect(row.chip).toEqual({ label: "Merged", tone: "merged", interactive: false }); + }); +}); + +describe("composerPullRequestChip", () => { + it("reads the check rollup while the pull request is open", () => { + expect( + composerPullRequestChip({ state: "open", detail: detail({ checksState: "failure" }) }), + ).toEqual({ label: "CI", tone: "failure", interactive: true }); + }); + + it("leads with the merge queue, which moves without anyone asking", () => { + expect( + composerPullRequestChip({ + state: "open", + detail: detail({ checksState: "success", mergeQueue: { position: 2 } }), + }), + ).toEqual({ label: "Queued", tone: "queued", interactive: true }); + }); + + it("says so when the host reported no checks at all", () => { + expect(composerPullRequestChip({ state: "open", detail: detail() })).toEqual({ + label: "No checks", + tone: "none", + interactive: true, + }); + }); + + it("states the outcome for a settled pull request and stops being a control", () => { + expect( + composerPullRequestChip({ state: "closed", detail: detail({ state: "closed" }) }), + ).toEqual({ label: "Closed", tone: "closed", interactive: false }); + }); +}); + +describe("composerPullRequestCheckBuckets", () => { + it("counts each status once, worst first, and drops the empty buckets", () => { + expect( + composerPullRequestCheckBuckets([ + check("success", "lint"), + check("failure", "test"), + check("pending", "build"), + check("success", "typecheck"), + check("pending", "e2e"), + ]), + ).toEqual([ + { id: "pending", label: "In progress", count: 2 }, + { id: "failure", label: "Failed", count: 1 }, + { id: "success", label: "Passed", count: 2 }, + ]); + }); +}); + +describe("canToggleComposerAutoMerge", () => { + it("is off where the host does not say whether the pull request is armed", () => { + expect(canToggleComposerAutoMerge(detail({ autoMergeEnabled: null }))).toBe(false); + expect(canToggleComposerAutoMerge(undefined)).toBe(false); + }); + + it("is off where the host offers neither action", () => { + expect( + canToggleComposerAutoMerge( + detail({ + capabilities: { ...detail().capabilities, actions: ["merge", "close"] }, + }), + ), + ).toBe(false); + }); + + it("is on where the host offers one of them", () => { + expect(canToggleComposerAutoMerge(detail())).toBe(true); + }); +}); diff --git a/apps/web/src/components/chat/composerPullRequest.logic.ts b/apps/web/src/components/chat/composerPullRequest.logic.ts new file mode 100644 index 000000000..6c056ace4 --- /dev/null +++ b/apps/web/src/components/chat/composerPullRequest.logic.ts @@ -0,0 +1,164 @@ +/** + * What the composer's pull request row says, derived from the two reads that + * feed it: the thread's own pull request (which the sidebar badge and the tab + * already resolve) and, once it arrives, the detail the Pull request tab reads. + * + * The row renders from the first alone, so it appears with the thread rather + * than a beat later, and gains its branch, its diff stat and its check rollup + * when the detail lands. + * + * @module composerPullRequest.logic + */ +import type { PullRequestCheck, PullRequestDetail, PullRequestState } from "@threadlines/contracts"; + +import { + summarizePullRequestChecks, + type ThreadPullRequest, +} from "../pull-requests/pullRequests.logic"; + +/** + * How the check chip reads. `pending`, `success` and `failure` are the host's + * own rollup; `queued` is a merge queue, which moves on its own and outranks + * whatever the checks say; `none` is a pull request with no checks at all, and + * `unknown` is the moment before the detail arrives. `merged` and `closed` + * state a fact about the pull request rather than its checks. + */ +export type ComposerPullRequestChipTone = + | "unknown" + | "pending" + | "success" + | "failure" + | "none" + | "queued" + | "merged" + | "closed"; + +export interface ComposerPullRequestChip { + readonly label: string; + readonly tone: ComposerPullRequestChipTone; + /** + * Whether the chip opens the checks popover. A settled pull request has + * nothing left to arm or wait for, so its chip is a word, not a button. + */ + readonly interactive: boolean; +} + +/** One status bucket of a check run, as the popover lists it. */ +export interface ComposerPullRequestCheckBucket { + readonly id: "pending" | "failure" | "success" | "skipped"; + readonly label: string; + readonly count: number; +} + +/** Everything the row draws, in the order it draws it. */ +export interface ComposerPullRequestRowModel { + readonly number: number; + readonly state: PullRequestState; + readonly isDraft: boolean; + readonly title: string; + readonly url: string; + /** Absent until the detail arrives. */ + readonly projectTitle: string | null; + readonly headBranch: string | null; + readonly diffStat: { readonly additions: number; readonly deletions: number } | null; + readonly chip: ComposerPullRequestChip; +} + +/** + * The buckets worth a row of their own, worst first: what is still running, + * what failed, what passed, and what the host skipped. A bucket nobody is in + * is left out rather than printed as a zero. + */ +export function composerPullRequestCheckBuckets( + checks: readonly PullRequestCheck[], +): readonly ComposerPullRequestCheckBucket[] { + const summary = summarizePullRequestChecks(checks); + return ( + [ + { id: "pending", label: "In progress", count: summary.pending }, + { id: "failure", label: "Failed", count: summary.failing }, + { id: "success", label: "Passed", count: summary.passing }, + { id: "skipped", label: "Skipped", count: summary.skipped }, + ] as const + ).filter((bucket) => bucket.count > 0); +} + +/** + * The chip's word and colour. A settled pull request says so and stops there; + * a queued one leads with the queue, since that is the part that moves without + * anyone here asking. Otherwise it is the check rollup, which reads as "CI" + * whichever way it is going -- the dot carries that -- and as "No checks" only + * where the host reported none at all. + */ +export function composerPullRequestChip(input: { + readonly state: PullRequestState; + readonly detail: PullRequestDetail | undefined; +}): ComposerPullRequestChip { + if (input.state === "merged") { + return { label: "Merged", tone: "merged", interactive: false }; + } + if (input.state === "closed") { + return { label: "Closed", tone: "closed", interactive: false }; + } + const detail = input.detail; + if (detail === undefined) { + return { label: "CI", tone: "unknown", interactive: true }; + } + if (detail.mergeQueue !== undefined && detail.mergeQueue.position !== null) { + return { label: "Queued", tone: "queued", interactive: true }; + } + if (detail.checksState === undefined && detail.checks.length === 0) { + return { label: "No checks", tone: "none", interactive: true }; + } + return { + label: "CI", + tone: detail.checksState ?? summarizePullRequestChecks(detail.checks).state, + interactive: true, + }; +} + +/** + * The row for one thread's pull request. The state, number and title come from + * the thread's own resolution so the row is never blank; everything the detail + * alone knows waits for it rather than being guessed at. + */ +export function composerPullRequestRow(input: { + readonly pullRequest: ThreadPullRequest; + readonly detail: PullRequestDetail | undefined; +}): ComposerPullRequestRowModel { + const { pullRequest, detail } = input; + // The detail is the fresher read of the two: it is re-read while checks run, + // while the listing behind the thread's resolution polls far more slowly. + const state = detail?.state ?? pullRequest.state; + return { + number: pullRequest.number, + state, + isDraft: detail?.isDraft ?? pullRequest.isDraft, + title: detail?.title ?? pullRequest.title, + url: detail?.url ?? pullRequest.url, + projectTitle: detail?.projectTitle ?? null, + headBranch: detail?.headBranch ?? null, + diffStat: detail ? { additions: detail.additions, deletions: detail.deletions } : null, + chip: composerPullRequestChip({ state, detail }), + }; +} + +/** + * Whether the "Merge when checks pass" toggle has anything to do. A host that + * does not say whether the pull request is armed cannot be asked to arm it, + * and neither can one that offers neither action. + */ +export function canToggleComposerAutoMerge(detail: PullRequestDetail | undefined): boolean { + if (detail === undefined || detail.autoMergeEnabled === null) { + return false; + } + return ( + detail.capabilities.actions.includes("enable-auto-merge") || + detail.capabilities.actions.includes("disable-auto-merge") + ); +} + +/** The host's own checks page for a pull request. */ +export function pullRequestChecksUrl(url: string): string { + return `${url.replace(/\/+$/u, "")}/checks`; +} diff --git a/apps/web/src/components/pull-requests/PullRequestHoverCard.tsx b/apps/web/src/components/pull-requests/PullRequestHoverCard.tsx new file mode 100644 index 000000000..58ef6a0e4 --- /dev/null +++ b/apps/web/src/components/pull-requests/PullRequestHoverCard.tsx @@ -0,0 +1,279 @@ +/** + * The card behind every pull request chip: the one in a transcript and the + * `#number` on the composer's pull request row. + * + * A chip is a number and a colour, which is the right size for a line of + * prose and too small to answer "which one is that, and how is it going". The + * card answers exactly that, and nothing more: the state, the repository, when + * it last moved, its title, who wrote it and how big it is. + * + * One handle per surface, the way {@link ThreadHoverCard} does it: two preview + * card roots sharing a handle is undefined behaviour, so each surface creates + * its own and every chip inside points at that one. A chip rendered outside a + * provider (a transcript in the pull request panel, say) stays a chip. + * + * @module PullRequestHoverCard + */ +import type { EnvironmentId, PullRequestRef, PullRequestState } from "@threadlines/contracts"; +import { useQuery } from "@tanstack/react-query"; +import { createContext, useContext, useMemo, useState, type ReactNode } from "react"; +import { useShallow } from "zustand/shallow"; + +import { DiffStatLabel } from "../chat/DiffStatLabel"; +import { + pullRequestDetailQueryOptions, + useLoadedPullRequestEntries, +} from "../../lib/pullRequestsReactQuery"; +import { cn } from "../../lib/utils"; +import { selectWorkspaceProjectsAcrossEnvironments, useStore } from "../../store"; +import { formatRelativeTimeLabel } from "../../timestampFormat"; +import { + PreviewCard, + PreviewCardPopup, + PreviewCardTrigger, + createPreviewCardHandle, +} from "../ui/previewCard"; +import { PullRequestActorAvatar } from "./pullRequestPresentation"; +import { + projectRepository, + pullRequestBadgeTone, + type ThreadPullRequest, +} from "./pullRequests.logic"; + +/** What a chip knows about its pull request before the card reads anything. */ +export interface PullRequestChipState { + readonly state: PullRequestState; + readonly isDraft: boolean; +} + +/** Everything the card needs to draw itself and then read the rest. */ +export interface PullRequestHoverCardPayload extends PullRequestChipState { + readonly environmentId: EnvironmentId; + readonly reference: PullRequestRef; +} + +type PullRequestCardHandle = ReturnType< + typeof createPreviewCardHandle +>; + +/** The checkout whose host tool can read one repository. */ +interface PullRequestRepositoryScope { + readonly environmentId: EnvironmentId; + readonly projectId: PullRequestRef["projectId"]; + readonly repository: string; +} + +interface PullRequestChipSurface { + readonly handle: PullRequestCardHandle; + /** The state a chip wears, keyed by {@link chipKey}. */ + readonly stateByKey: ReadonlyMap; + /** Which environment and project can read a repository, keyed by its lowercased name. */ + readonly scopeByRepository: ReadonlyMap; +} + +const PullRequestChipSurfaceContext = createContext(null); + +function chipKey(repository: string, number: number): string { + return `${repository.toLowerCase()}:${number}`; +} + +/** + * What one chip should look like and, where the workspace can read it, what the + * card would address. A repository no project here points at still gets a chip, + * drawn as an open pull request in the muted tone that says "state unknown". + */ +export function usePullRequestChip( + repository: string, + number: number, +): { + readonly state: PullRequestChipState | null; + readonly payload: PullRequestHoverCardPayload | null; +} { + const surface = useContext(PullRequestChipSurfaceContext); + const state = surface?.stateByKey.get(chipKey(repository, number)) ?? null; + const scope = surface?.scopeByRepository.get(repository.toLowerCase()) ?? null; + return useMemo( + () => ({ + state, + payload: + scope === null + ? null + : { + environmentId: scope.environmentId, + reference: { projectId: scope.projectId, repository: scope.repository, number }, + state: state?.state ?? "open", + isDraft: state?.isDraft ?? false, + }, + }), + [number, scope, state], + ); +} + +/** + * A chip's half of the card: a trigger carrying its pull request as payload. + * Renders its child untouched where the surface has no card, or where the + * workspace cannot read this repository and there would be nothing to show. + */ +export function PullRequestHoverCard({ + payload, + children, +}: { + readonly payload: PullRequestHoverCardPayload | null; + readonly children: ReactNode; +}) { + const surface = useContext(PullRequestChipSurfaceContext); + if (surface === null || payload === null) { + return <>{children}; + } + return ( + + ); +} + +/** + * A surface's card and the two lookups its chips read: which state a pull + * request is in, and which checkout could read it. Both are computed once here + * rather than per chip, so a transcript full of references costs one + * subscription. + */ +export function PullRequestHoverCardProvider({ + threadPullRequest = null, + children, +}: { + /** + * The thread's own pull request, whose state the listings may not carry: a + * checkout reports a merge long before the merged listing is re-read. + */ + readonly threadPullRequest?: ThreadPullRequest | null; + readonly children: ReactNode; +}) { + const [handle] = useState(() => createPreviewCardHandle()); + const entries = useLoadedPullRequestEntries(); + const projects = useStore(useShallow(selectWorkspaceProjectsAcrossEnvironments)); + + const stateByKey = useMemo(() => { + const byKey = new Map(); + for (const entry of entries) { + byKey.set(chipKey(entry.repository, entry.number), { + state: entry.state, + isDraft: entry.isDraft, + }); + } + // The thread's own resolution wins: it is the one read that can see a + // branch land without waiting for the settled listing to come round again. + if (threadPullRequest?.repository) { + byKey.set(chipKey(threadPullRequest.repository, threadPullRequest.number), { + state: threadPullRequest.state, + isDraft: threadPullRequest.isDraft, + }); + } + return byKey; + }, [entries, threadPullRequest]); + + const scopeByRepository = useMemo(() => { + const byRepository = new Map(); + for (const project of projects) { + const repository = projectRepository(project); + if (repository === null || byRepository.has(repository.toLowerCase())) { + continue; + } + byRepository.set(repository.toLowerCase(), { + environmentId: project.environmentId, + projectId: project.id, + repository, + }); + } + return byRepository; + }, [projects]); + + const surface = useMemo( + () => ({ handle, scopeByRepository, stateByKey }), + [handle, scopeByRepository, stateByKey], + ); + + return ( + + {children} + + {({ payload }: { payload: PullRequestHoverCardPayload | undefined }) => ( + + {payload ? : null} + + )} + + + ); +} + +function PullRequestHoverCardContent({ + environmentId, + reference, + state, + isDraft, +}: PullRequestHoverCardPayload) { + const detail = useQuery(pullRequestDetailQueryOptions({ environmentId, reference })).data; + const tone = pullRequestBadgeTone(detail?.state ?? state, detail?.isDraft ?? isDraft); + const settledAt = detail ? (detail.mergedAt ?? detail.closedAt) : null; + const timestamp = detail ? formatRelativeTimeLabel(settledAt ?? detail.updatedAt) : null; + + return ( + <> +
+ + + {tone.label} + + + {reference.repository} #{reference.number} + + {timestamp ? ( + {timestamp} + ) : null} +
+

+ {detail?.title ?? "Loading…"} +

+ {detail ? ( +
+ + {detail.author?.login ?? "ghost"} + + + + + + {detail.changedFiles} {detail.changedFiles === 1 ? "file" : "files"} + + +
+ ) : null} + + ); +} diff --git a/apps/web/src/environments/primary/context.ts b/apps/web/src/environments/primary/context.ts index c865f7b56..01c16dbc2 100644 --- a/apps/web/src/environments/primary/context.ts +++ b/apps/web/src/environments/primary/context.ts @@ -68,6 +68,16 @@ export function readPrimaryEnvironmentDescriptor(): ExecutionEnvironmentDescript return usePrimaryEnvironmentBootstrapStore.getState().descriptor; } +/** + * The primary environment as this render sees it. Reactive, unlike + * {@link readPrimaryEnvironmentDescriptor}: a surface that renders before + * bootstrap finishes has to redraw when the descriptor lands, and one that + * only reads it once is stuck with whatever it saw first. + */ +export function usePrimaryEnvironmentDescriptor(): ExecutionEnvironmentDescriptor | null { + return usePrimaryEnvironmentBootstrapStore((state) => state.descriptor); +} + export function usePrimaryEnvironmentId(): EnvironmentId | null { return usePrimaryEnvironmentBootstrapStore((state) => state.descriptor?.environmentId ?? null); } diff --git a/apps/web/src/environments/primary/index.ts b/apps/web/src/environments/primary/index.ts index 897031c4f..c9dc60935 100644 --- a/apps/web/src/environments/primary/index.ts +++ b/apps/web/src/environments/primary/index.ts @@ -3,6 +3,7 @@ export { readPrimaryEnvironmentDescriptor, resetPrimaryEnvironmentDescriptorForTests, resolveInitialPrimaryEnvironmentDescriptor, + usePrimaryEnvironmentDescriptor, usePrimaryEnvironmentId, writePrimaryEnvironmentDescriptor, __resetPrimaryEnvironmentBootstrapForTests, diff --git a/apps/web/src/lib/pullRequestsReactQuery.ts b/apps/web/src/lib/pullRequestsReactQuery.ts index b06a82198..3be1e9ebe 100644 --- a/apps/web/src/lib/pullRequestsReactQuery.ts +++ b/apps/web/src/lib/pullRequestsReactQuery.ts @@ -12,11 +12,13 @@ import type { PullRequestReviewerKind, PullRequestUpdateMethod, } from "@threadlines/contracts"; +import type { PullRequestDetail } from "@threadlines/contracts"; import { keepPreviousData, mutationOptions, queryOptions, useQueries, + useQuery, type QueryClient, } from "@tanstack/react-query"; import { useMemo } from "react"; @@ -29,7 +31,7 @@ import { type PullRequestProjectFailure, } from "~/components/pull-requests/pullRequests.logic"; import { ensureEnvironmentApi } from "~/environmentApi"; -import { readPrimaryEnvironmentDescriptor, usePrimaryEnvironmentId } from "~/environments/primary"; +import { usePrimaryEnvironmentDescriptor, usePrimaryEnvironmentId } from "~/environments/primary"; import { useSavedEnvironmentRegistryStore, useSavedEnvironmentRuntimeStore, @@ -111,6 +113,34 @@ export function pullRequestDetailQueryOptions(input: PullRequestReadInput) { }); } +/** + * A pull request that may not exist yet, read on the same key the Pull request + * tab reads: one poll serves the tab, the composer's row and anything else on + * screen. Idle until there is something to address, which is why the key it + * would rest on names nothing real. + */ +export function usePullRequestDetail(input: { + readonly environmentId: EnvironmentId | null; + readonly reference: PullRequestRef | null; +}): PullRequestDetail | undefined { + const enabled = input.environmentId !== null && input.reference !== null; + return useQuery({ + ...pullRequestDetailQueryOptions({ + environmentId: input.environmentId ?? IDLE_ENVIRONMENT_ID, + reference: input.reference ?? IDLE_PULL_REQUEST_REF, + }), + enabled, + }).data; +} + +/** The key an idle read rests on. Never fetched, and names no real project. */ +const IDLE_ENVIRONMENT_ID = "" as EnvironmentId; +const IDLE_PULL_REQUEST_REF = { + projectId: "", + repository: "", + number: 0, +} as unknown as PullRequestRef; + export function pullRequestActivityQueryOptions(input: PullRequestReadInput) { return queryOptions({ queryKey: pullRequestQueryKeys.activity( @@ -448,7 +478,7 @@ export async function refreshPullRequestList( */ export function usePullRequestEnvironments(): readonly PullRequestEnvironment[] { const primaryEnvironmentId = usePrimaryEnvironmentId(); - const primaryDescriptor = readPrimaryEnvironmentDescriptor(); + const primaryDescriptor = usePrimaryEnvironmentDescriptor(); const primarySupported = primaryDescriptor?.capabilities.pullRequests === true; const primaryLabel = primaryDescriptor?.label ?? null; const savedEnvironmentsById = useSavedEnvironmentRegistryStore((state) => state.byId); diff --git a/apps/web/src/pullRequestReference.test.ts b/apps/web/src/pullRequestReference.test.ts index 2da8ca08b..b98b53b56 100644 --- a/apps/web/src/pullRequestReference.test.ts +++ b/apps/web/src/pullRequestReference.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vite-plus/test"; -import { parsePullRequestReference } from "./pullRequestReference"; +import { parsePullRequestReference, parsePullRequestUrl } from "./pullRequestReference"; describe("parsePullRequestReference", () => { it("accepts GitHub pull request URLs", () => { @@ -73,3 +73,29 @@ describe("parsePullRequestReference", () => { expect(parsePullRequestReference("feature/my-branch")).toBeNull(); }); }); + +describe("parsePullRequestUrl", () => { + it("names the repository and number a GitHub address points at", () => { + expect(parsePullRequestUrl("https://github.com/Threadlines/threadlines/pull/234")).toEqual({ + repository: "Threadlines/threadlines", + number: 234, + }); + }); + + it("resolves a link into one part of the same pull request", () => { + expect( + parsePullRequestUrl("https://github.com/Threadlines/threadlines/pull/234/files"), + ).toEqual({ repository: "Threadlines/threadlines", number: 234 }); + }); + + it("names nothing for the hosts whose rows nothing here lists by repository", () => { + expect(parsePullRequestUrl("https://gitlab.com/group/project/-/merge_requests/42")).toBeNull(); + expect( + parsePullRequestUrl("https://dev.azure.com/acme/project/_git/t3code/pullrequest/42"), + ).toBeNull(); + }); + + it("names nothing for a GitHub address that is not a pull request", () => { + expect(parsePullRequestUrl("https://github.com/Threadlines/threadlines/issues/234")).toBeNull(); + }); +}); diff --git a/apps/web/src/pullRequestReference.ts b/apps/web/src/pullRequestReference.ts index b919e736c..077f12248 100644 --- a/apps/web/src/pullRequestReference.ts +++ b/apps/web/src/pullRequestReference.ts @@ -1,5 +1,5 @@ const GITHUB_PULL_REQUEST_URL_PATTERN = - /^https:\/\/github\.com\/[^/\s]+\/[^/\s]+\/pull\/(\d+)(?:[/?#].*)?$/i; + /^https:\/\/github\.com\/(?[^/\s]+\/[^/\s]+)\/pull\/(?\d+)(?:[/?#].*)?$/i; const GITLAB_MERGE_REQUEST_URL_PATTERN = /^https:\/\/[^/\s]*gitlab[^/\s]*\/.+\/-\/merge_requests\/(\d+)(?:[/?#].*)?$/i; const AZURE_DEVOPS_PULL_REQUEST_URL_PATTERN = @@ -46,7 +46,8 @@ export function parsePullRequestReference(input: string): string | null { GITHUB_PULL_REQUEST_URL_PATTERN.exec(normalizedInput) ?? GITLAB_MERGE_REQUEST_URL_PATTERN.exec(normalizedInput) ?? AZURE_DEVOPS_PULL_REQUEST_URL_PATTERN.exec(normalizedInput); - if (urlMatch?.[1]) { + // Every pattern requires a number, so a match is already a whole reference. + if (urlMatch) { return normalizedInput; } @@ -57,3 +58,30 @@ export function parsePullRequestReference(input: string): string | null { return null; } + +/** A GitHub pull request the app can address by itself: its repository and number. */ +export interface PullRequestUrlReference { + /** `owner/name`, in the spelling the link used. */ + readonly repository: string; + readonly number: number; +} + +/** + * The pull request a web address points at, or null when it points at + * something else. GitHub only for now: the chip this feeds colours itself from + * listings the app already holds, and nothing lists GitLab or Azure DevOps + * rows by repository yet. + * + * A deeper link into the same pull request (its files, one comment) still + * names it, so it resolves the same way. Whether a click opens the app's own + * tab is the stricter question `isLinkToPullRequest` answers. + */ +export function parsePullRequestUrl(href: string): PullRequestUrlReference | null { + const match = GITHUB_PULL_REQUEST_URL_PATTERN.exec(href.trim()); + const repository = match?.groups?.["repository"]; + const number = Number(match?.groups?.["number"] ?? Number.NaN); + if (repository === undefined || !Number.isSafeInteger(number) || number <= 0) { + return null; + } + return { repository, number }; +} diff --git a/apps/web/src/routes/_chat.$environmentId.$threadId.tsx b/apps/web/src/routes/_chat.$environmentId.$threadId.tsx index 9e5dd0c70..0db4554ca 100644 --- a/apps/web/src/routes/_chat.$environmentId.$threadId.tsx +++ b/apps/web/src/routes/_chat.$environmentId.$threadId.tsx @@ -31,9 +31,12 @@ import { import { useGitStatus } from "../lib/gitStatusState"; import { PULL_REQUEST_COUNT_REFETCH_INTERVAL_MS, + usePullRequestDetail, usePullRequestLists, } from "../lib/pullRequestsReactQuery"; import { LazyPullRequestDetailPanel } from "../components/pull-requests/LazyPullRequestDetailPanel"; +import { PullRequestHoverCardProvider } from "../components/pull-requests/PullRequestHoverCard"; +import type { ComposerPullRequest } from "../components/chat/ComposerPullRequestRow"; import { resolveThreadPullRequest } from "../components/pull-requests/pullRequests.logic"; import { Button } from "~/components/ui/button"; import { Empty, EmptyContent, EmptyDescription, EmptyHeader } from "~/components/ui/empty"; @@ -484,6 +487,25 @@ function ChatThreadRouteView() { : null, [selectTab, threadPullRequest, threadPullRequestReference], ); + // The composer's docked row reads the same detail the Pull request tab does, + // on the same key: one poll while checks run, whichever of them is on screen. + const threadPullRequestDetail = usePullRequestDetail({ + environmentId: threadPullRequestReference ? (threadRef?.environmentId ?? null) : null, + reference: threadPullRequestReference, + }); + const composerPullRequest = useMemo( + () => + threadRef && threadPullRequest && threadPullRequestReference + ? { + environmentId: threadRef.environmentId, + reference: threadPullRequestReference, + pullRequest: threadPullRequest, + detail: threadPullRequestDetail, + onOpen: () => selectTab("pullRequest"), + } + : null, + [selectTab, threadPullRequest, threadPullRequestDetail, threadPullRequestReference, threadRef], + ); const closeTab = useCallback( (tab: RightPanelTab) => { const nextTab = closeRightPanelTab(currentThreadKey, tab); @@ -655,14 +677,17 @@ function ChatThreadRouteView() { <> - + + + - + + + + + + + + Mockup: pull request strip, chat chips, and hover card + + + +
+ Mockup pull request strip above the composer, PR chips in chat, shared hover + card. + + Placement: + + + + + + Scenario: 1 of 6 + + + +
+ +
+ + +
+
+ Threadlines/Nightly .275 update logs +
+ +
+
+
+

+ Step 5 shrinks to one sentence. The "one concern per PR" rule already exists + in the repo guidelines and + + + + #221 + ignored it, so a new rule would not have helped there. What is worth adding is a + single line in AGENTS.md saying that migrations touching existing rows must be timed + against a big database before merge. +

+

+ So the plan becomes: PR one is the migration fix. PR two is "desktop waits while + migrating" plus the per-migration timing log. And one sentence in the guidelines. + Nothing else. +

+
+
alright yeah lets do that plan
+
+

+ Starting with PR 1: adding the one-line guideline, committing the migration fix on a + descriptively named branch, pushing, and opening the PR. +

+
Ran 8 commands ›
+
+ PR + + + + #234 + is open. I found that SQLite's synchronous driver freezes the backend during long + statements, so the readiness signal must be written before migration starts rather + than reported mid-process. Now gathering the runtime-state file pattern and the test + harness to write PR 2's spec. +
+
Ran 5 commands ›
+ +
+ ✸ + 3m 41s · 2.6k tokens · Almost done thinking… +
+
+
+
+ +
+
+
+ +
+
+
+
Ask a follow-up, or describe the next change…
+
+ ✸ Fable 5.1 ▾ + ☰ High ▾ + Build + Auto ▾ + + ⌂+ + ↑ + +
+
+
+
+ ⎇ Worktree · threadlines/nightly-275 + + From main ▾ +
+ +
+ +
+ Placement buttons at the top switch where the PRs live. Try it: hover any + #234 chip for the card. Click the CI chip on a strip row for the + checks popover. Advance the scenario to watch the rows change state: open → checks pass → + second PR → queued → merged → wrapped up. +
+
+
+ + +
+
+ + + +
+
+
+ badcuban + +
+
+ + +
+
+ Checks +
+
+
+ + + +
+
+ + + +