From 3260b22edeef509eca87096d57a5d5243c139c8b Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Tue, 15 Sep 2026 10:01:45 -0400 Subject: [PATCH 1/5] feat(web): unlink a pull request by right-clicking its number Unlinking a pull request from a thread was reachable from the thread's pull-requests panel, or from a right-click on the original URL in the transcript. Neither is where a reader is looking when they notice the wrong number: that is the badge on the sidebar row and the pill in the branch toolbar, and a right-click there opened the platform's own edit menu. Those numbers now carry the same menu the transcript link does, with **Unlink from thread** last and behind a divider. It is offered only for a pull request the thread is actually linked to; the number a thread picked up from its branch keeps copy and open, because nothing linked it to undo. Whether the thread is linked is read when the menu opens and again when the item is chosen. A menu sits open for as long as it takes to read, and the agent can link or unlink from underneath it in that time, so clearing whatever happens to be there by then would unlink something nobody chose. The work goes through `usePullRequestLinking`, so servers on `thread.pull-request.unlink` and older ones on the legacy `linkedPullRequest` metadata both route correctly, and a server that supports neither offers nothing. Co-Authored-By: Claude Opus 5 --- .../BranchToolbarBranchSelector.tsx | 10 +++ apps/web/src/components/LegacySidebar.tsx | 14 ++++ apps/web/src/components/Sidebar.tsx | 12 +++ .../src/components/ThreadStatusIndicators.tsx | 4 + .../pullRequestLinkContextMenu.test.ts | 76 ++++++++++++++++++- .../pullRequest/pullRequestLinkContextMenu.ts | 40 ++++++++-- .../useThreadPullRequestLinkContextMenu.ts | 57 ++++++++++++++ docs/user/thread-sidebar.md | 6 ++ 8 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index ba5251538685..0692c869d954 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -58,6 +58,7 @@ import { resolveThreadPullRequestBadge, useLinkedThreadPullRequest, } from "./ThreadStatusIndicators"; +import { useThreadPullRequestLinkContextMenu } from "./pullRequest/useThreadPullRequestLinkContextMenu"; import { Button } from "./ui/button"; import { Switch } from "./ui/switch"; import { getVirtualizedScrollFadeClassName } from "./ui/scroll-area"; @@ -671,6 +672,7 @@ export function BranchToolbarBranchSelector({ const prNumber = currentLinkedPr?.number ?? displayedPr?.number; const prUrl = currentLinkedPr?.url ?? displayedPr?.url; const openPrLink = useOpenPrLink(threadRef); + const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); function renderPickerItem(itemValue: string, index: number) { if (checkoutPullRequestItemValue && itemValue === checkoutPullRequestItemValue) { @@ -783,6 +785,14 @@ export function BranchToolbarBranchSelector({ onOpenPullRequest={(event) => { if (prUrl) openPrLink(event, prUrl); }} + onContextMenuPullRequest={(event) => + openPrContextMenu(event, { + url: prUrl, + providerKind: + linkedStatus?.sourceControlProvider.kind ?? + branchStatusQuery.data?.sourceControlProvider?.kind, + }) + } /> {/* Context menu lives on the wrapper: the disabled Button has pointer-events-none, so the trigger itself never sees right-clicks diff --git a/apps/web/src/components/LegacySidebar.tsx b/apps/web/src/components/LegacySidebar.tsx index 81fd6047f13e..dd165609f7d9 100644 --- a/apps/web/src/components/LegacySidebar.tsx +++ b/apps/web/src/components/LegacySidebar.tsx @@ -22,6 +22,7 @@ import { ThreadWorktreeIndicator, useLinkedThreadPullRequest, } from "./ThreadStatusIndicators"; +import { useThreadPullRequestLinkContextMenu } from "./pullRequest/useThreadPullRequestLinkContextMenu"; import { EnvironmentMachineIcon } from "./EnvironmentMachineIcon"; import { ProjectFavicon } from "./ProjectFavicon"; import { useAtomValue } from "@effect/atom-react"; @@ -609,6 +610,17 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP threadRef, ], ); + const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); + // Without this the row's own menu answers, because the number sits inside the row. + const handlePrContextMenu = useCallback( + (event: React.MouseEvent) => { + openPrContextMenu(event, { + url: prStatus?.url ?? currentLinkedPr?.url, + providerKind: linkedPullRequestStatus?.sourceControlProvider.kind, + }); + }, + [currentLinkedPr, linkedPullRequestStatus, openPrContextMenu, prStatus], + ); const handleRenameInputRef = useCallback( (element: HTMLInputElement | null) => { if (element && renamingInputRef.current !== element) { @@ -733,6 +745,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP 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} > event.stopPropagation()} onClick={handlePrClick} + onContextMenu={handlePrContextMenu} className="text-muted-foreground" aria-label={`PR #${currentLinkedPr.number}, status pending`} > diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index e5a86ff98a3c..f1703a6cd130 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -203,6 +203,7 @@ import { type TerminalStatusIndicator, useLinkedThreadPullRequest, } from "./ThreadStatusIndicators"; +import { useThreadPullRequestLinkContextMenu } from "./pullRequest/useThreadPullRequestLinkContextMenu"; import { resolveSnoozePresets, snoozeWakeDescription, @@ -1393,6 +1394,16 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { threadRef, ], ); + const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); + const handlePrContextMenu = useCallback( + (event: ReactMouseEvent) => { + openPrContextMenu(event, { + url: pr?.url ?? currentLinkedPr?.url, + providerKind: linkedPullRequestStatus?.sourceControlProvider.kind, + }); + }, + [currentLinkedPr, linkedPullRequestStatus, openPrContextMenu, pr], + ); // All sidebar rows share one surface model. Live threads used to look // like elevated cards while settled threads were plain rows, leaving neither @@ -1514,6 +1525,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { status={prStatus} onOpenStack={handlePrStackClick} onOpenPullRequest={handlePrClick} + onContextMenuPullRequest={handlePrContextMenu} /> ) : null; const terminalStatusIcon = terminalStatus ? ( diff --git a/apps/web/src/components/ThreadStatusIndicators.tsx b/apps/web/src/components/ThreadStatusIndicators.tsx index 7201779f13f1..3f3b10e48bd1 100644 --- a/apps/web/src/components/ThreadStatusIndicators.tsx +++ b/apps/web/src/components/ThreadStatusIndicators.tsx @@ -146,6 +146,7 @@ export function ThreadPullRequestBadgeControl({ status, onOpenStack, onOpenPullRequest, + onContextMenuPullRequest, }: { variant: "underline" | "ghost"; badge: ThreadPullRequestBadge | null; @@ -154,6 +155,8 @@ export function ThreadPullRequestBadgeControl({ status: PrStatusIndicator | null; onOpenStack: () => void; onOpenPullRequest: (event: MouseEvent) => void; + /** Only the single-pull-request shape carries one; a stack has no one number to act on. */ + onContextMenuPullRequest?: ((event: MouseEvent) => void) | undefined; }) { const isStack = badge?.kind === "stack"; const linkedCount = badge?.kind === "pull-request" && badge.others > 0 ? badge.others + 1 : null; @@ -206,6 +209,7 @@ export function ThreadPullRequestBadgeControl({ aria-label={label} onPointerDown={(event) => event.stopPropagation()} onClick={onOpenPullRequest} + {...(onContextMenuPullRequest ? { onContextMenu: onContextMenuPullRequest } : {})} /> ) } diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts index eb6f47b4c803..a580f6eafef5 100644 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts @@ -1,6 +1,52 @@ +import type { ContextMenuItem } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; -import { openOnHostLabel } from "./pullRequestLinkContextMenu"; +import { + openOnHostLabel, + showPullRequestLinkContextMenu, + type PullRequestLinkContextMenuAction, +} from "./pullRequestLinkContextMenu"; + +type Items = readonly ContextMenuItem[]; + +const URL = "https://github.com/pingdotgg/t3code/pull/23"; + +/** + * Opens the menu against a stubbed desktop bridge and hands back what it offered. + * + * These suites run on node, so the bridge `readLocalApi` reaches for is stood up here rather than + * in a DOM. Going through the exported entry point rather than the items helper keeps the test on + * the surface callers actually use. + */ +async function openMenu( + options: { + readonly unlinkFromThread?: ((url: string) => Promise) | undefined; + }, + choose: PullRequestLinkContextMenuAction | null = null, +): Promise { + let items: Items = []; + const globals = globalThis as { window?: unknown }; + const previousWindow = globals.window; + globals.window = { + desktopBridge: { + showContextMenu: async (shown: Items) => { + items = shown; + return choose; + }, + }, + }; + try { + await showPullRequestLinkContextMenu({ + url: URL, + openLabel: "Open on GitHub", + position: { x: 0, y: 0 }, + ...(options.unlinkFromThread ? { unlinkFromThread: options.unlinkFromThread } : {}), + }); + } finally { + globals.window = previousWindow; + } + return items; +} describe("pull request link context menu", () => { it("names every host it knows, and says nothing false about one it does not", () => { @@ -10,4 +56,32 @@ describe("pull request link context menu", () => { expect(openOnHostLabel("azure-devops")).toBe("Open on Azure DevOps"); expect(openOnHostLabel("something-else")).toBe("Open on host"); }); + + it("leaves unlinking out until the caller says this number is the thread's own", async () => { + expect(await openMenu({})).toEqual([ + { id: "copy-link", label: "Copy link", icon: "copy" }, + { id: "open-external", label: "Open on GitHub" }, + ]); + }); + + it("puts unlinking last, behind a divider, so a misclick lands on copy instead", async () => { + expect(await openMenu({ unlinkFromThread: async () => {} })).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[] = []; + await openMenu( + { + unlinkFromThread: async (url) => { + acted.push(url); + }, + }, + "unlink-from-thread", + ); + expect(acted).toEqual([URL]); + }); }); diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts index 16c4fa5d3559..d123116328fe 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. */ const OPEN_ON_HOST_LABELS: Partial> = { @@ -19,14 +19,25 @@ 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 a misclick on it is the only one that costs + * anything. + */ function pullRequestLinkContextMenuItems( openLabel: string, + canUnlinkFromThread: boolean, ): 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; } /** @@ -42,16 +53,29 @@ export async function showPullRequestLinkContextMenu({ url, openLabel, position, + unlinkFromThread, }: { readonly url: string; readonly openLabel: string; readonly position: { readonly x: number; readonly y: number }; + /** + * Absent wherever this number is not one the thread is linked to: a pull request read off a + * branch, a row on the list page, a server too old to record links at all. + * + * Handed the URL the menu was opened on, so it can decline once that is no longer a link the + * thread holds. A menu stays open for as long as it takes to read, and in that time the agent + * can link or unlink from the same thread. + */ + readonly unlinkFromThread?: ((url: string) => Promise) | 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 !== undefined), + 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. @@ -60,10 +84,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/useThreadPullRequestLinkContextMenu.ts b/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts new file mode 100644 index 000000000000..6812c3f74316 --- /dev/null +++ b/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts @@ -0,0 +1,57 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; +import { useCallback, type MouseEvent as ReactMouseEvent } from "react"; + +import { usePullRequestLinking } from "~/hooks/usePullRequestLinking"; +import { readThreadShell } from "~/state/entities"; + +import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; + +/** + * The right-click behind every pull request number a thread wears: the sidebar row, the composer's + * branch toolbar, the legacy sidebar. + * + * Those numbers are the only place a link is visible from the thread you are reading, so they are + * where a reader goes to undo one. Unlinking was reachable before this only from the right panel or + * from the original URL in the transcript, which may be hundreds of turns up or may never have been + * a message at all when the agent linked the pull request itself. + * + * Whether the thread is linked is read when the menu opens, and again when the item is chosen: a + * menu sits open for as long as it takes to read, and the agent can link or unlink from underneath + * it in that time. + */ +export function useThreadPullRequestLinkContextMenu(threadRef: ScopedThreadRef | null | undefined) { + const pullRequestLinking = usePullRequestLinking(threadRef?.environmentId); + return useCallback( + ( + event: ReactMouseEvent, + pullRequest: { + readonly url: string | undefined; + readonly providerKind: string | undefined; + }, + ) => { + const url = pullRequest.url; + if (url === undefined) return; + event.preventDefault(); + event.stopPropagation(); + const linked = + threadRef != null && pullRequestLinking.isLinked(readThreadShell(threadRef), url); + void showPullRequestLinkContextMenu({ + url, + openLabel: openOnHostLabel(pullRequest.providerKind ?? ""), + position: { x: event.clientX, y: event.clientY }, + // Offered only for a number the thread is actually linked to. The same badge also shows a + // pull request read off the thread's branch, and that one is a fact about git rather than + // a choice anyone made to undo. + ...(linked && threadRef != null + ? { + unlinkFromThread: async (target: string) => { + if (!pullRequestLinking.isLinked(readThreadShell(threadRef), target)) return; + await pullRequestLinking.changeLink(threadRef, target, false); + }, + } + : {}), + }); + }, + [pullRequestLinking, threadRef], + ); +} diff --git a/docs/user/thread-sidebar.md b/docs/user/thread-sidebar.md index 67574f0b0286..7f27e3360b5a 100644 --- a/docs/user/thread-sidebar.md +++ b/docs/user/thread-sidebar.md @@ -107,6 +107,12 @@ On web and desktop, right-click a pull request link in a thread and choose same link to return to the branch PR, if one exists. The linked pull request participates in automatic settlement. +A linked pull request also shows its number on the thread's sidebar row and in +the branch toolbar above the composer. Right-click either number for the same +**Unlink from thread**, alongside **Copy link** and opening the PR on its host. +The number a thread picked up from its branch can be copied and opened the same +way, but not unlinked, because nothing linked it. + ## Find and reference work On web and desktop, open the command palette with `Cmd/Ctrl+K` to search threads From d8ef6fa467b386d8e7efbfcc5d0869220553e0b0 Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Tue, 15 Sep 2026 10:31:35 -0400 Subject: [PATCH 2/5] feat(web): give the thread's pull request its own header button The git actions button answered two questions at once. It read "Commit & push" while work was in flight and "View PR" once it landed, so the control under the cursor changed meaning as the branch did, and the number of the pull request a thread carried was nowhere in the header. Split them. The git actions button now only moves the ref along: commit, push, pull, publish. Beside it sits a pull request button that shows the thread's number, opens the Linked pull requests panel, and right-clicks to unlink. With no pull request yet it reads "Create PR" and runs the same create flow, which stays in GitActionsControl with its progress stages, default-ref confirmation and result toast, reached through a window event so neither corner of the header has to own the other's state. Where the quick action has nothing left to do but a pull request could still be opened or read, its hint points at the new button rather than claiming no action is needed. Co-Authored-By: Claude Opus 5 --- .../GitActionsControl.logic.test.ts | 201 +++++------------- .../src/components/GitActionsControl.logic.ts | 108 ++++------ apps/web/src/components/GitActionsControl.tsx | 58 +---- apps/web/src/components/chat/ChatHeader.tsx | 9 +- .../ThreadPullRequestHeaderPill.tsx | 131 ++++++++++++ .../threadPullRequestHeaderPill.logic.test.ts | 157 ++++++++++++++ .../threadPullRequestHeaderPill.logic.ts | 41 ++++ apps/web/src/gitActionsBus.ts | 14 ++ docs/user/source-control.md | 8 +- 9 files changed, 457 insertions(+), 270 deletions(-) create mode 100644 apps/web/src/components/pullRequest/ThreadPullRequestHeaderPill.tsx create mode 100644 apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.test.ts create mode 100644 apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.ts create mode 100644 apps/web/src/gitActionsBus.ts diff --git a/apps/web/src/components/GitActionsControl.logic.test.ts b/apps/web/src/components/GitActionsControl.logic.test.ts index f302e976ca70..de3cf77f5b70 100644 --- a/apps/web/src/components/GitActionsControl.logic.test.ts +++ b/apps/web/src/components/GitActionsControl.logic.test.ts @@ -33,7 +33,7 @@ function status(overrides: Partial = {}): VcsStatusResult { } describe("when: ref is clean and has an open PR", () => { - it("resolveQuickAction opens the existing PR", () => { + it("has nothing left to do, and says where the pull request is", () => { const quick = resolveQuickAction( status({ pr: { @@ -47,7 +47,12 @@ describe("when: ref is clean and has an open PR", () => { }), false, ); - assert.deepInclude(quick, { kind: "open_pr", label: "View PR", disabled: false }); + assert.deepInclude(quick, { + kind: "show_hint", + label: "Push", + disabled: true, + hint: "Everything is pushed. The pull request is on the header's own button.", + }); }); it("buildMenuItems disables commit/push and enables open PR", () => { @@ -81,13 +86,6 @@ describe("when: ref is clean and has an open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "View PR", - disabled: false, - icon: "pr", - kind: "open_pr", - }, ]); }); }); @@ -122,14 +120,6 @@ describe("when: actions are busy", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -202,24 +192,17 @@ describe("when: ref is clean, ahead, and has an open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "View PR", - disabled: false, - icon: "pr", - kind: "open_pr", - }, ]); }); }); describe("when: ref is clean, ahead, and has no open PR", () => { - it("resolveQuickAction pushes and creates a PR", () => { + it("pushes, and leaves opening the pull request to the header's pill", () => { const quick = resolveQuickAction(status({ aheadCount: 2, pr: null }), false); assert.deepInclude(quick, { kind: "run_action", - action: "create_pr", - label: "Push & create PR", + action: "push", + label: "Push", }); }); @@ -242,46 +225,42 @@ describe("when: ref is clean, ahead, and has no open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: false, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); describe("when: source control provider uses merge requests", () => { - it("uses GitLab MR terminology in quick actions and menu items", () => { - const gitlabStatus = status({ - aheadCount: 2, - sourceControlProvider: { - kind: "gitlab", - name: "GitLab", - baseUrl: "https://gitlab.com", - }, - }); + const gitlab = { + kind: "gitlab", + name: "GitLab", + baseUrl: "https://gitlab.com", + } as const; - const quick = resolveQuickAction(gitlabStatus, false); - const items = buildMenuItems(gitlabStatus, false); + it("moves a ref without naming the change request at all", () => { + const quick = resolveQuickAction( + status({ aheadCount: 2, sourceControlProvider: gitlab }), + false, + ); + + assert.deepInclude(quick, { kind: "run_action", action: "push", label: "Push" }); + }); + + it("names the host's own word where it points at the header's pill", () => { + const quick = resolveQuickAction( + status({ aheadOfDefaultCount: 2, sourceControlProvider: gitlab }), + false, + ); assert.deepInclude(quick, { - kind: "run_action", - action: "create_pr", - label: "Push & create MR", - }); - assert.deepInclude(items[2], { - id: "pr", - label: "Create MR", + kind: "show_hint", + disabled: true, + hint: "Everything is pushed. Open a merge request from the header's own button.", }); }); }); describe("when: ref is clean, up to date, and has no open PR", () => { - it("enables create PR when synced with upstream but ahead of default", () => { + it("points at the header's pill when synced with upstream but ahead of default", () => { const syncedFeature = status({ aheadCount: 0, behindCount: 0, @@ -291,14 +270,17 @@ describe("when: ref is clean, up to date, and has no open PR", () => { const quick = resolveQuickAction(syncedFeature, false); assert.deepInclude(quick, { - label: "Create PR", - disabled: false, - kind: "run_action", - action: "create_pr", + label: "Push", + disabled: true, + kind: "show_hint", + hint: "Everything is pushed. Open a pull request from the header's own button.", }); const items = buildMenuItems(syncedFeature, false); - assert.equal(items.find((item) => item.id === "pr")?.disabled, false); + assert.deepEqual( + items.map((item) => item.id), + ["commit", "push"], + ); }); it("resolveQuickAction returns disabled no-action state", () => { @@ -328,14 +310,6 @@ describe("when: ref is clean, up to date, and has no open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -365,14 +339,6 @@ describe("when: ref is behind upstream", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -390,12 +356,12 @@ describe("when: ref has diverged from upstream", () => { }); describe("when: working tree has local changes", () => { - it("resolveQuickAction returns commit, push, and create PR", () => { + it("resolveQuickAction returns commit and push", () => { const quick = resolveQuickAction(status({ hasWorkingTreeChanges: true }), false); assert.deepInclude(quick, { kind: "run_action", - action: "commit_push_pr", - label: "Commit, push & PR", + action: "commit_push", + label: "Commit & push", }); }); @@ -455,14 +421,6 @@ describe("when: working tree has local changes", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); @@ -497,14 +455,6 @@ describe("when: working tree has local changes", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -540,15 +490,15 @@ describe("when: on default ref without open PR", () => { }); describe("when: working tree has local changes and ref is behind upstream", () => { - it("resolveQuickAction still prefers commit, push, and create PR", () => { + it("resolveQuickAction still prefers commit and push", () => { const quick = resolveQuickAction( status({ hasWorkingTreeChanges: true, behindCount: 1 }), false, ); assert.deepInclude(quick, { kind: "run_action", - action: "commit_push_pr", - label: "Commit, push & PR", + action: "commit_push", + label: "Commit & push", }); }); @@ -571,14 +521,6 @@ describe("when: working tree has local changes and ref is behind upstream", () = kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -611,14 +553,6 @@ describe("when: HEAD is detached and there are no local changes", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -637,7 +571,7 @@ describe("when: ref has no upstream configured", () => { }); }); - it("resolveQuickAction opens PR when clean, no upstream, no local commits are ahead, and PR exists", () => { + it("says where the pull request is when clean, unpublished, and one already exists", () => { const quick = resolveQuickAction( status({ hasUpstream: false, @@ -652,11 +586,14 @@ describe("when: ref has no upstream configured", () => { }, }), false, + false, + false, ); assert.deepInclude(quick, { - kind: "open_pr", - label: "View PR", - disabled: false, + kind: "show_hint", + label: "Push", + disabled: true, + hint: "Nothing to push. The pull request is on the header's own button.", }); }); @@ -703,18 +640,10 @@ describe("when: ref has no upstream configured", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); - it("resolveQuickAction runs push and create PR when no upstream and commits are ahead", () => { + it("resolveQuickAction runs push when no upstream and commits are ahead", () => { const quick = resolveQuickAction( status({ hasUpstream: false, @@ -725,8 +654,8 @@ describe("when: ref has no upstream configured", () => { ); assert.deepInclude(quick, { kind: "run_action", - action: "create_pr", - label: "Push & create PR", + action: "push", + label: "Push", disabled: false, }); }); @@ -768,14 +697,6 @@ describe("when: ref has no upstream configured", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: false, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); @@ -862,14 +783,6 @@ describe("when: ref has no upstream configured", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts index 96f7af794ace..326f24c01e3b 100644 --- a/apps/web/src/components/GitActionsControl.logic.ts +++ b/apps/web/src/components/GitActionsControl.logic.ts @@ -10,23 +10,30 @@ import { type ChangeRequestTerminology, } from "../sourceControlPresentation"; -export type GitActionIconName = "commit" | "push" | "pr"; +export type GitActionIconName = "commit" | "push"; -export type GitDialogAction = "commit" | "push" | "create_pr"; +export type GitDialogAction = "commit" | "push"; export interface GitActionMenuItem { - id: "commit" | "push" | "pr"; + id: "commit" | "push"; label: string; disabled: boolean; icon: GitActionIconName; - kind: "open_dialog" | "open_pr"; + kind: "open_dialog"; dialogAction?: GitDialogAction; } +/** + * Only ever moves the ref along: commit, push, pull, publish. + * + * Viewing and creating a pull request live on the header's own pill. Those two used to surface + * here, which meant one button read "Commit & push" while work was in flight and "View PR" once it + * landed, so the control under the cursor changed meaning as the branch did. + */ export interface GitQuickAction { label: string; disabled: boolean; - kind: "run_action" | "run_pull" | "open_pr" | "open_publish" | "show_hint"; + kind: "run_action" | "run_pull" | "open_publish" | "show_hint"; action?: GitStackedAction; hint?: string; } @@ -97,13 +104,10 @@ export function buildMenuItems( hasPrimaryRemote = true, ): GitActionMenuItem[] { if (!gitStatus) return []; - const terminology = resolveChangeRequestTerminology(gitStatus); const hasBranch = gitStatus.refName !== null; const hasChanges = gitStatus.hasWorkingTreeChanges; - const hasOpenPr = gitStatus.pr?.state === "open"; const isBehind = gitStatus.behindCount > 0; - const hasDefaultBranchDelta = (gitStatus.aheadOfDefaultCount ?? gitStatus.aheadCount) > 0; const canPushWithoutUpstream = hasPrimaryRemote && !gitStatus.hasUpstream; const canCommit = !isBusy && hasChanges; const canPush = @@ -112,15 +116,6 @@ export function buildMenuItems( !isBehind && gitStatus.aheadCount > 0 && (gitStatus.hasUpstream || canPushWithoutUpstream); - const canCreatePr = - !isBusy && - hasBranch && - !hasChanges && - !hasOpenPr && - hasDefaultBranchDelta && - !isBehind && - (gitStatus.hasUpstream || canPushWithoutUpstream); - const canOpenPr = !isBusy && hasOpenPr; const commitItem: GitActionMenuItem = { id: "commit", @@ -145,22 +140,6 @@ export function buildMenuItems( kind: "open_dialog", dialogAction: "push", }, - hasOpenPr - ? { - id: "pr", - label: `View ${terminology.shortLabel}`, - disabled: !canOpenPr, - icon: "pr", - kind: "open_pr", - } - : { - id: "pr", - label: `Create ${terminology.shortLabel}`, - disabled: !canCreatePr, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]; } @@ -205,21 +184,18 @@ export function resolveQuickAction( if (!gitStatus.hasUpstream && !hasPrimaryRemote) { return { label: "Commit", disabled: false, kind: "run_action", action: "commit" }; } - if (hasOpenPr || isDefaultRef) { - return { label: "Commit & push", disabled: false, kind: "run_action", action: "commit_push" }; - } - return { - label: `Commit, push & ${terminology.shortLabel}`, - disabled: false, - kind: "run_action", - action: "commit_push_pr", - }; + return { label: "Commit & push", disabled: false, kind: "run_action", action: "commit_push" }; } if (!gitStatus.hasUpstream) { if (!hasPrimaryRemote) { if (hasOpenPr && !isAhead) { - return { label: `View ${terminology.shortLabel}`, disabled: false, kind: "open_pr" }; + return { + label: "Push", + disabled: true, + kind: "show_hint", + hint: `Nothing to push. The ${terminology.singular} is on the header's own button.`, + }; } return { label: "Publish repository", @@ -228,9 +204,6 @@ export function resolveQuickAction( }; } if (!isAhead) { - if (hasOpenPr) { - return { label: `View ${terminology.shortLabel}`, disabled: false, kind: "open_pr" }; - } return { label: "Push", disabled: true, @@ -238,19 +211,11 @@ export function resolveQuickAction( hint: "No local commits to push.", }; } - if (hasOpenPr || isDefaultRef) { - return { - label: "Push", - disabled: false, - kind: "run_action", - action: isDefaultRef ? "commit_push" : "push", - }; - } return { - label: `Push & create ${terminology.shortLabel}`, + label: "Push", disabled: false, kind: "run_action", - action: "create_pr", + action: isDefaultRef ? "commit_push" : "push", }; } @@ -272,32 +237,31 @@ export function resolveQuickAction( } if (isAhead) { - if (hasOpenPr || isDefaultRef) { - return { - label: "Push", - disabled: false, - kind: "run_action", - action: isDefaultRef ? "commit_push" : "push", - }; - } return { - label: `Push & create ${terminology.shortLabel}`, + label: "Push", disabled: false, kind: "run_action", - action: "create_pr", + action: isDefaultRef ? "commit_push" : "push", }; } - if (hasOpenPr && gitStatus.hasUpstream) { - return { label: `View ${terminology.shortLabel}`, disabled: false, kind: "open_pr" }; + // Nothing left to move. Where a change request could still be opened or read, the hint points at + // the control that does it rather than leaving "no action needed" over work that is not finished. + if (hasOpenPr) { + return { + label: "Push", + disabled: true, + kind: "show_hint", + hint: `Everything is pushed. The ${terminology.singular} is on the header's own button.`, + }; } if (hasDefaultBranchDelta && !isDefaultRef) { return { - label: `Create ${terminology.shortLabel}`, - disabled: false, - kind: "run_action", - action: "create_pr", + label: "Push", + disabled: true, + kind: "show_hint", + hint: `Everything is pushed. Open a ${terminology.singular} from the header's own button.`, }; } diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index 99db055b667b..fa84228f7e1c 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -46,6 +46,7 @@ import { GitLabIcon, ForgejoIcon, } from "~/components/Icons"; +import { onRequestCreatePullRequest } from "~/gitActionsBus"; import { RadioGroup } from "~/components/ui/radio-group"; import { Spinner } from "~/components/ui/spinner"; import { toggleVariants } from "~/components/ui/toggle"; @@ -111,11 +112,6 @@ interface GitActionsControlProps { gitCwd: string | null; activeThreadRef: ScopedThreadRef | null; draftId?: DraftId; - /** - * Opens the thread's own change request beside it. Absent when the thread has no project to - * place it against, in which case it still opens in the browser. - */ - onOpenPullRequest?: ((number: number) => void) | undefined; } interface PendingDefaultBranchAction { @@ -379,7 +375,6 @@ function GitQuickActionIcon({ SourceControlIcon: ReturnType["Icon"]; }) { const iconClassName = "size-3.5"; - if (quickAction.kind === "open_pr") return ; if (quickAction.kind === "open_publish") return ; if (quickAction.kind === "run_pull") return ; if (quickAction.kind === "run_action") { @@ -947,7 +942,6 @@ export default function GitActionsControl({ gitCwd, activeThreadRef, draftId, - onOpenPullRequest, }: GitActionsControlProps) { const updateThreadMetadata = useAtomCommand( threadEnvironment.updateMetadata, @@ -964,7 +958,6 @@ export default function GitActionsControl({ [activeThreadRef], ); const openPrLink = useOpenPrLink(activeThreadRef ?? undefined); - const openLink = useOpenLink(activeThreadRef); const activeDraftThread = useComposerDraftStore((store) => draftId ? store.getDraftSession(draftId) @@ -1193,36 +1186,6 @@ export default function GitActionsControl({ }; }, [activeEnvironmentId, gitCwd, refreshVcsStatus]); - const openExistingPr = useCallback(async () => { - const openPr = gitStatusForActions?.pr?.state === "open" ? gitStatusForActions.pr : null; - // Beside the thread where it was made, the way the browser opens beside it. Checked before - // the shell, which opening in the app does not need. - if (openPr && onOpenPullRequest) { - onOpenPullRequest(openPr.number); - return; - } - const prUrl = openPr?.url ?? null; - if (!prUrl) { - toastManager.add({ - type: "error", - title: "No open pull request found.", - data: threadToastData, - }); - return; - } - void openLink(prUrl).catch((err: unknown) => { - console.error(err); - toastManager.add( - stackedThreadToast({ - type: "error", - title: "Unable to open pull request link", - description: err instanceof Error ? err.message : "An error occurred.", - ...(threadToastData !== undefined ? { data: threadToastData } : {}), - }), - ); - }); - }, [gitStatusForActions, onOpenPullRequest, openLink, threadToastData]); - runGitActionWithToast = useEffectEvent( async ({ action, @@ -1504,11 +1467,14 @@ export default function GitActionsControl({ }); }; + // Creating a change request is offered by the header's pull request pill, but the flow stays + // here with the progress stages, the default-ref confirmation and the toast that reports it. + const createPullRequestFromHeader = useEffectEvent(() => { + void runGitActionWithToast({ action: "create_pr" }); + }); + useEffect(() => onRequestCreatePullRequest(createPullRequestFromHeader), []); + const runQuickAction = () => { - if (quickAction.kind === "open_pr") { - void openExistingPr(); - return; - } if (quickAction.kind === "open_publish") { setIsPublishDialogOpen(true); return; @@ -1569,18 +1535,10 @@ export default function GitActionsControl({ const openDialogForMenuItem = (item: GitActionMenuItem) => { if (item.disabled) return; - if (item.kind === "open_pr") { - void openExistingPr(); - return; - } if (item.dialogAction === "push") { void runGitActionWithToast({ action: "push" }); return; } - if (item.dialogAction === "create_pr") { - void runGitActionWithToast({ action: "create_pr" }); - return; - } setExcludedFiles(new Set()); setIsEditingFiles(false); setIsCommitDialogOpen(true); diff --git a/apps/web/src/components/chat/ChatHeader.tsx b/apps/web/src/components/chat/ChatHeader.tsx index fbebc323a950..24aa2ea725af 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 { ThreadPullRequestHeaderPill } from "../pullRequest/ThreadPullRequestHeaderPill"; import { isTrailingDoubleClick } from "../Sidebar.logic"; import { type DraftId } from "~/composerDraftStore"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; @@ -409,6 +410,13 @@ export const ChatHeader = memo(function ChatHeader({ "[[data-panel-animations=true]_&]:motion-safe:transition-[padding-right] [[data-panel-animations=true]_&]:motion-safe:[transition-duration:var(--panel-animation-duration)] [[data-panel-animations=true]_&]:motion-safe:ease-out", )} > + {activeProjectName && ( + + )} {activeProjectScripts && ( )} diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestHeaderPill.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestHeaderPill.tsx new file mode 100644 index 000000000000..d740ab40d893 --- /dev/null +++ b/apps/web/src/components/pullRequest/ThreadPullRequestHeaderPill.tsx @@ -0,0 +1,131 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; +import { resolveThreadCurrentPullRequestLink } from "@t3tools/shared/threadPullRequests"; +import { GitPullRequestArrowIcon } from "lucide-react"; +import { useCallback, useMemo, type MouseEvent as ReactMouseEvent } from "react"; + +import { useSupportsMultiplePullRequests } from "~/hooks/useSupportsMultiplePullRequests"; +import { cn } from "~/lib/utils"; +import { requestCreatePullRequest } from "~/gitActionsBus"; +import { useRightPanelStore } from "~/rightPanelStore"; +import { useThreadShell } from "~/state/entities"; +import { useEnvironmentQuery } from "~/state/query"; +import { vcsEnvironment } from "~/state/vcs"; + +import { + PrStatusTooltipContent, + prStatusIndicator, + useLinkedThreadPullRequest, +} from "../ThreadStatusIndicators"; +import { Button } from "../ui/button"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; +import { resolveThreadPullRequestHeaderPill } from "./threadPullRequestHeaderPill.logic"; +import { useThreadPullRequestLinkContextMenu } from "./useThreadPullRequestLinkContextMenu"; + +/** + * The thread's pull request, beside the git actions in the header. + * + * It opens the linked pull requests panel rather than the pull request itself, because the number + * shown here is also the handle for changing which pull requests the thread carries. The host is + * still one right-click away, along with unlinking. + */ +export function ThreadPullRequestHeaderPill({ + threadRef, + gitCwd, + onOpenPullRequest, +}: { + readonly threadRef: ScopedThreadRef; + readonly gitCwd: string | null; + /** + * Opens one change request beside the thread. Used where the environment has no linked pull + * requests panel to manage, so the click still lands on something rather than an empty surface. + */ + readonly onOpenPullRequest?: ((number: number) => void) | undefined; +}) { + const thread = useThreadShell(threadRef); + const linkedStatus = useLinkedThreadPullRequest( + threadRef.environmentId, + thread?.linkedPullRequest, + true, + thread?.pullRequests, + thread?.branchPullRequest, + ); + const supportsMultiplePullRequests = useSupportsMultiplePullRequests(threadRef.environmentId); + const currentLinkedPr = supportsMultiplePullRequests + ? resolveThreadCurrentPullRequestLink(thread?.pullRequests ?? []) + : null; + // Shares the atom the git actions control already reads, so the header asks for one status. + const gitStatus = useEnvironmentQuery( + gitCwd === null + ? null + : vcsEnvironment.status({ environmentId: threadRef.environmentId, input: { cwd: gitCwd } }), + ); + const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); + + const pr = linkedStatus?.pr ?? null; + const pullRequest = useMemo(() => { + const number = pr?.number ?? currentLinkedPr?.number; + const url = pr?.url ?? currentLinkedPr?.url; + return number === undefined || url === undefined ? null : { number, url }; + }, [currentLinkedPr, pr]); + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest, + gitStatus: gitStatus.data ?? null, + }); + + const pillNumber = pill.kind === "linked" ? pill.number : null; + const handleClick = useCallback(() => { + if (pillNumber === null) { + requestCreatePullRequest(); + return; + } + if (!supportsMultiplePullRequests && onOpenPullRequest) { + onOpenPullRequest(pillNumber); + return; + } + useRightPanelStore.getState().open(threadRef, "pull-requests"); + }, [onOpenPullRequest, pillNumber, supportsMultiplePullRequests, threadRef]); + const handleContextMenu = useCallback( + (event: ReactMouseEvent) => { + openPrContextMenu(event, { + url: pullRequest?.url, + providerKind: linkedStatus?.sourceControlProvider.kind, + }); + }, + [linkedStatus, openPrContextMenu, pullRequest], + ); + + if (pill.kind === "hidden") return null; + + const status = + pill.kind === "linked" ? prStatusIndicator(pr, linkedStatus?.sourceControlProvider) : null; + const label = pill.kind === "create" ? "Create PR" : `#${pill.number}`; + const tooltip = + pill.kind === "create" + ? "Create a pull request for this ref" + : (status?.tooltip ?? `Pull request ${label}`); + + return ( + + + } + > + + + {label} + + + + {status ? : tooltip} + + + ); +} diff --git a/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.test.ts b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.test.ts new file mode 100644 index 000000000000..408c42fb3021 --- /dev/null +++ b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.test.ts @@ -0,0 +1,157 @@ +import type { VcsStatusResult } from "@t3tools/contracts"; +import { assert, describe, it } from "vite-plus/test"; + +import { resolveThreadPullRequestHeaderPill } from "./threadPullRequestHeaderPill.logic"; + +function status(overrides: Partial = {}): VcsStatusResult { + return { + isRepo: true, + hasPrimaryRemote: true, + isDefaultRef: false, + refName: "feature/test", + hasWorkingTreeChanges: false, + workingTree: { files: [], insertions: 0, deletions: 0 }, + hasUpstream: true, + aheadCount: 0, + behindCount: 0, + aheadOfDefaultCount: 1, + pr: null, + ...overrides, + }; +} + +describe("when: the thread carries a pull request", () => { + it("wears its number, whatever the ref is doing", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: { number: 42, url: "https://example.com/pr/42" }, + gitStatus: status({ hasWorkingTreeChanges: true, behindCount: 3 }), + }); + + assert.deepEqual(pill, { kind: "linked", number: 42, url: "https://example.com/pr/42" }); + }); + + it("wears its number even before git status arrives", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: { number: 7, url: "https://example.com/pr/7" }, + gitStatus: null, + }); + + assert.deepEqual(pill, { kind: "linked", number: 7, url: "https://example.com/pr/7" }); + }); +}); + +describe("when: the thread has no pull request", () => { + it("offers to create one from a clean ref that is ahead of default", () => { + assert.deepEqual( + resolveThreadPullRequestHeaderPill({ pullRequest: null, gitStatus: status() }), + { + kind: "create", + }, + ); + }); + + it("offers to create one from an unpublished ref that has a remote to publish to", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ hasUpstream: false, aheadCount: 2, aheadOfDefaultCount: undefined }), + }); + + assert.deepEqual(pill, { kind: "create" }); + }); + + it("falls back to the ahead count when the default branch delta is unknown", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ aheadCount: 0, aheadOfDefaultCount: undefined }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides while git status is still loading", () => { + assert.deepEqual(resolveThreadPullRequestHeaderPill({ pullRequest: null, gitStatus: null }), { + kind: "hidden", + }); + }); + + it("hides on a detached head", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ refName: null }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides with uncommitted work, because creating one would leave it behind", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ hasWorkingTreeChanges: true }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides on a ref with nothing on top of the default branch", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ aheadOfDefaultCount: 0 }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides while the ref is behind its upstream", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ behindCount: 1 }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides when there is nowhere to push the ref", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ hasUpstream: false, hasPrimaryRemote: false }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides when the ref already has an open pull request the thread has not linked", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ + pr: { + number: 21, + title: "Open PR", + url: "https://example.com/pr/21", + baseRef: "main", + headRef: "feature/test", + state: "open", + }, + }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("offers to create one when the ref's last pull request is closed", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ + pr: { + number: 20, + title: "Closed PR", + url: "https://example.com/pr/20", + baseRef: "main", + headRef: "feature/test", + state: "closed", + }, + }), + }); + + assert.deepEqual(pill, { kind: "create" }); + }); +}); diff --git a/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.ts b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.ts new file mode 100644 index 000000000000..fcab0d394e9f --- /dev/null +++ b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.ts @@ -0,0 +1,41 @@ +import type { VcsStatusResult } from "@t3tools/contracts"; + +export type ThreadPullRequestHeaderPill = + | { readonly kind: "linked"; readonly number: number; readonly url: string } + | { readonly kind: "create" } + | { readonly kind: "hidden" }; + +/** + * What the header's pull request control should be right now. + * + * The git actions button beside it answers "what moves my work forward", which is why it changes + * label with every commit and push. This answers the question that outlives those: where the pull + * request is, or that there is none yet. Splitting them is what stops one button meaning + * "Commit & push" in the morning and "View PR" in the afternoon. + * + * Hidden is the answer when creating one would fail anyway. Offering it on a thread with nothing + * committed, or behind its upstream, would put a button in the header whose only outcome is an + * error toast. + */ +export function resolveThreadPullRequestHeaderPill(input: { + /** The number the thread wears: an explicit link first, otherwise the one read off its branch. */ + readonly pullRequest: { readonly number: number; readonly url: string } | null; + readonly gitStatus: VcsStatusResult | null; +}): ThreadPullRequestHeaderPill { + if (input.pullRequest !== null) { + return { kind: "linked", number: input.pullRequest.number, url: input.pullRequest.url }; + } + const gitStatus = input.gitStatus; + if (gitStatus === null || gitStatus.refName === null) return { kind: "hidden" }; + // Mirrors the old menu's own create gate, so the pill appears exactly where "Create PR" used to + // be offered and nowhere it was not. + const canPushWithoutUpstream = gitStatus.hasPrimaryRemote && !gitStatus.hasUpstream; + const hasDefaultBranchDelta = (gitStatus.aheadOfDefaultCount ?? gitStatus.aheadCount) > 0; + const canCreate = + !gitStatus.hasWorkingTreeChanges && + gitStatus.pr?.state !== "open" && + hasDefaultBranchDelta && + gitStatus.behindCount === 0 && + (gitStatus.hasUpstream || canPushWithoutUpstream); + return canCreate ? { kind: "create" } : { kind: "hidden" }; +} diff --git a/apps/web/src/gitActionsBus.ts b/apps/web/src/gitActionsBus.ts new file mode 100644 index 000000000000..d99fad91b135 --- /dev/null +++ b/apps/web/src/gitActionsBus.ts @@ -0,0 +1,14 @@ +// Lets the header's pull request pill reach the git actions control, which owns the create flow: +// its progress stages, its default-ref confirmation, and the toast that reports the result. The +// two sit in different corners of the header, so a shared parent would have to thread that state +// through everything between them. +const CREATE_PULL_REQUEST_EVENT = "t3code:create-pull-request"; + +export function requestCreatePullRequest(): void { + window.dispatchEvent(new CustomEvent(CREATE_PULL_REQUEST_EVENT)); +} + +export function onRequestCreatePullRequest(listener: () => void): () => void { + window.addEventListener(CREATE_PULL_REQUEST_EVENT, listener); + return () => window.removeEventListener(CREATE_PULL_REQUEST_EVENT, listener); +} diff --git a/docs/user/source-control.md b/docs/user/source-control.md index d8a06fc34b64..db18dd06b1ea 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -87,8 +87,9 @@ make your first commit before pushing. ## Create a pull request -Use a thread's Git actions to commit, push, and create a pull request. T3 Code can generate commit -messages, review titles, and descriptions from your changes. +Use a thread's Git actions to commit and push, then the pull request button beside them to open one. +Once the thread has a pull request, that button shows its number and opens **Linked pull requests**. +T3 Code can generate commit messages, review titles, and descriptions from your changes. Choose the writing style and model in **Settings → Source Control**. **Repository conventions** uses the project's instructions and recent commit subjects. @@ -147,7 +148,8 @@ links. On mobile, the Git overview lists linked reviews and their stacks; tap a Linking and unlinking are available in the web and desktop clients. The **Linked pull requests** panel lists every review and groups stacks. Unlink a review from its -row menu. An unlinked stack layer stays out of later syncs. Open linked reviews refresh on the server; +row menu, or right-click its number anywhere it appears: the thread list, the thread header, or a +link in the conversation. An unlinked stack layer stays out of later syncs. Open linked reviews refresh on the server; closed reviews refresh periodically so reopening one on the host is detected. Merged reviews refresh when requested. With **Auto-settle merged threads** enabled, a thread can settle after every linked review is terminal. An open or unsynced link keeps it active. From 3b5728e0eb6f0693d4c8a536541dd82ad4fc505d Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Tue, 15 Sep 2026 10:41:20 -0400 Subject: [PATCH 3/5] refactor(web): drop the composer's pull request badge The branch toolbar carried a ghost-styled copy of the sidebar's linked-PR badge. It showed a number and nothing else, and the header now shows the same number with somewhere to go: the Linked pull requests panel, where the thread's pull requests are actually managed, plus Create PR when there is none. Two reads of the same fact, a few hundred pixels apart, and the composer one was the thinner of them. Removing it leaves resolveBranchToolbarPrBranch with no caller and the badge control with one variant, so both collapse. The sidebar badge and the stack doorway it opens are unchanged. Co-Authored-By: Claude Opus 5 --- .../components/BranchToolbar.logic.test.ts | 30 --------- .../web/src/components/BranchToolbar.logic.ts | 7 -- .../BranchToolbarBranchSelector.tsx | 64 ------------------- apps/web/src/components/Sidebar.tsx | 1 - .../src/components/ThreadStatusIndicators.tsx | 12 +--- docs/user/thread-sidebar.md | 4 +- 6 files changed, 5 insertions(+), 113 deletions(-) diff --git a/apps/web/src/components/BranchToolbar.logic.test.ts b/apps/web/src/components/BranchToolbar.logic.test.ts index ff4bb76bf12a..101c11531c37 100644 --- a/apps/web/src/components/BranchToolbar.logic.test.ts +++ b/apps/web/src/components/BranchToolbar.logic.test.ts @@ -10,7 +10,6 @@ import { resolveEffectiveEnvMode, resolveEnvModeLabel, resolveBranchTriggerLabel, - resolveBranchToolbarPrBranch, resolveBranchToolbarValue, resolveLockedWorkspaceLabel, resolveLocalCheckoutBranchMismatch, @@ -272,35 +271,6 @@ describe("resolveBranchTriggerLabel", () => { }); }); -describe("resolveBranchToolbarPrBranch", () => { - it("uses the explicit thread branch when it matches the displayed branch", () => { - expect( - resolveBranchToolbarPrBranch({ - activeThreadBranch: "feature/current", - resolvedActiveBranch: "feature/current", - }), - ).toBe("feature/current"); - }); - - it("hides PR state while an optimistic branch switch is in flight", () => { - expect( - resolveBranchToolbarPrBranch({ - activeThreadBranch: "feature/current", - resolvedActiveBranch: "feature/next", - }), - ).toBeNull(); - }); - - it("does not infer PR state without an explicit thread branch", () => { - expect( - resolveBranchToolbarPrBranch({ - activeThreadBranch: null, - resolvedActiveBranch: "feature/current", - }), - ).toBeNull(); - }); -}); - describe("resolveLocalCheckoutBranchMismatch", () => { it("detects when a local thread is associated with a different branch than the checkout", () => { expect( diff --git a/apps/web/src/components/BranchToolbar.logic.ts b/apps/web/src/components/BranchToolbar.logic.ts index 0577f5e8dd1f..f73123325fba 100644 --- a/apps/web/src/components/BranchToolbar.logic.ts +++ b/apps/web/src/components/BranchToolbar.logic.ts @@ -211,13 +211,6 @@ export function resolveBranchTriggerLabel(input: { return resolvedActiveBranch; } -export function resolveBranchToolbarPrBranch(input: { - activeThreadBranch: string | null; - resolvedActiveBranch: string | null; -}): string | null { - return input.activeThreadBranch === input.resolvedActiveBranch ? input.activeThreadBranch : null; -} - export function resolveLocalCheckoutBranchMismatch(input: { effectiveEnvMode: EnvMode; activeWorktreePath: string | null; diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 0692c869d954..b699c8857190 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -1,6 +1,3 @@ -import { useSupportsMultiplePullRequests } from "~/hooks/useSupportsMultiplePullRequests"; -import { resolveThreadCurrentPullRequestLink } from "@t3tools/shared/threadPullRequests"; -import { useRightPanelStore } from "../rightPanelStore"; import { RefreshIcon } from "~/components/ui/refresh-icon"; import { scopeProjectRef, scopeThreadRef } from "@t3tools/client-runtime/environment"; import { @@ -29,7 +26,6 @@ import { import { useComposerDraftStore, type DraftId } from "../composerDraftStore"; import { writeTextToClipboard } from "../hooks/useCopyToClipboard"; import { readLocalApi } from "../localApi"; -import { useOpenPrLink } from "../lib/openPullRequestLink"; import { shouldLoadNextBranchPageAfterScroll } from "../state/paginatedBranches"; import { usePaginatedBranches } from "../state/queries"; import { useProject, useThreadShell } from "../state/entities"; @@ -44,7 +40,6 @@ import { useComposerMenuProps } from "./chat/composerEventScope"; import { deriveLocalBranchNameFromRemoteRef, resolveBranchTriggerLabel, - resolveBranchToolbarPrBranch, resolveBranchSelectionTarget, resolveBranchToolbarValue, resolveDraftEnvModeAfterBranchChange, @@ -52,13 +47,6 @@ import { sanitizeNewRefName, shouldIncludeBranchPickerItem, } from "./BranchToolbar.logic"; -import { - ThreadPullRequestBadgeControl, - prStatusIndicator, - resolveThreadPullRequestBadge, - useLinkedThreadPullRequest, -} from "./ThreadStatusIndicators"; -import { useThreadPullRequestLinkContextMenu } from "./pullRequest/useThreadPullRequestLinkContextMenu"; import { Button } from "./ui/button"; import { Switch } from "./ui/switch"; import { getVirtualizedScrollFadeClassName } from "./ui/scroll-area"; @@ -641,39 +629,6 @@ export function BranchToolbarBranchSelector({ startFromOrigin, }); - // Branch status is the fallback when this thread has no linked pull requests. - const branchPrBranch = resolveBranchToolbarPrBranch({ - activeThreadBranch, - resolvedActiveBranch, - }); - const branchPr = - branchPrBranch !== null && branchStatusQuery.data?.refName === branchPrBranch - ? (branchStatusQuery.data.pr ?? null) - : null; - const supportsMultiplePullRequests = useSupportsMultiplePullRequests(environmentId); - const linkedStatus = useLinkedThreadPullRequest( - environmentId, - serverThread?.linkedPullRequest, - true, - serverThread?.pullRequests, - serverThread?.branchPullRequest, - ); - const currentLinkedPr = supportsMultiplePullRequests - ? resolveThreadCurrentPullRequestLink(serverThread?.pullRequests ?? []) - : null; - const prBadge = supportsMultiplePullRequests - ? resolveThreadPullRequestBadge(serverThread?.pullRequests) - : null; - const displayedPr = linkedStatus?.pr ?? (currentLinkedPr === null ? branchPr : null); - const displayedPrStatus = prStatusIndicator( - displayedPr, - linkedStatus?.sourceControlProvider ?? branchStatusQuery.data?.sourceControlProvider, - ); - const prNumber = currentLinkedPr?.number ?? displayedPr?.number; - const prUrl = currentLinkedPr?.url ?? displayedPr?.url; - const openPrLink = useOpenPrLink(threadRef); - const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); - function renderPickerItem(itemValue: string, index: number) { if (checkoutPullRequestItemValue && itemValue === checkoutPullRequestItemValue) { return ( @@ -775,25 +730,6 @@ export function BranchToolbarBranchSelector({ className={cn("flex min-w-0 items-center gap-1", className)} data-composer-context-control > - useRightPanelStore.getState().open(threadRef, "pull-requests")} - onOpenPullRequest={(event) => { - if (prUrl) openPrLink(event, prUrl); - }} - onContextMenuPullRequest={(event) => - openPrContextMenu(event, { - url: prUrl, - providerKind: - linkedStatus?.sourceControlProvider.kind ?? - branchStatusQuery.data?.sourceControlProvider?.kind, - }) - } - /> {/* Context menu lives on the wrapper: the disabled Button has pointer-events-none, so the trigger itself never sees right-clicks while refs are loading or a branch action is pending. */} diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index f1703a6cd130..262c6bda22bd 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -1518,7 +1518,6 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { const prBadge = prBadgeShape?.kind === "stack" || pr || currentLinkedPr ? ( ; } -/** The complete linked-PR control shared by the sidebar and composer footer. */ +/** The complete linked-PR control worn by a thread row. */ export function ThreadPullRequestBadgeControl({ - variant, badge, number, url, @@ -148,7 +147,6 @@ export function ThreadPullRequestBadgeControl({ onOpenPullRequest, onContextMenuPullRequest, }: { - variant: "underline" | "ghost"; badge: ThreadPullRequestBadge | null; number?: number | undefined; url?: string | undefined; @@ -169,12 +167,8 @@ export function ThreadPullRequestBadgeControl({ : "" }`; const className = cn( - variant === "ghost" - ? buttonVariants({ variant: "ghost", size: "xs" }) - : "inline-flex shrink-0 cursor-pointer items-center gap-0.5 whitespace-nowrap border-b border-transparent hover:border-current focus-visible:outline-2 focus-visible:outline-ring", + "inline-flex shrink-0 cursor-pointer items-center gap-0.5 whitespace-nowrap border-b border-transparent hover:border-current focus-visible:outline-2 focus-visible:outline-ring", "text-xs tabular-nums", - variant === "ghost" && - "font-normal text-xs! active:scale-100 [--control-icon-color:currentColor]", badge !== null && (isStack || linkedCount !== null) ? PR_STATE_COLOR_CLASS[badge.state] : (status?.colorClass ?? "text-muted-foreground"), diff --git a/docs/user/thread-sidebar.md b/docs/user/thread-sidebar.md index 7f27e3360b5a..cbc246e16c92 100644 --- a/docs/user/thread-sidebar.md +++ b/docs/user/thread-sidebar.md @@ -107,8 +107,8 @@ On web and desktop, right-click a pull request link in a thread and choose same link to return to the branch PR, if one exists. The linked pull request participates in automatic settlement. -A linked pull request also shows its number on the thread's sidebar row and in -the branch toolbar above the composer. Right-click either number for the same +A linked pull request also shows its number on the thread's sidebar row and on +the thread header's own button. Right-click either number for the same **Unlink from thread**, alongside **Copy link** and opening the PR on its host. The number a thread picked up from its branch can be copied and opened the same way, but not unlinked, because nothing linked it. From b88406d7490d64a994783ebfd44ab76dbb13dceb Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Tue, 15 Sep 2026 10:47:20 -0400 Subject: [PATCH 4/5] fix(web): stop the git hint naming a button that may not be there The disabled quick action told the reader the pull request was on the header's own button whenever the ref had an open one. The header's pill shows the thread's pull request, and a ref can carry one the thread never linked, so the hint sometimes pointed at nothing. Report it as a fact about the ref instead: "This ref already has an open pull request." The create hint still names the button, because the pill's create gate and that branch of resolveQuickAction agree, and both controls render under the same activeProjectName and gitCwd. Found by CodeRabbit on #11900. Co-Authored-By: Claude Opus 5 --- .../GitActionsControl.logic.test.ts | 29 +++++++++++++++++-- .../src/components/GitActionsControl.logic.ts | 12 +++++--- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/apps/web/src/components/GitActionsControl.logic.test.ts b/apps/web/src/components/GitActionsControl.logic.test.ts index de3cf77f5b70..53718975e8f7 100644 --- a/apps/web/src/components/GitActionsControl.logic.test.ts +++ b/apps/web/src/components/GitActionsControl.logic.test.ts @@ -51,7 +51,7 @@ describe("when: ref is clean and has an open PR", () => { kind: "show_hint", label: "Push", disabled: true, - hint: "Everything is pushed. The pull request is on the header's own button.", + hint: "Everything is pushed. This ref already has an open pull request.", }); }); @@ -257,6 +257,29 @@ describe("when: source control provider uses merge requests", () => { hint: "Everything is pushed. Open a merge request from the header's own button.", }); }); + + it("names the host's own word when reporting one the ref already has", () => { + const quick = resolveQuickAction( + status({ + sourceControlProvider: gitlab, + pr: { + number: 30, + title: "Open MR", + url: "https://gitlab.com/g/p/-/merge_requests/30", + baseRef: "main", + headRef: "feature/test", + state: "open", + }, + }), + false, + ); + + assert.deepInclude(quick, { + kind: "show_hint", + disabled: true, + hint: "Everything is pushed. This ref already has an open merge request.", + }); + }); }); describe("when: ref is clean, up to date, and has no open PR", () => { @@ -571,7 +594,7 @@ describe("when: ref has no upstream configured", () => { }); }); - it("says where the pull request is when clean, unpublished, and one already exists", () => { + it("reports the existing pull request when clean, unpublished, and one already exists", () => { const quick = resolveQuickAction( status({ hasUpstream: false, @@ -593,7 +616,7 @@ describe("when: ref has no upstream configured", () => { kind: "show_hint", label: "Push", disabled: true, - hint: "Nothing to push. The pull request is on the header's own button.", + hint: "Nothing to push. This ref already has an open pull request.", }); }); diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts index 326f24c01e3b..d4092d910fda 100644 --- a/apps/web/src/components/GitActionsControl.logic.ts +++ b/apps/web/src/components/GitActionsControl.logic.ts @@ -194,7 +194,7 @@ export function resolveQuickAction( label: "Push", disabled: true, kind: "show_hint", - hint: `Nothing to push. The ${terminology.singular} is on the header's own button.`, + hint: `Nothing to push. This ref already has an open ${terminology.singular}.`, }; } return { @@ -245,14 +245,18 @@ export function resolveQuickAction( }; } - // Nothing left to move. Where a change request could still be opened or read, the hint points at - // the control that does it rather than leaving "no action needed" over work that is not finished. + // Nothing left to move. Where a change request could still be opened or read, the hint says so + // rather than leaving "no action needed" over work that is not finished. + // + // An existing one is reported as a fact about the ref, not as a place to click. The header's pill + // shows the thread's own pull request, and a ref can carry one the thread never linked, so + // pointing at the pill here would sometimes name a button that is not on screen. if (hasOpenPr) { return { label: "Push", disabled: true, kind: "show_hint", - hint: `Everything is pushed. The ${terminology.singular} is on the header's own button.`, + hint: `Everything is pushed. This ref already has an open ${terminology.singular}.`, }; } From c218e5afac688c4be0d40b54bd8fca183558398a Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Tue, 15 Sep 2026 11:22:07 -0400 Subject: [PATCH 5/5] fix(web): name the pull request the unlink menu will drop A thread that holds several links wears one aggregate badge that reads "+3" rather than a number, and right-clicking it offered a bare "Unlink from thread" over whichever link is current. The menu now reads "Unlink #123 from thread", derived from the url it was opened on, so the destructive item names its target everywhere the number is written. The same hook also stopped subscribing to the projects and server-config atoms. It is mounted once per row of the thread list, and nothing it reads there is needed until a right-click lands, so the linking state is now resolved from the registry inside the handler. A project or config change no longer wakes every row, and the projects array is no longer filtered once per row on mount. Co-Authored-By: Claude Opus 5 --- .../pullRequestLinkContextMenu.test.ts | 17 +- .../pullRequest/pullRequestLinkContextMenu.ts | 20 +- .../useThreadPullRequestLinkContextMenu.ts | 18 +- apps/web/src/hooks/usePullRequestLinking.ts | 183 +++++++++++------- apps/web/src/state/entities.ts | 4 + 5 files changed, 159 insertions(+), 83 deletions(-) diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts index a580f6eafef5..0384e84fd370 100644 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts @@ -21,6 +21,7 @@ const URL = "https://github.com/pingdotgg/t3code/pull/23"; async function openMenu( options: { readonly unlinkFromThread?: ((url: string) => Promise) | undefined; + readonly url?: string; }, choose: PullRequestLinkContextMenuAction | null = null, ): Promise { @@ -37,7 +38,7 @@ async function openMenu( }; try { await showPullRequestLinkContextMenu({ - url: URL, + url: options.url ?? URL, openLabel: "Open on GitHub", position: { x: 0, y: 0 }, ...(options.unlinkFromThread ? { unlinkFromThread: options.unlinkFromThread } : {}), @@ -68,10 +69,22 @@ describe("pull request link context menu", () => { expect(await openMenu({ unlinkFromThread: async () => {} })).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 }, + { id: "unlink-from-thread", label: "Unlink #23 from thread", separatorBefore: true }, ]); }); + it("falls back to the bare label for a url it cannot read a number out of", async () => { + const items = await openMenu({ + url: "https://example.com/some/page", + unlinkFromThread: async () => {}, + }); + expect(items.at(-1)).toEqual({ + 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[] = []; await openMenu( diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts index d123116328fe..0b2d1e9d3ac8 100644 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts +++ b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts @@ -1,4 +1,5 @@ import type { ContextMenuItem } from "@t3tools/contracts"; +import { parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl"; import { writeTextToClipboard } from "~/hooks/useCopyToClipboard"; import { readLocalApi } from "~/localApi"; @@ -25,17 +26,21 @@ export const openOnHostLabel = (provider: string): string => * 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 a misclick on it is the only one that costs * anything. + * + * It also names its number, which the other two do not need to: the badge a reader right-clicks is + * sometimes an aggregate that reads `+3` instead of a number, and the link on that badge is only + * one of the three. The menu says which one is about to go. */ function pullRequestLinkContextMenuItems( openLabel: string, - canUnlinkFromThread: boolean, + unlinkLabel: string | null, ): readonly ContextMenuItem[] { 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 }); + if (unlinkLabel !== null) { + items.push({ id: "unlink-from-thread", label: unlinkLabel, separatorBefore: true }); } return items; } @@ -70,10 +75,17 @@ export async function showPullRequestLinkContextMenu({ }): Promise { const api = readLocalApi(); if (!api) return; + const number = parseChangeRequestUrl(url)?.number; + const unlinkLabel = + unlinkFromThread === undefined + ? null + : number === undefined + ? "Unlink from thread" + : `Unlink #${number} from thread`; let action: PullRequestLinkContextMenuAction | null = null; try { action = await api.contextMenu.show( - pullRequestLinkContextMenuItems(openLabel, unlinkFromThread !== undefined), + pullRequestLinkContextMenuItems(openLabel, unlinkLabel), position, ); } catch { diff --git a/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts b/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts index 6812c3f74316..eea6a62350ae 100644 --- a/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts +++ b/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts @@ -1,7 +1,7 @@ import type { ScopedThreadRef } from "@t3tools/contracts"; import { useCallback, type MouseEvent as ReactMouseEvent } from "react"; -import { usePullRequestLinking } from "~/hooks/usePullRequestLinking"; +import { useLazyPullRequestLinking } from "~/hooks/usePullRequestLinking"; import { readThreadShell } from "~/state/entities"; import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; @@ -17,10 +17,11 @@ import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLi * * Whether the thread is linked is read when the menu opens, and again when the item is chosen: a * menu sits open for as long as it takes to read, and the agent can link or unlink from underneath - * it in that time. + * it in that time. That same lateness is why the linking state is resolved per click here rather + * than subscribed to: this hook is mounted once per row of the thread list. */ export function useThreadPullRequestLinkContextMenu(threadRef: ScopedThreadRef | null | undefined) { - const pullRequestLinking = usePullRequestLinking(threadRef?.environmentId); + const resolvePullRequestLinking = useLazyPullRequestLinking(threadRef?.environmentId); return useCallback( ( event: ReactMouseEvent, @@ -33,8 +34,8 @@ export function useThreadPullRequestLinkContextMenu(threadRef: ScopedThreadRef | if (url === undefined) return; event.preventDefault(); event.stopPropagation(); - const linked = - threadRef != null && pullRequestLinking.isLinked(readThreadShell(threadRef), url); + const linking = resolvePullRequestLinking(); + const linked = threadRef != null && linking.isLinked(readThreadShell(threadRef), url); void showPullRequestLinkContextMenu({ url, openLabel: openOnHostLabel(pullRequest.providerKind ?? ""), @@ -45,13 +46,14 @@ export function useThreadPullRequestLinkContextMenu(threadRef: ScopedThreadRef | ...(linked && threadRef != null ? { unlinkFromThread: async (target: string) => { - if (!pullRequestLinking.isLinked(readThreadShell(threadRef), target)) return; - await pullRequestLinking.changeLink(threadRef, target, false); + const current = resolvePullRequestLinking(); + if (!current.isLinked(readThreadShell(threadRef), target)) return; + await current.changeLink(threadRef, target, false); }, } : {}), }); }, - [pullRequestLinking, threadRef], + [resolvePullRequestLinking, threadRef], ); } diff --git a/apps/web/src/hooks/usePullRequestLinking.ts b/apps/web/src/hooks/usePullRequestLinking.ts index f6c076f3009d..c6c3394cdfce 100644 --- a/apps/web/src/hooks/usePullRequestLinking.ts +++ b/apps/web/src/hooks/usePullRequestLinking.ts @@ -1,4 +1,4 @@ -import { useMemo } from "react"; +import { useCallback, useMemo } from "react"; import { sourceControlRepositorySelector } from "@t3tools/shared/sourceControl"; import type { EnvironmentId, @@ -24,81 +24,126 @@ import { matchesLinkedPullRequestUrl, parseChangeRequestUrl, } from "~/lib/openPullRequestLink"; -import { useProjects, useServerConfigs } from "~/state/entities"; +import type { EnvironmentProject } from "@t3tools/client-runtime/state/shell"; +import { readProjects, readServerConfigs, useProjects, useServerConfigs } from "~/state/entities"; import { threadEnvironment } from "~/state/threads"; import { useAtomCommand } from "~/state/use-atom-command"; -/** Routes link actions through the command advertised by this environment. */ -export function usePullRequestLinking(environmentId: EnvironmentId | null | undefined) { - const configs = useServerConfigs(); - const projects = useProjects(); - const capabilities = - environmentId == null ? undefined : configs.get(environmentId)?.environment.capabilities; - const mode = threadPullRequestLinkMode(capabilities); +/** The three writes, which depend on nothing a render can change. */ +function usePullRequestLinkCommands() { const link = useAtomCommand(threadEnvironment.linkPullRequest, { reportFailure: false }); const unlink = useAtomCommand(threadEnvironment.unlinkPullRequest, { reportFailure: false }); const updateMetadata = useAtomCommand(threadEnvironment.updateMetadata, { reportFailure: false }); - return useMemo(() => { - const environmentProjects = projects.filter( - (project) => project.environmentId === environmentId, + return useMemo(() => ({ link, unlink, updateMetadata }), [link, unlink, updateMetadata]); +} + +function buildPullRequestLinking({ + environmentId, + capabilities, + projects, + commands, +}: { + readonly environmentId: EnvironmentId | null | undefined; + readonly capabilities: Parameters[0]; + readonly projects: ReadonlyArray; + readonly commands: ReturnType; +}) { + const { link, unlink, updateMetadata } = commands; + const mode = threadPullRequestLinkMode(capabilities); + const environmentProjects = projects.filter((project) => project.environmentId === environmentId); + const canLink = (url: string) => { + const parsed = parseChangeRequestUrl(url); + if (parsed === null || mode === "unsupported") return false; + return ( + (mode === "multiple" ? findProjectOnChangeRequestHost : findProjectForChangeRequest)( + environmentProjects, + parsed, + ) !== undefined ); - const canLink = (url: string) => { - const parsed = parseChangeRequestUrl(url); - if (parsed === null || mode === "unsupported") return false; - return ( - (mode === "multiple" ? findProjectOnChangeRequestHost : findProjectForChangeRequest)( - environmentProjects, - parsed, - ) !== undefined - ); - }; - const isLinked = ( - thread: { - readonly pullRequests?: readonly ThreadPullRequestLink[]; - readonly linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; - } | null, - url: string, - ) => { - if (thread === null || mode === "unsupported") return false; - if (mode !== "multiple") - return ( - thread.linkedPullRequest != null && - matchesLinkedPullRequestUrl(thread.linkedPullRequest, url) - ); - const parsed = parseChangeRequestUrl(url); + }; + const isLinked = ( + thread: { + readonly pullRequests?: readonly ThreadPullRequestLink[]; + readonly linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; + } | null, + url: string, + ) => { + if (thread === null || mode === "unsupported") return false; + if (mode !== "multiple") return ( - parsed !== null && - visibleThreadPullRequests(thread.pullRequests ?? []).some((entry) => - threadPullRequestKeysEqual(entry, parsed), - ) + thread.linkedPullRequest != null && + matchesLinkedPullRequestUrl(thread.linkedPullRequest, url) ); - }; - const changeLink = async (threadRef: ScopedThreadRef, url: string, linked: boolean) => { - const parsed = parseChangeRequestUrl(url); - if (parsed === null || threadRef.environmentId !== environmentId || (linked && !canLink(url))) - throw new Error("The pull request is not available in this environment."); - const legacyProject = findProjectForChangeRequest(environmentProjects, parsed); - const mutation = planThreadPullRequestMutation({ - capabilities, - threadId: threadRef.threadId, - reference: { ...parsed, url }, - legacyProjectId: legacyProject?.id ?? null, - legacyRepository: - sourceControlRepositorySelector(legacyProject?.repositoryIdentity) ?? undefined, - linked, - }); - if (mutation === null) - throw new Error("This environment does not support linking this pull request."); - const result = await (mutation.type === "thread.meta.update" - ? updateMetadata({ environmentId: threadRef.environmentId, input: mutation.input }) - : mutation.type === "thread.pull-request.link" - ? link({ environmentId: threadRef.environmentId, input: mutation.input }) - : unlink({ environmentId: threadRef.environmentId, input: mutation.input })); - if (result._tag === "Failure") { - if (isAtomCommandInterrupted(result)) throw new Error("Link update interrupted."); - throw squashAtomCommandFailure(result); - } - }; - return { mode, canLink, isLinked, changeLink }; - }, [capabilities, environmentId, link, mode, projects, unlink, updateMetadata]); + const parsed = parseChangeRequestUrl(url); + return ( + parsed !== null && + visibleThreadPullRequests(thread.pullRequests ?? []).some((entry) => + threadPullRequestKeysEqual(entry, parsed), + ) + ); + }; + const changeLink = async (threadRef: ScopedThreadRef, url: string, linked: boolean) => { + const parsed = parseChangeRequestUrl(url); + if (parsed === null || threadRef.environmentId !== environmentId || (linked && !canLink(url))) + throw new Error("The pull request is not available in this environment."); + const legacyProject = findProjectForChangeRequest(environmentProjects, parsed); + const mutation = planThreadPullRequestMutation({ + capabilities, + threadId: threadRef.threadId, + reference: { ...parsed, url }, + legacyProjectId: legacyProject?.id ?? null, + legacyRepository: + sourceControlRepositorySelector(legacyProject?.repositoryIdentity) ?? undefined, + linked, + }); + if (mutation === null) + throw new Error("This environment does not support linking this pull request."); + const result = await (mutation.type === "thread.meta.update" + ? updateMetadata({ environmentId: threadRef.environmentId, input: mutation.input }) + : mutation.type === "thread.pull-request.link" + ? link({ environmentId: threadRef.environmentId, input: mutation.input }) + : unlink({ environmentId: threadRef.environmentId, input: mutation.input })); + if (result._tag === "Failure") { + if (isAtomCommandInterrupted(result)) throw new Error("Link update interrupted."); + throw squashAtomCommandFailure(result); + } + }; + return { mode, canLink, isLinked, changeLink }; +} + +/** Routes link actions through the command advertised by this environment. */ +export function usePullRequestLinking(environmentId: EnvironmentId | null | undefined) { + const configs = useServerConfigs(); + const projects = useProjects(); + const commands = usePullRequestLinkCommands(); + const capabilities = + environmentId == null ? undefined : configs.get(environmentId)?.environment.capabilities; + return useMemo( + () => buildPullRequestLinking({ environmentId, capabilities, projects, commands }), + [capabilities, commands, environmentId, projects], + ); +} + +/** + * The same actions, resolved on the click rather than on every render. + * + * A caller that only acts once a menu is open has no use for the projects or the server config + * while it waits. This one is mounted per thread row, so subscribing there would put a project + * or config change through every row in the list and filter the whole projects array once per row. + */ +export function useLazyPullRequestLinking(environmentId: EnvironmentId | null | undefined) { + const commands = usePullRequestLinkCommands(); + return useCallback( + () => + buildPullRequestLinking({ + environmentId, + capabilities: + environmentId == null + ? undefined + : readServerConfigs().get(environmentId)?.environment.capabilities, + projects: readProjects(), + commands, + }), + [commands, environmentId], + ); } diff --git a/apps/web/src/state/entities.ts b/apps/web/src/state/entities.ts index d9610e20717f..72b26b113e85 100644 --- a/apps/web/src/state/entities.ts +++ b/apps/web/src/state/entities.ts @@ -154,6 +154,10 @@ export function readProjects(): ReadonlyArray { return appAtomRegistry.get(environmentProjects.projectsAtom); } +export function readServerConfigs(): ReadonlyMap { + return appAtomRegistry.get(environmentServerConfigsAtom); +} + /** Resolves when the project event reaches the live client store. */ export function waitForProject( ref: ScopedProjectRef,