From f8d9f8d7c225ef793ed979d7f7f56dd589716296 Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Sat, 5 Sep 2026 00:41:08 -0400 Subject: [PATCH] feat(web): links to the thread's pull request open the Pull request tab A link an agent writes to the thread's own pull request used to load the github.com page in the thread's browser panel. A plain click now opens the Pull request tab in the right panel instead, the same place the sidebar badge goes. Only the bare address counts: links into the Files tab, the checks page, or a particular comment keep their old behaviour, since the tab cannot show those spots. Links to any other pull request are untouched. Modifier and middle clicks still go outside, and the right-click menu on such a link gains "Open in browser panel" so the host page stays one click away. --- .../src/components/ChatMarkdown.browser.tsx | 55 +++++++++++++++++++ apps/web/src/components/chat/ChatWebLink.tsx | 42 +++++++++++--- .../chat/ThreadPullRequestLinkContext.ts | 18 ++++++ .../pull-requests/pullRequests.logic.test.ts | 43 +++++++++++++++ .../pull-requests/pullRequests.logic.ts | 37 +++++++++++++ .../routes/_chat.$environmentId.$threadId.tsx | 44 ++++++++++----- 6 files changed, 216 insertions(+), 23 deletions(-) create mode 100644 apps/web/src/components/chat/ThreadPullRequestLinkContext.ts diff --git a/apps/web/src/components/ChatMarkdown.browser.tsx b/apps/web/src/components/ChatMarkdown.browser.tsx index 99cc3bfaf..764fa5ed1 100644 --- a/apps/web/src/components/ChatMarkdown.browser.tsx +++ b/apps/web/src/components/ChatMarkdown.browser.tsx @@ -51,6 +51,7 @@ vi.mock("../localApi", () => ({ })); import ChatMarkdown from "./ChatMarkdown"; +import { ThreadPullRequestLinkContext } from "./chat/ThreadPullRequestLinkContext"; import { __resetEnvironmentApiOverridesForTests, __setEnvironmentApiOverrideForTests, @@ -64,6 +65,12 @@ const CHAT_MARKDOWN_THREAD_REF = scopeThreadRef( CHAT_MARKDOWN_ENVIRONMENT_ID, CHAT_MARKDOWN_THREAD_ID, ); +// The pull request the thread route would hand a transcript, with its opener +// mocked so a click can be observed. +const THREAD_PULL_REQUEST_LINK = { + url: "https://github.com/Threadlines/threadlines/pull/223", + open: vi.fn(), +}; // The inline image loader reads files through react-query, so those renders // need the provider the app root supplies. @@ -540,6 +547,54 @@ describe("ChatMarkdown", () => { } }); + it("opens the thread's own pull request in its tab and leaves deeper links alone", async () => { + const { url: pullRequestUrl, open } = THREAD_PULL_REQUEST_LINK; + open.mockClear(); + const screen = await render( + + + , + ); + + // A click the link leaves alone bubbles past React's root to the window, + // still unclaimed; catching it there keeps the test page from actually + // leaving. A click the link claims is stopped before it gets here. + let leftAlone = false; + const observeClick = (event: Event) => { + leftAlone = !event.defaultPrevented; + event.preventDefault(); + }; + window.addEventListener("click", observeClick); + + try { + const click = (name: string) => { + const anchor = Array.from(document.querySelectorAll("a")).find( + (candidate) => candidate.textContent === name, + ); + if (!anchor) { + throw new Error(`No link named ${name}`); + } + leftAlone = false; + return anchor.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true })); + }; + + expect(click("the PR")).toBe(false); + expect(open).toHaveBeenCalledTimes(1); + + click("its files"); + expect(leftAlone).toBe(true); + expect(open).toHaveBeenCalledTimes(1); + } finally { + window.removeEventListener("click", observeClick); + await screen.unmount(); + } + }); + it("keeps normal web links unchanged", async () => { const screen = await render( , diff --git a/apps/web/src/components/chat/ChatWebLink.tsx b/apps/web/src/components/chat/ChatWebLink.tsx index b60d26157..961ebce84 100644 --- a/apps/web/src/components/chat/ChatWebLink.tsx +++ b/apps/web/src/components/chat/ChatWebLink.tsx @@ -4,8 +4,10 @@ import { memo, useCallback, type MouseEvent as ReactMouseEvent, type ReactNode } import { isElectron } from "../../env"; import { readLocalApi } from "../../localApi"; import { isPlainPrimaryClick, openUrlInBrowserPanel } from "../browser/openInBrowserPanel"; +import { isLinkToPullRequest } from "../pull-requests/pullRequests.logic"; import { stackedThreadToast, toastManager } from "../ui/toast"; import { copyTextWithToast } from "./copyTextWithToast"; +import { useThreadPullRequestLink } from "./ThreadPullRequestLinkContext"; export interface ChatWebLinkProps { href: string; @@ -20,11 +22,13 @@ export interface ChatWebLinkProps { /** * A web address in a transcript. * - * The plain click goes to the thread's own browser, because a link an agent - * wrote is about the work in front of you and leaving the app to read it costs - * you the place you were. Everything else -- modifiers, middle click, and any - * thread without a panel -- falls through to the anchor's own behaviour, so the - * link is never less capable than an ordinary one. + * The plain click stays in the app, because a link an agent wrote is about the + * work in front of you and leaving to read it costs you the place you were. A + * link to the thread's own pull request opens the Pull request tab, the same + * place the sidebar badge goes; any other page goes to the thread's browser. + * Everything else -- modifiers, middle click, and any thread without a panel -- + * falls through to the anchor's own behaviour, so the link is never less + * capable than an ordinary one. */ export const ChatWebLink = memo(function ChatWebLink({ href, @@ -33,9 +37,22 @@ export const ChatWebLink = memo(function ChatWebLink({ className, title, }: ChatWebLinkProps) { + const pullRequestLink = useThreadPullRequestLink(); + const opensPullRequestTab = + pullRequestLink !== null && isLinkToPullRequest(href, pullRequestLink.url); + const handleClick = useCallback( (event: ReactMouseEvent) => { - if (!isPlainPrimaryClick(event) || !isElectron || threadRef === null) { + if (!isPlainPrimaryClick(event)) { + return; + } + if (opensPullRequestTab) { + event.preventDefault(); + event.stopPropagation(); + pullRequestLink?.open(); + return; + } + if (!isElectron || threadRef === null) { return; } if (!openUrlInBrowserPanel(threadRef, href)) { @@ -44,7 +61,7 @@ export const ChatWebLink = memo(function ChatWebLink({ event.preventDefault(); event.stopPropagation(); }, - [href, threadRef], + [href, opensPullRequestTab, pullRequestLink, threadRef], ); const handleContextMenu = useCallback( @@ -55,14 +72,23 @@ export const ChatWebLink = memo(function ChatWebLink({ event.preventDefault(); event.stopPropagation(); + // The browser-panel entry only appears where the plain click no longer + // goes there, so every destination stays one right-click away. const clicked = await api.contextMenu.show( [ + ...(opensPullRequestTab && threadRef !== null + ? [{ id: "open-browser-panel", label: "Open in browser panel" } as const] + : []), { id: "open-external", label: "Open in external browser" }, { id: "copy-link", label: "Copy link address" }, ] as const, { x: event.clientX, y: event.clientY }, ); + if (clicked === "open-browser-panel" && threadRef !== null) { + openUrlInBrowserPanel(threadRef, href); + return; + } if (clicked === "open-external") { void api.shell.openExternal(href).catch((error: unknown) => { toastManager.add( @@ -79,7 +105,7 @@ export const ChatWebLink = memo(function ChatWebLink({ copyTextWithToast(href, "Link address"); } }, - [href], + [href, opensPullRequestTab, threadRef], ); return ( diff --git a/apps/web/src/components/chat/ThreadPullRequestLinkContext.ts b/apps/web/src/components/chat/ThreadPullRequestLinkContext.ts new file mode 100644 index 000000000..aea980e21 --- /dev/null +++ b/apps/web/src/components/chat/ThreadPullRequestLinkContext.ts @@ -0,0 +1,18 @@ +import { createContext, useContext } from "react"; + +/** + * The pull request the thread's right panel can show, handed to the transcript + * so a link to it opens the Pull request tab instead of the host's page. The + * thread route provides it; anywhere else a transcript renders, it is null and + * links behave as they always have. + */ +export interface ThreadPullRequestLink { + readonly url: string; + readonly open: () => void; +} + +export const ThreadPullRequestLinkContext = createContext(null); + +export function useThreadPullRequestLink(): ThreadPullRequestLink | null { + return useContext(ThreadPullRequestLinkContext); +} diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts index 90509e7e5..55e09dc04 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts @@ -23,6 +23,7 @@ import { formatPullRequestChecksSummary, groupPullRequests, groupTimelineRows, + isLinkToPullRequest, linkThreadsToPullRequests, matchesPullRequestQuery, matchesPullRequestSelection, @@ -1133,3 +1134,45 @@ describe("applyPendingPullRequestReactions", () => { ).toEqual([{ content: "thumbs-up", count: 3, viewerReacted: true }]); }); }); + +describe("isLinkToPullRequest", () => { + const pullRequestUrl = "https://github.com/Threadlines/threadlines/pull/223"; + + it("matches the bare address, ignoring host casing and a trailing slash", () => { + expect(isLinkToPullRequest(pullRequestUrl, pullRequestUrl)).toBe(true); + expect( + isLinkToPullRequest("https://GitHub.com/Threadlines/threadlines/pull/223/", pullRequestUrl), + ).toBe(true); + }); + + it("leaves links into a part of the pull request the tab cannot show", () => { + expect( + isLinkToPullRequest( + "https://github.com/Threadlines/threadlines/pull/223/files", + pullRequestUrl, + ), + ).toBe(false); + expect( + isLinkToPullRequest( + "https://github.com/Threadlines/threadlines/pull/223#issuecomment-1", + pullRequestUrl, + ), + ).toBe(false); + expect( + isLinkToPullRequest( + "https://github.com/Threadlines/threadlines/pull/223?diff=split", + pullRequestUrl, + ), + ).toBe(false); + }); + + it("leaves other pull requests and unreadable addresses alone", () => { + expect( + isLinkToPullRequest("https://github.com/Threadlines/threadlines/pull/224", pullRequestUrl), + ).toBe(false); + expect( + isLinkToPullRequest("https://github.com/Other/threadlines/pull/223", pullRequestUrl), + ).toBe(false); + expect(isLinkToPullRequest("not a url", pullRequestUrl)).toBe(false); + }); +}); diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.ts b/apps/web/src/components/pull-requests/pullRequests.logic.ts index a3008fe04..7d56251f5 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.ts @@ -515,6 +515,43 @@ export interface ThreadPullRequest { readonly repository: string | null; } +/** + * Whether a link in a transcript points at exactly this pull request, so the + * app can show its own Pull request tab instead of the host's page. + * + * Only the bare address counts: a link into the Files tab, the checks page, + * or a particular comment names a spot our tab cannot show, and landing at the + * top of the pull request instead would be the misleading case. The comparison + * ignores the host's casing and a trailing slash, nothing else. + */ +export function isLinkToPullRequest(href: string, pullRequestUrl: string): boolean { + const link = parseHttpUrl(href); + const target = parseHttpUrl(pullRequestUrl); + if (link === null || target === null) { + return false; + } + if (link.search !== "" || link.hash !== "") { + return false; + } + return ( + link.origin.toLowerCase() === target.origin.toLowerCase() && + stripTrailingSlash(link.pathname) === stripTrailingSlash(target.pathname) + ); +} + +function parseHttpUrl(value: string): URL | null { + try { + const url = new URL(value.trim()); + return url.protocol === "https:" || url.protocol === "http:" ? url : null; + } catch { + return null; + } +} + +function stripTrailingSlash(pathname: string): string { + return pathname.endsWith("/") ? pathname.slice(0, -1) : pathname; +} + /** The thread fields both the sidebar summary and the full thread record carry. */ export interface ThreadPullRequestSubject { readonly environmentId: EnvironmentId; diff --git a/apps/web/src/routes/_chat.$environmentId.$threadId.tsx b/apps/web/src/routes/_chat.$environmentId.$threadId.tsx index 1ba1141da..9e5dd0c70 100644 --- a/apps/web/src/routes/_chat.$environmentId.$threadId.tsx +++ b/apps/web/src/routes/_chat.$environmentId.$threadId.tsx @@ -18,6 +18,7 @@ import { finalizePromotedDraftThreadByRef, useComposerDraftStore } from "../comp import { useSavedEnvironmentRegistryStore } from "../environments/runtime"; import { type DiffRouteSearch, parseDiffRouteSearch } from "../diffRouteSearch"; import { AgentsPanel } from "../components/chat/AgentsPanel"; +import { ThreadPullRequestLinkContext } from "../components/chat/ThreadPullRequestLinkContext"; import { ChatRightPanel } from "../components/ChatRightPanel"; import { useAgentsPanelSource } from "../agentsPanelStore"; import { preloadDiffPanel, schedulePreloadDiffPanel } from "../diffPanelPreload"; @@ -474,6 +475,15 @@ function ChatThreadRouteView() { }, [activateDiffTab, currentThreadKey, diffTarget, navigateToTab], ); + // What a transcript link to this pull request opens. Only offered once the + // tab can actually show it, so a link never lands on the tab's fallback. + const threadPullRequestLink = useMemo( + () => + threadPullRequest && threadPullRequestReference + ? { url: threadPullRequest.url, open: () => selectTab("pullRequest") } + : null, + [selectTab, threadPullRequest, threadPullRequestReference], + ); const closeTab = useCallback( (tab: RightPanelTab) => { const nextTab = closeRightPanelTab(currentThreadKey, tab); @@ -644,14 +654,16 @@ function ChatThreadRouteView() { return ( <> - + + + - + + +