From 9eb52a12ff52943688fc3bf50d67992bc221cae3 Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Sun, 6 Sep 2026 04:06:22 -0400 Subject: [PATCH] feat(web): dock the thread's pull request on the composer The sidebar badge says a thread has a pull request and the Pull request tab says everything about it, but neither is in view while writing the next message, which is when "did the checks pass" matters. The composer's top edge now carries a row for the thread's pull request: state glyph and number, project, branch, diff stat, and a CI chip whose popover lists the check counts, a link to the checks page, and switches for GitHub auto-merge and the wrap-up-on-settle setting. The notice rows share the same frame under it. GitHub pull request links in a transcript render as icon-and-number chips, and both the chips and the row's number share one hover card with the state, repository, title, author and size. The row reads the same detail query as the tab, so one poll serves both. Along the way: the pull request listing read the primary environment descriptor through a non-reactive getter, so a surface mounted before bootstrap finished never saw the capability arrive and the listing stayed disabled. It now reads the reactive selector. --- .../src/components/ChatMarkdown.browser.tsx | 42 +- apps/web/src/components/ChatMarkdown.tsx | 86 + apps/web/src/components/ChatView.browser.tsx | 200 +++ apps/web/src/components/ChatView.tsx | 8 + apps/web/src/components/chat/ChatComposer.tsx | 22 +- apps/web/src/components/chat/ComposerDock.tsx | 49 + .../components/chat/ComposerNoticeDock.tsx | 19 +- .../chat/ComposerPullRequestRow.tsx | 301 ++++ .../web/src/components/chat/DiffStatLabel.tsx | 14 +- .../chat/composerPullRequest.logic.test.ts | 180 +++ .../chat/composerPullRequest.logic.ts | 164 ++ .../pull-requests/PullRequestHoverCard.tsx | 279 ++++ apps/web/src/environments/primary/context.ts | 10 + apps/web/src/environments/primary/index.ts | 1 + apps/web/src/lib/pullRequestsReactQuery.ts | 34 +- apps/web/src/pullRequestReference.test.ts | 28 +- apps/web/src/pullRequestReference.ts | 32 +- .../routes/_chat.$environmentId.$threadId.tsx | 58 +- docs/mockups/pull-request-strip.html | 1437 +++++++++++++++++ 19 files changed, 2923 insertions(+), 41 deletions(-) create mode 100644 apps/web/src/components/chat/ComposerDock.tsx create mode 100644 apps/web/src/components/chat/ComposerPullRequestRow.tsx create mode 100644 apps/web/src/components/chat/composerPullRequest.logic.test.ts create mode 100644 apps/web/src/components/chat/composerPullRequest.logic.ts create mode 100644 apps/web/src/components/pull-requests/PullRequestHoverCard.tsx create mode 100644 docs/mockups/pull-request-strip.html 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 +
+
+
+ + + +
+
+ + + +