diff --git a/apps/web/src/components/LegacySidebar.tsx b/apps/web/src/components/LegacySidebar.tsx index 189ffaa0fb9e..9b20b0b3da66 100644 --- a/apps/web/src/components/LegacySidebar.tsx +++ b/apps/web/src/components/LegacySidebar.tsx @@ -22,6 +22,11 @@ import { ThreadWorktreeIndicator, useLinkedThreadPullRequest, } from "./ThreadStatusIndicators"; +import { + openOnHostLabel, + showPullRequestLinkContextMenu, +} from "./pullRequest/pullRequestLinkContextMenu"; +import { useUnlinkThreadPullRequest } from "./pullRequest/useUnlinkThreadPullRequest"; import { ProjectFavicon } from "./ProjectFavicon"; import { useAtomValue } from "@effect/atom-react"; import { autoAnimate } from "@formkit/auto-animate"; @@ -462,6 +467,7 @@ export const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThr thread.environmentId, thread.linkedPullRequest, ); + const unlinkThreadPullRequest = useUnlinkThreadPullRequest(threadRef); const pr = thread.linkedPullRequest == null ? resolveThreadPr({ threadBranch: thread.branch, gitStatus: gitStatus.data }) @@ -470,6 +476,24 @@ export const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThr pr, linkedPullRequestStatus?.sourceControlProvider ?? gitStatus.data?.sourceControlProvider, ); + // Same gesture as the current sidebar's number: without it the right-click bubbles to the row + // and offers the thread menu, which has nothing to say about the pull request under the cursor. + const prSourceControlProvider = + linkedPullRequestStatus?.sourceControlProvider ?? gitStatus.data?.sourceControlProvider; + const handlePrContextMenu = useCallback( + (event: React.MouseEvent) => { + if (!prStatus) return; + event.preventDefault(); + event.stopPropagation(); + void showPullRequestLinkContextMenu({ + url: prStatus.url, + openLabel: openOnHostLabel(prSourceControlProvider?.kind ?? ""), + position: { x: event.clientX, y: event.clientY }, + unlinkFromThread: thread.linkedPullRequest == null ? null : unlinkThreadPullRequest, + }); + }, + [prSourceControlProvider, prStatus, thread.linkedPullRequest, unlinkThreadPullRequest], + ); const terminalStatus = terminalStatusFromRunningIds(runningTerminalIds); const isConfirmingArchive = confirmingArchiveThreadKey === threadKey && !isThreadRunning; const threadMetaClassName = isConfirmingArchive @@ -713,6 +737,7 @@ export const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThr className={`inline-flex items-center justify-center ${prStatus.colorClass} cursor-pointer rounded-sm outline-hidden focus-visible:ring-1 focus-visible:ring-ring`} onPointerDown={(event) => event.stopPropagation()} onClick={handlePrClick} + onContextMenu={handlePrContextMenu} > diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index a5df7bc53538..6f887bdd2b1f 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -168,6 +168,11 @@ import { snoozeWakeLabel, type SnoozePreset, } from "./Sidebar.snooze"; +import { + openOnHostLabel, + showPullRequestLinkContextMenu, +} from "./pullRequest/pullRequestLinkContextMenu"; +import { useUnlinkThreadPullRequest } from "./pullRequest/useUnlinkThreadPullRequest"; import { ProjectFavicon } from "./ProjectFavicon"; import { ProviderInstanceIcon } from "./chat/ProviderInstanceIcon"; import { getTriggerDisplayModelLabel } from "./chat/providerIconUtils"; @@ -794,6 +799,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { const lastVisitedAt = useUiStateStore((state) => state.threadLastVisitedAtById[threadKey]); const isSelected = useThreadSelectionStore((state) => state.selectedThreadKeys.has(threadKey)); const openPrLink = useOpenPrLink(); + const unlinkThreadPullRequest = useUnlinkThreadPullRequest(threadRef); const runningTerminalIds = useThreadRunningTerminalIds({ environmentId: thread.environmentId, threadId: thread.id, @@ -1117,6 +1123,25 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { }, [onThreadActivate, openPrLink, openPullRequestsInRightPanel, pr, props.isActive, threadRef], ); + // Right-clicking the number reaches the pull request itself. Without this the event bubbles to + // the row and opens the thread menu, which is why the link a thread settles on had nowhere to be + // undone except the message the agent originally wrote it in. + const handlePrContextMenu = useCallback( + (event: ReactMouseEvent) => { + if (!pr?.url) return; + event.preventDefault(); + event.stopPropagation(); + void showPullRequestLinkContextMenu({ + url: pr.url, + openLabel: openOnHostLabel(prProvider?.kind ?? ""), + position: { x: event.clientX, y: event.clientY }, + // Only the linked number can be unlinked: the same badge also shows a pull request read + // off the thread's branch, and that one is a fact about git, not a choice to undo. + unlinkFromThread: thread.linkedPullRequest == null ? null : unlinkThreadPullRequest, + }); + }, + [pr, prProvider, thread.linkedPullRequest, unlinkThreadPullRequest], + ); // All sidebar rows share one surface model. Live threads used to look // like elevated cards while settled threads were plain rows, leaving neither @@ -1191,6 +1216,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { rel="noopener noreferrer" onPointerDown={(event) => event.stopPropagation()} onClick={handlePrClick} + onContextMenu={handlePrContextMenu} className={cn( // Sidebar chrome follows the interface font; tabular digits keep the // number from reflowing as PR states stream in. diff --git a/apps/web/src/components/chat/ChatHeader.tsx b/apps/web/src/components/chat/ChatHeader.tsx index dbba327489ac..d661512d4366 100644 --- a/apps/web/src/components/chat/ChatHeader.tsx +++ b/apps/web/src/components/chat/ChatHeader.tsx @@ -23,6 +23,7 @@ import { type MouseEvent as ReactMouseEvent, } from "react"; import GitActionsControl from "../GitActionsControl"; +import { ThreadPullRequestPill } from "../pullRequest/ThreadPullRequestPill"; import { isTrailingDoubleClick } from "../Sidebar.logic"; import { type DraftId } from "~/composerDraftStore"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; @@ -403,11 +404,14 @@ export const ChatHeader = memo(function ChatHeader({ {activeProjectName && ( )} + {/* Beside the git actions rather than among them: it reports what this thread is tied to, + and it is the only place in the thread that says so. Renders nothing when unlinked. */} + ); diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestPill.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestPill.tsx new file mode 100644 index 000000000000..da999035b7ed --- /dev/null +++ b/apps/web/src/components/pullRequest/ThreadPullRequestPill.tsx @@ -0,0 +1,93 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; +import { useCallback, type MouseEvent as ReactMouseEvent } from "react"; + +import { cn } from "~/lib/utils"; +import { useOpenPrLink } from "~/lib/openPullRequestLink"; +import { useThreadShell } from "~/state/entities"; + +import { + ChangeRequestStatusIcon, + PrStatusTooltipContent, + prStatusIndicator, + useLinkedThreadPullRequest, +} from "../ThreadStatusIndicators"; +import { buttonVariants } from "../ui/button"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; +import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; +import { useUnlinkThreadPullRequest } from "./useUnlinkThreadPullRequest"; + +/** + * The pull request a thread is linked to, sitting beside the git actions in the header. + * + * Only the link is shown here, never a pull request merely read off the thread's branch: this + * chip exists because the link is otherwise invisible from the thread you are reading, and it is + * the link — not the branch — that settles the thread once the pull request merges. It behaves + * like the sidebar's number, down to the right-click that undoes it. + */ +export function ThreadPullRequestPill({ threadRef }: { readonly threadRef: ScopedThreadRef }) { + const linkedPullRequest = useThreadShell(threadRef)?.linkedPullRequest ?? null; + const linkedStatus = useLinkedThreadPullRequest(threadRef.environmentId, linkedPullRequest); + const openPrLink = useOpenPrLink(threadRef); + const unlinkThreadPullRequest = useUnlinkThreadPullRequest(threadRef); + + const url = linkedPullRequest?.url ?? null; + const handleClick = useCallback( + (event: ReactMouseEvent) => { + if (url !== null) openPrLink(event, url); + }, + [openPrLink, url], + ); + const handleContextMenu = useCallback( + (event: ReactMouseEvent) => { + if (url === null) return; + event.preventDefault(); + event.stopPropagation(); + void showPullRequestLinkContextMenu({ + url, + openLabel: openOnHostLabel(linkedStatus?.sourceControlProvider.kind ?? ""), + position: { x: event.clientX, y: event.clientY }, + unlinkFromThread: unlinkThreadPullRequest, + }); + }, + [linkedStatus, unlinkThreadPullRequest, url], + ); + + if (linkedPullRequest === null || url === null) return null; + + // The number comes from the link itself, so the chip renders at once and only takes on its + // open/merged/closed colour once the provider answers. Waiting for that would blink a control + // in and out of the header on every thread switch. + const status = prStatusIndicator(linkedStatus?.pr ?? null, linkedStatus?.sourceControlProvider); + const label = `#${linkedPullRequest.number}`; + + return ( + + + } + > + + {label} + + + {status ? : `Pull request ${label}`} + + + ); +} diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts index db105f97fe95..c381899799ec 100644 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; -import { openOnHostLabel, pullRequestLinkContextMenuItems } from "./pullRequestLinkContextMenu"; +import { + openOnHostLabel, + pullRequestLinkContextMenuItems, + showPullRequestLinkContextMenu, +} from "./pullRequestLinkContextMenu"; describe("pull request link context menu", () => { it("offers the copy first and the host's own page after it", () => { @@ -10,6 +14,46 @@ describe("pull request link context menu", () => { ]); }); + it("leaves unlinking out until the caller says this number is the thread's own", () => { + expect(pullRequestLinkContextMenuItems("Open on GitHub", false)).toHaveLength(2); + expect( + pullRequestLinkContextMenuItems("Open on GitHub", false).some( + (item) => item.id === "unlink-from-thread", + ), + ).toBe(false); + }); + + it("puts unlinking last, behind a divider, so a misclick lands on copy instead", () => { + const items = pullRequestLinkContextMenuItems("Open on GitHub", true); + expect(items).toEqual([ + { id: "copy-link", label: "Copy link", icon: "copy" }, + { id: "open-external", label: "Open on GitHub" }, + { id: "unlink-from-thread", label: "Unlink from thread", separatorBefore: true }, + ]); + }); + + it("tells the unlink callback which url was acted on, so a stale menu can decline", async () => { + const acted: string[] = []; + // These suites run on node, so the desktop bridge the menu reaches for is stood up here + // rather than in a DOM. Only `contextMenu.show` is exercised, and it answers from the bridge. + const globals = globalThis as { window?: unknown }; + const previousWindow = globals.window; + globals.window = { desktopBridge: { showContextMenu: async () => "unlink-from-thread" } }; + try { + await showPullRequestLinkContextMenu({ + url: "https://github.com/pingdotgg/t3code/pull/23", + openLabel: "Open on GitHub", + position: { x: 0, y: 0 }, + unlinkFromThread: async (url) => { + acted.push(url); + }, + }); + } finally { + globals.window = previousWindow; + } + expect(acted).toEqual(["https://github.com/pingdotgg/t3code/pull/23"]); + }); + it("names every host it knows, and says nothing false about one it does not", () => { expect(openOnHostLabel("github")).toBe("Open on GitHub"); expect(openOnHostLabel("gitlab")).toBe("Open on GitLab"); diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts index 16b749445d4c..fde9cf7c7fcf 100644 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts +++ b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts @@ -5,7 +5,7 @@ import { readLocalApi } from "~/localApi"; import { toastManager } from "../ui/toast"; -export type PullRequestLinkContextMenuAction = "copy-link" | "open-external"; +export type PullRequestLinkContextMenuAction = "copy-link" | "open-external" | "unlink-from-thread"; /** Named for the host rather than "externally": the point is where you will land. */ export const OPEN_ON_HOST_LABELS: Partial> = { @@ -18,14 +18,29 @@ export const OPEN_ON_HOST_LABELS: Partial> = { export const openOnHostLabel = (provider: string): string => OPEN_ON_HOST_LABELS[provider] ?? "Open on host"; -/** Copy first: it is the reason to right-click a number rather than click it. */ +/** + * Copy first: it is the reason to right-click a number rather than click it. + * + * Unlinking comes last, behind a divider, because it is the one item here that changes the thread + * rather than the clipboard or the browser — and because a thread only settles on its own once a + * link exists, so reaching for it is rare next to the two above it. + */ export function pullRequestLinkContextMenuItems( openLabel: string, + canUnlinkFromThread = false, ): readonly ContextMenuItem[] { - return [ + const items: ContextMenuItem[] = [ { id: "copy-link", label: "Copy link", icon: "copy" }, { id: "open-external", label: openLabel }, ]; + if (canUnlinkFromThread) { + items.push({ + id: "unlink-from-thread", + label: "Unlink from thread", + separatorBefore: true, + }); + } + return items; } /** @@ -41,16 +56,28 @@ export async function showPullRequestLinkContextMenu({ url, openLabel, position, + unlinkFromThread, }: { readonly url: string; readonly openLabel: string; readonly position: { readonly x: number; readonly y: number }; + /** + * Absent where the number being right-clicked is not the one its thread is linked to — a pull + * request read off a branch, a row on the list page, a server that does not record links at all. + * + * Handed the URL the menu was opened on, so it can decline once that is no longer the link the + * thread holds. + */ + readonly unlinkFromThread?: ((url: string) => Promise) | null | undefined; }): Promise { const api = readLocalApi(); if (!api) return; let action: PullRequestLinkContextMenuAction | null = null; try { - action = await api.contextMenu.show(pullRequestLinkContextMenuItems(openLabel), position); + action = await api.contextMenu.show( + pullRequestLinkContextMenuItems(openLabel, unlinkFromThread != null), + position, + ); } catch { // A menu that could not be shown has already cost the reader their right-click; there is // nothing to say about it that a second popup would not make worse. @@ -59,10 +86,16 @@ export async function showPullRequestLinkContextMenu({ try { if (action === "copy-link") await writeTextToClipboard(url, "link"); else if (action === "open-external") await api.shell.openExternal(url); + else if (action === "unlink-from-thread") await unlinkFromThread?.(url); } catch { toastManager.add({ type: "error", - title: action === "copy-link" ? "Could not copy the link" : "Could not open the link", + title: + action === "copy-link" + ? "Could not copy the link" + : action === "unlink-from-thread" + ? "Could not unlink the pull request" + : "Could not open the link", }); } } diff --git a/apps/web/src/components/pullRequest/useUnlinkThreadPullRequest.ts b/apps/web/src/components/pullRequest/useUnlinkThreadPullRequest.ts new file mode 100644 index 000000000000..af92dce207d9 --- /dev/null +++ b/apps/web/src/components/pullRequest/useUnlinkThreadPullRequest.ts @@ -0,0 +1,54 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; +import { + isAtomCommandInterrupted, + squashAtomCommandFailure, +} from "@t3tools/client-runtime/state/runtime"; +import { useCallback } from "react"; + +import { matchesLinkedPullRequestUrl } from "~/lib/openPullRequestLink"; +import { readThreadShell, useServerConfigs } from "~/state/entities"; +import { threadEnvironment } from "~/state/threads"; +import { useAtomCommand } from "~/state/use-atom-command"; + +/** + * Clears the pull request a thread is linked to, or null where this server cannot store the link + * in the first place. + * + * Null is the answer a caller leaves the action out of its menu for: the link is what makes a + * thread settle when its pull request merges, so offering to undo it on a server that never + * recorded one would promise something nothing can honour. + * + * The returned function is given the URL the reader acted on, and drops the request when that is + * no longer the link the thread holds. A context menu is open for as long as it takes to read it, + * and in that time the agent can write a newer pull request link or another device can change the + * thread — clearing whatever happens to be there by then would unlink something nobody chose. + */ +export function useUnlinkThreadPullRequest( + threadRef: ScopedThreadRef | null | undefined, +): ((url: string) => Promise) | null { + const serverConfigs = useServerConfigs(); + const updateThreadMetadata = useAtomCommand(threadEnvironment.updateMetadata, { + reportFailure: false, + }); + const unlink = useCallback( + async (url: string) => { + if (threadRef == null) return; + const current = readThreadShell(threadRef)?.linkedPullRequest; + if (current == null || !matchesLinkedPullRequestUrl(current, url)) return; + const result = await updateThreadMetadata({ + environmentId: threadRef.environmentId, + input: { threadId: threadRef.threadId, linkedPullRequest: null }, + }); + if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { + throw squashAtomCommandFailure(result); + } + }, + [threadRef, updateThreadMetadata], + ); + + if (threadRef == null) return null; + return serverConfigs.get(threadRef.environmentId)?.environment.capabilities + .threadPullRequestLinking === true + ? unlink + : null; +} diff --git a/docs/user/thread-sidebar.md b/docs/user/thread-sidebar.md index f0fbfdccbd6c..a43893546754 100644 --- a/docs/user/thread-sidebar.md +++ b/docs/user/thread-sidebar.md @@ -15,7 +15,12 @@ away. Its timestamps do not change. Other threads keep their positions. Right-click a pull request link in a thread and choose **Link to thread** to show that pull request in the sidebar. The thread settles when the linked pull request merges if **Auto-settle merged -threads** is enabled. Right-click the same link and choose **Unlink from thread** to remove it. +threads** is enabled. + +A linked pull request shows its number in the thread's sidebar row and beside the git actions in +the thread header. Click either one to open the pull request. Right-click either one, or the +original link in the conversation, and choose **Unlink from thread** to remove the link and stop +the thread settling when that pull request merges. On web and desktop, drag a pinned thread to change its position. On mobile, open the thread's menu and choose **Move up** or **Move down**. The order is stored by the server and appears on your