authorgravatar for 62337003+Bil0000@users.noreply.github.comBilal Bakr <62337003+Bil0000@users.noreply.github.com> 2026-09-18 01:38:11+03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-17 15:38:11-07:00
loge26af33b228792bb72349ca4e849a96381578a57
treed234e2dcab7266673f7b53615cef3e6fe9067181
parent6deac7a924e6d90eef400cd218703cd3ad00a383
signature Signed by PGP key B5690EEEBB952194

fix(web): keep PR panel actions in the current thread (#12320)

Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

9 files changed, 502 insertions(+), 267 deletions(-)

apps/web/src/components/ChatView.tsx+10-17
...@@ -210,7 +210,7 @@ import {...@@ -210,7 +210,7 @@ import {
210 selectThreadPreviewMiniPlayer,210 selectThreadPreviewMiniPlayer,
211 usePreviewMiniPlayerStore,211 usePreviewMiniPlayerStore,
212} from "../previewMiniPlayerStore";212} from "../previewMiniPlayerStore";
213import { isThreadOwnPullRequest } from "./pullRequest/pullRequestDetail.logic";213import { pullRequestPanelContext } from "./pullRequest/pullRequestDetail.logic";
214import { PullRequestDetailPanel } from "./pullRequest/PullRequestDetailPanel";214import { PullRequestDetailPanel } from "./pullRequest/PullRequestDetailPanel";
215import { PullRequestDetailGhost } from "./pullRequest/PullRequestGhosts";215import { PullRequestDetailGhost } from "./pullRequest/PullRequestGhosts";
216import { PullRequestsUnavailableState } from "./pullRequest/PullRequestsUnavailableState";216import { PullRequestsUnavailableState } from "./pullRequest/PullRequestsUnavailableState";
...@@ -9627,22 +9627,15 @@ export default function ChatView(props: ChatViewProps) {...@@ -9627,22 +9627,15 @@ export default function ChatView(props: ChatViewProps) {
9627 repository: renderedRightPanelSurface.repository,9627 repository: renderedRightPanelSurface.repository,
9628 number: renderedRightPanelSurface.number,9628 number: renderedRightPanelSurface.number,
9629 }}9629 }}
9630 context={9630 context={pullRequestPanelContext(
9631 isThreadOwnPullRequest(9631 {
9632 {9632 projectId: activeThreadMetadata?.projectId ?? null,
9633 projectId: linkedThreadPullRequest?.projectId ?? null,9633 pullRequests: activeThreadMetadata?.pullRequests,
9634 repository: linkedThreadPullRequest?.repository ?? null,9634 linkedPullRequest: activeThreadMetadata?.linkedPullRequest,
9635 number: linkedThreadPullRequest?.number ?? null,9635 branchPullRequest: activeThreadMetadata?.branchPullRequest,
9636 },9636 },
9637 {9637 renderedRightPanelSurface,
9638 projectId: renderedRightPanelSurface.projectId,9638 )}
9639 repository: renderedRightPanelSurface.repository,
9640 number: renderedRightPanelSurface.number,
9641 },
9642 )
9643 ? "thread"
9644 : "page"
9645 }
9646 composerDraftTarget={composerDraftTarget}9639 composerDraftTarget={composerDraftTarget}
9647 onBack={9640 onBack={
9648 activeThreadRef !== null && pullRequestsSurfaceAvailable && visiblePullRequestCount > 19641 activeThreadRef !== null && pullRequestsSurfaceAvailable && visiblePullRequestCount > 1
apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx created+335
...@@ -0,0 +1,335 @@
1import {
2 EnvironmentId,
3 ProjectId,
4 ThreadId,
5 type ScopedThreadRef,
6 type PullRequestDetailView,
7 type ThreadPullRequestLink,
8} from "@t3tools/contracts";
9import { DEFAULT_CLIENT_SETTINGS } from "@t3tools/contracts/settings";
10import { act, type ReactNode, type ReactElement, type ComponentProps } from "react";
11import { create, type ReactTestRenderer } from "react-test-renderer";
12import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test";
13import { DraftId, useComposerDraftStore } from "~/composerDraftStore";
14
15const { newThread, prepareThread, refresh, Wrapper, Trigger } = vi.hoisted(() => ({
16 newThread: vi.fn(),
17 prepareThread: vi.fn(),
18 refresh: vi.fn(),
19 Wrapper: ({ children }: { children?: ReactNode }) => children,
20 Trigger: ({ children, render }: { children?: ReactNode; render?: ReactElement }) => (
21 <>
22 {render}
23 {children}
24 </>
25 ),
26}));
27vi.mock("@effect/atom-react", () => ({ useAtomValue: () => [] }));
28vi.mock("~/state/server", () => ({ primaryServerKeybindingsAtom: {} }));
29vi.mock("~/state/entities", () => ({ useProjects: () => [], useServerConfigs: () => new Map() }));
30vi.mock("~/state/environments", () => ({
31 useEnvironments: () => ({ environments: [] }),
32 usePrimaryEnvironmentId: () => EnvironmentId.make("env-1"),
33}));
34vi.mock("~/hooks/useSettings", () => ({
35 useClientSettings: (select: (settings: typeof DEFAULT_CLIENT_SETTINGS) => unknown) =>
36 select(DEFAULT_CLIENT_SETTINGS),
37}));
38vi.mock("~/hooks/useLiveRefresh", () => ({ useLiveRefresh: () => {} }));
39vi.mock("~/hooks/useHandleNewThread", () => ({ useNewThreadHandler: () => newThread }));
40vi.mock("~/lib/sourceControlActions", () => ({
41 usePreparePullRequestThreadAction: () => ({ run: prepareThread }),
42}));
43vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => vi.fn() }));
44vi.mock("~/state/pullRequests", () => ({
45 pullRequestEnvironment: { detail: () => "detail", activity: () => "activity" },
46 usePullRequestTurnRefresh: () => 0,
47 useSharedPullRequestSummary: () => null,
48}));
49vi.mock("~/state/vcs", () => ({ vcsEnvironment: { listRefs: () => null } }));
50vi.mock("~/state/query", () => ({
51 useEnvironmentQuery: (query: string) => ({
52 data: query === "detail" ? detail : null,
53 isPending: false,
54 isSuccess: true,
55 error: null,
56 refresh,
57 }),
58}));
59vi.mock("~/state/usePullRequestStack", () => ({
60 usePullRequestStack: () => ({
61 data: null,
62 isSuccess: true,
63 isPending: false,
64 error: null,
65 refresh,
66 }),
67}));
68vi.mock("../ui/toast", () => ({ toastManager: { add: vi.fn(), update: vi.fn() } }));
69vi.mock("../ui/tooltip", () => ({
70 TooltipProvider: Wrapper,
71 Tooltip: Wrapper,
72 TooltipTrigger: Trigger,
73 TooltipPopup: () => null,
74}));
75vi.mock("../ui/menu", () => ({
76 Menu: Wrapper,
77 MenuPopup: Wrapper,
78 MenuTrigger: Trigger,
79 MenuItem: "button",
80 MenuRadioGroup: Wrapper,
81 MenuRadioItem: "button",
82 MenuSeparator: () => null,
83 MenuShortcut: () => null,
84}));
85vi.mock("../ui/alert-dialog", () => ({
86 AlertDialog: () => null,
87 AlertDialogPopup: Wrapper,
88 AlertDialogHeader: Wrapper,
89 AlertDialogTitle: Wrapper,
90 AlertDialogDescription: Wrapper,
91 AlertDialogFooter: Wrapper,
92 AlertDialogClose: Wrapper,
93}));
94vi.mock("./PullRequestMarkdown", () => ({
95 PullRequestMarkdownContext: Wrapper,
96 PullRequestMarkdown: () => null,
97}));
98vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => vi.fn() }));
99vi.mock("./PullRequestThreadLinks", () => ({ PullRequestThreadLinks: () => null }));
100vi.mock("./PullRequestSummaryTab", () => ({
101 PullRequestSummaryTab: ({
102 onFixFinding,
103 }: ComponentProps<typeof import("./PullRequestSummaryTab").PullRequestSummaryTab>) => (
104 <button
105 onClick={() =>
106 onFixFinding?.({
107 kind: "check",
108 check: { name: "Unit tests", status: "failure", description: "Test failed", url: null },
109 })
110 }
111 >
112 Fix check
113 </button>
114 ),
115}));
116vi.mock("./PullRequestCodeTab", () => ({
117 default: ({
118 onAddToAgentSelection,
119 }: ComponentProps<typeof import("./PullRequestCodeTab").default>) => (
120 <button
121 onClick={() =>
122 onAddToAgentSelection?.({
123 request: "Fix this line",
124 comment: {
125 id: "note-1",
126 sectionId: "file:a.ts",
127 sectionTitle: "a.ts",
128 filePath: "a.ts",
129 startIndex: 0,
130 endIndex: 0,
131 rangeLabel: "L1",
132 text: "Please fix",
133 diff: "+broken()",
134 },
135 })
136 }
137 >
138 Add to agent
139 </button>
140 ),
141}));
142
143import { PullRequestDetailPanel } from "./PullRequestDetailPanel";
144import { pullRequestPanelContext } from "./pullRequestDetail.logic";
145
146const detail: PullRequestDetailView = {
147 provider: "github",
148 projectId: ProjectId.make("project"),
149 projectTitle: "Project",
150 workspaceRoot: "/workspace",
151 repository: "owner/repo",
152 number: 1,
153 title: "Test pull request",
154 body: "Original description",
155 url: "https://github.com/owner/repo/pull/1",
156 author: { login: "author", name: null, avatarUrl: null },
157 viewer: "author",
158 state: "open",
159 isDraft: false,
160 mergeability: "conflicting",
161 additions: 1,
162 deletions: 0,
163 changedFiles: 1,
164 headBranch: "feature",
165 baseBranch: "main",
166 createdAt: "2026-09-01T00:00:00Z",
167 updatedAt: "2026-09-01T00:00:00Z",
168 mergedAt: null,
169 closedAt: null,
170 reviewers: [],
171 labels: [],
172 checks: [{ name: "Unit tests", status: "success", description: null, url: null }],
173 comments: [],
174 commentCount: 0,
175 commentsTruncated: false,
176 reviewThreads: [],
177 commits: [],
178 mergeCapabilities: { merge: false, squash: false, rebase: false },
179 capabilities: {
180 diff: true,
181 comment: false,
182 search: true,
183 actions: [],
184 mergeMethods: [],
185 review: { inlineComment: false, reply: false, resolve: false, verdicts: [] },
186 reviewers: { request: false, listCandidates: false },
187 edit: { changeRequest: true, comment: false },
188 },
189 viewerPermissions: {
190 actions: [],
191 comment: false,
192 resolve: false,
193 verdicts: [],
194 requestReviewers: false,
195 },
196};
197
198const threadRef: ScopedThreadRef = {
199 environmentId: EnvironmentId.make("env-1"),
200 threadId: ThreadId.make("thread-1"),
201};
202const draftId = DraftId.make("draft-1");
203const newDraftId = DraftId.make("new-draft");
204let renderer: ReactTestRenderer;
205
206beforeEach(() => {
207 vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);
208 vi.stubGlobal("window", { addEventListener: vi.fn(), removeEventListener: vi.fn() });
209 useComposerDraftStore.setState({ draftsByThreadKey: {} });
210 newThread
211 .mockReset()
212 .mockResolvedValue({ draftId: newDraftId, threadId: ThreadId.make("new-thread") });
213 prepareThread.mockReset().mockResolvedValue({
214 _tag: "Success",
215 value: { branch: "feature", worktreePath: "/workspace/pr" },
216 });
217});
218afterEach(() => {
219 act(() => renderer?.unmount());
220 vi.unstubAllGlobals();
221});
222
223async function click(label: string) {
224 const button = renderer.root
225 .findAllByType("button")
226 .find(
227 (node) =>
228 node.props["aria-label"] === label ||
229 node.findAll((child) => child.children.includes(label)).length > 0,
230 );
231 expect(button, label).toBeDefined();
232 await act(async () =>
233 button!.props.onClick({
234 nativeEvent: new Event("click"),
235 preventDefault() {},
236 stopPropagation() {},
237 }),
238 );
239}
240
241const actions = [
242 "Resolve conflicts",
243 "Ask a question",
244 "Explain this PR",
245 "Fix findings in this thread",
246 "Fix check",
247 "Add to agent",
248];
249
250// The surface ChatView opens for `detail`, and the thread states it can be opened beside. The
251// context prop is derived here the way ChatView derives it, so a wrong answer from the thread's
252// link list fails these cases rather than only a hand-picked prop.
253const surface = { projectId: detail.projectId, repository: detail.repository, number: 1 };
254const link = (number: number, source: ThreadPullRequestLink["source"]): ThreadPullRequestLink => ({
255 host: "github.com",
256 repository: detail.repository,
257 number,
258 url: `https://github.com/${detail.repository}/pull/${number}`,
259 source,
260 linkedAt: "2026-09-01T00:00:00Z",
261 snapshot: null,
262 stack: null,
263});
264const stackThread = {
265 projectId: detail.projectId,
266 pullRequests: [link(3, "manual"), link(2, "stack"), link(1, "stack")],
267 // The server's one-slot field names the top layer; the panel shows the bottom one.
268 linkedPullRequest: { ...surface, number: 3, url: link(3, "manual").url },
269};
270const unrelatedThread = {
271 projectId: detail.projectId,
272 pullRequests: [link(9, "created")],
273 linkedPullRequest: { ...surface, number: 9, url: link(9, "created").url },
274};
275
276describe.each([
277 ["own PR, a lower layer of the thread's stack", stackThread, threadRef],
278 ["another PR beside the current thread", unrelatedThread, threadRef],
279 ["PR beside an unsent draft", null, draftId],
280 ["standalone PR page", null, undefined],
281] as const)("%s", (_name, thread, target) => {
282 const context = thread ? pullRequestPanelContext(thread, surface) : "page";
283
284 function render() {
285 renderer = create(
286 <PullRequestDetailPanel
287 environmentId={threadRef.environmentId}
288 reference={detail}
289 context={context}
290 {...(target ? { composerDraftTarget: target, threadRef } : {})}
291 shortcutsEnabled={false}
292 getShortcutContext={() => ({
293 terminalFocus: false,
294 terminalOpen: false,
295 previewFocus: false,
296 previewOpen: false,
297 })}
298 />,
299 );
300 }
301
302 it(`${thread === stackThread ? "hides" : "offers"} the checkout`, async () => {
303 await act(async () => render());
304 const checkout = renderer.root
305 .findAllByType("button")
306 .filter((node) => node.props["aria-label"] === "Check out");
307 expect(checkout).toHaveLength(thread === stackThread ? 0 : 1);
308 });
309
310 it.each(actions)("%s writes to the correct composer", async (action) => {
311 if (target) useComposerDraftStore.getState().setPrompt(target, "Keep my draft");
312 await act(async () => render());
313 if (action === "Add to agent") await click("Code");
314 await click(target ? action : action.replace("in this thread", "in a thread"));
315 const draft = useComposerDraftStore.getState().getComposerDraft(target ?? newDraftId);
316 if (action === "Resolve conflicts") expect(draft?.prompt).toContain("resolve every conflict");
317 else if (action === "Fix check") expect(draft?.prompt).toContain("Fix the failing check");
318 else if (action.startsWith("Fix findings"))
319 expect(draft?.prompt).toContain("Fix the actionable findings");
320 else expect(draft?.reviewComments?.length).toBeGreaterThan(0);
321 if (action === "Add to agent") {
322 expect(draft?.prompt).toContain("Fix this line");
323 expect(draft?.reviewComments).toEqual(
324 expect.arrayContaining([expect.objectContaining({ id: "note-1", diff: "+broken()" })]),
325 );
326 }
327 if (target) {
328 expect(draft?.prompt).toContain("Keep my draft");
329 expect(newThread).not.toHaveBeenCalled();
330 expect(prepareThread).not.toHaveBeenCalled();
331 } else {
332 expect(newThread).toHaveBeenCalled();
333 }
334 });
335});
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+2-9
...@@ -138,7 +138,6 @@ import {...@@ -138,7 +138,6 @@ import {
138 isStackedPullRequestBase,138 isStackedPullRequestBase,
139 pullRequestActionMenuHasGroup,139 pullRequestActionMenuHasGroup,
140 pullRequestActionNeedsHostRefresh,140 pullRequestActionNeedsHostRefresh,
141 pullRequestComposerTarget,
142 pullRequestCheckoutCommand,141 pullRequestCheckoutCommand,
143 pullRequestFindingKey,142 pullRequestFindingKey,
144 pullRequestHandoffLabels,143 pullRequestHandoffLabels,
...@@ -511,10 +510,7 @@ export function PullRequestDetailPanel({...@@ -511,10 +510,7 @@ export function PullRequestDetailPanel({
511 * again is at best a no-op and at worst git refusing a branch two checkouts.510 * again is at best a no-op and at worst git refusing a branch two checkouts.
512 */511 */
513 context?: "page" | "thread";512 context?: "page" | "thread";
514 /**513 /** The open thread's composer. */
515 * The open thread's composer. Beside the thread whose own pull request this is, hand-offs
516 * land here instead of opening a new thread — the branch is already under the reader's feet.
517 */
518 composerDraftTarget?: ScopedThreadRef | DraftId;514 composerDraftTarget?: ScopedThreadRef | DraftId;
519 /**515 /**
520 * Beside a thread, the way back to that thread's list of pull requests. The tab strip can516 * Beside a thread, the way back to that thread's list of pull requests. The tab strip can
...@@ -1064,10 +1060,7 @@ export function PullRequestDetailPanel({...@@ -1064,10 +1060,7 @@ export function PullRequestDetailPanel({
1064 reviewComments?: ReadonlyArray<ReviewCommentContext>;1060 reviewComments?: ReadonlyArray<ReviewCommentContext>;
1065 };1061 };
10661062
1067 // Beside the thread whose own pull request this is, a task belongs in that thread's composer:1063 const attachTarget = composerDraftTarget ?? null;
1068 // the branch is already checked out under it, so opening a second thread would only scatter
1069 // the work.
1070 const attachTarget = pullRequestComposerTarget(context, composerDraftTarget);
1071 const handoffLabels = pullRequestHandoffLabels(attachTarget !== null);1064 const handoffLabels = pullRequestHandoffLabels(attachTarget !== null);
10721065
1073 const writeTaskToComposer = (target: ScopedThreadRef | DraftId, task: ThreadTask) => {1066 const writeTaskToComposer = (target: ScopedThreadRef | DraftId, task: ThreadTask) => {
apps/web/src/components/pullRequest/PullRequestListEmptyState.test.tsx deleted-54
...@@ -1,54 +0,0 @@
1/**
2 * Which of the four states wins, and which of them offer to ask the hosts again. The component is
3 * called as a plain function and its tree read for text: the elements are walked rather than
4 * invoked, so the button's own hooks never run outside a render.
5 */
6import { isValidElement, type ReactElement, type ReactNode } from "react";
7import { describe, expect, it } from "vite-plus/test";
8
9import { PullRequestListEmptyState } from "./PullRequestListEmptyState";
10
11function textOf(node: ReactNode): string {
12 if (typeof node === "string" || typeof node === "number") return String(node);
13 if (Array.isArray(node)) return node.map(textOf).join(" ");
14 if (!isValidElement(node)) return "";
15 return textOf((node as ReactElement<{ children?: ReactNode }>).props.children);
16}
17
18const baseProps = {
19 query: "",
20 filtered: false,
21 searching: false,
22 hasProjects: true,
23 canLoadMore: false,
24 loadingMore: false,
25 refreshing: false,
26 onClearQuery: () => {},
27 onLoadMore: () => {},
28 onRefresh: () => {},
29};
30
31function render(props: Partial<typeof baseProps>): string {
32 return textOf(PullRequestListEmptyState({ ...baseProps, ...props }));
33}
34
35describe("PullRequestListEmptyState", () => {
36 it("asks for a project ahead of anything a search or a filter could say", () => {
37 const text = render({ hasProjects: false, searching: true, query: "fix", filtered: true });
38 expect(text).toContain("No projects in this workspace");
39 expect(text).toContain("Add project");
40 });
41
42 it("leaves the retry off the states where asking again could not change the answer", () => {
43 expect(render({ hasProjects: false })).not.toContain("Check again");
44 expect(render({ searching: true, query: "fix" })).not.toContain("Check again");
45 });
46
47 it("offers the retry once the hosts have answered", () => {
48 expect(render({})).toContain("Check again");
49 expect(render({ filtered: true })).toContain("Check again");
50 expect(render({ query: "fix" })).toContain("Check again");
51 expect(render({ canLoadMore: true })).toContain("Load more pull requests");
52 expect(render({ refreshing: true })).toContain("Checking...");
53 });
54});
apps/web/src/components/pullRequest/PullRequestsUnavailableState.test.tsx deleted-73
...@@ -1,73 +0,0 @@
1import { isValidElement, type ReactElement, type ReactNode } from "react";
2import { renderToStaticMarkup } from "react-dom/server";
3import { describe, expect, it } from "vite-plus/test";
4
5import { PullRequestsUnavailableState } from "./PullRequestsUnavailableState";
6
7function textOf(node: ReactNode): string {
8 if (typeof node === "string" || typeof node === "number") return String(node);
9 if (Array.isArray(node)) return node.map(textOf).join(" ");
10 if (!isValidElement(node)) return "";
11 return textOf((node as ReactElement<{ children?: ReactNode }>).props.children);
12}
13
14describe("PullRequestsUnavailableState", () => {
15 it("can explain an unsupported environment without offering a futile retry", () => {
16 const text = textOf(
17 PullRequestsUnavailableState({
18 title: "Pull requests unavailable",
19 error: "Update this environment's T3 Code server to browse pull requests.",
20 }),
21 );
22
23 expect(text).toContain("Pull requests unavailable");
24 expect(text).toContain("Update this environment's T3 Code server");
25 expect(text).not.toContain("Retry");
26 });
27
28 it("retains the retry for transient load failures", () => {
29 const html = renderToStaticMarkup(
30 <PullRequestsUnavailableState
31 error="GitHub did not answer."
32 onRetry={() => {}}
33 gitHubUrl="https://github.com/pingdotgg/t3code/pull/42"
34 />,
35 );
36
37 expect(html).toContain("Retry");
38 expect(html).toContain("Open on GitHub");
39 expect(html).toContain('href="https://github.com/pingdotgg/t3code/pull/42"');
40 expect(html).toContain('target="_blank"');
41 expect(html).toContain('rel="noopener noreferrer"');
42 });
43
44 it("can offer the browser without offering a retry", () => {
45 const html = renderToStaticMarkup(
46 <PullRequestsUnavailableState
47 error="This server cannot read the pull request."
48 gitHubUrl="https://github.com/pingdotgg/t3code/pull/9"
49 />,
50 );
51
52 expect(html).toContain("Open on GitHub");
53 expect(html).not.toContain("Retry");
54 });
55
56 it("can offer a retry without offering GitHub", () => {
57 const html = renderToStaticMarkup(
58 <PullRequestsUnavailableState error="The host did not answer." onRetry={() => {}} />,
59 );
60
61 expect(html).toContain("Retry");
62 expect(html).not.toContain("Open on GitHub");
63 });
64
65 it("renders no action content without a retry or browser target", () => {
66 const html = renderToStaticMarkup(
67 <PullRequestsUnavailableState error="This project has no known remote." />,
68 );
69
70 expect(html).not.toContain('data-slot="empty-content"');
71 expect(html).not.toContain("href=");
72 });
73});
apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts+111-55
...@@ -8,6 +8,7 @@ import {...@@ -8,6 +8,7 @@ import {
8 type PullRequestDetail,8 type PullRequestDetail,
9 type PullRequestDetailView,9 type PullRequestDetailView,
10 type PullRequestReviewThread,10 type PullRequestReviewThread,
11 type ThreadPullRequestLink,
11} from "@t3tools/contracts";12} from "@t3tools/contracts";
12import { describe, expect, it } from "vite-plus/test";13import { describe, expect, it } from "vite-plus/test";
13import { formatInlineContextReference } from "~/lib/composerContextReferences";14import { formatInlineContextReference } from "~/lib/composerContextReferences";
...@@ -26,7 +27,7 @@ import {...@@ -26,7 +27,7 @@ import {
26 stripPullRequestHandoffReferences,27 stripPullRequestHandoffReferences,
27 isPullRequestVerdictStale,28 isPullRequestVerdictStale,
28 isStackedPullRequestBase,29 isStackedPullRequestBase,
29 isThreadOwnPullRequest,30 pullRequestPanelContext,
30 latestPullRequestReviewOutcomes,31 latestPullRequestReviewOutcomes,
31 newestPullRequestCommitAt,32 newestPullRequestCommitAt,
32 mergePullRequestThreadComments,33 mergePullRequestThreadComments,
...@@ -34,9 +35,7 @@ import {...@@ -34,9 +35,7 @@ import {
34 pullRequestActionMenuHasGroup,35 pullRequestActionMenuHasGroup,
35 pullRequestActionNeedsHostRefresh,36 pullRequestActionNeedsHostRefresh,
36 pullRequestCheckoutCommand,37 pullRequestCheckoutCommand,
37 pullRequestComposerTarget,
38 pullRequestFindingKey,38 pullRequestFindingKey,
39 pullRequestHandoffLabels,
40 pullRequestReviewOutcome,39 pullRequestReviewOutcome,
41 readableFailure,40 readableFailure,
42 readPullRequestDetailSnapshot,41 readPullRequestDetailSnapshot,
...@@ -250,33 +249,6 @@ describe("pull request primary control", () => {...@@ -250,33 +249,6 @@ describe("pull request primary control", () => {
250 });249 });
251});250});
252251
253describe("pull request handoff labels", () => {
254 it("names the open thread when actions write to its composer", () => {
255 expect(pullRequestHandoffLabels(true)).toEqual({
256 fixFinding: "Fix in this thread",
257 fixCheck: "Fix in this thread",
258 fixFindings: "Fix findings in this thread",
259 });
260 });
261
262 it("keeps the standalone pull request page labels", () => {
263 expect(pullRequestHandoffLabels(false)).toEqual({
264 fixFinding: "Fix in a thread",
265 fixCheck: "Fix",
266 fixFindings: "Fix findings in a thread",
267 });
268 });
269});
270
271describe("pull request composer target", () => {
272 it("rejects a page composer so agent comments cannot open another thread", () => {
273 const target = { environmentId: "env-1", threadId: "thread-1" };
274
275 expect(pullRequestComposerTarget("page", target)).toBeNull();
276 expect(pullRequestComposerTarget("thread", target)).toBe(target);
277 });
278});
279
280describe("stacked pull request classification", () => {252describe("stacked pull request classification", () => {
281 it("requires a known default branch", () => {253 it("requires a known default branch", () => {
282 expect(isStackedPullRequestBase("main", [{ name: "main", isDefault: false }])).toBe(false);254 expect(isStackedPullRequestBase("main", [{ name: "main", isDefault: false }])).toBe(false);
...@@ -1373,40 +1345,124 @@ describe("how the branch stands against its base", () => {...@@ -1373,40 +1345,124 @@ describe("how the branch stands against its base", () => {
1373 });1345 });
1374});1346});
13751347
1376describe("whether the panel is showing the thread's own pull request", () => {1348describe("pull request panel context beside a thread", () => {
1377 const surface = { projectId: "proj-a", repository: "acme/app", number: 7 };1349 // Shapes copied from real threads: a thread that opened a stack holds the top layer as a
1350 // manual link and every lower layer as a "stack" link, with the legacy field pointing at
1351 // whichever one the server chose. Snapshots are null until the sync reactor's first pass.
1352 const link = (
1353 number: number,
1354 overrides: Partial<ThreadPullRequestLink> = {},
1355 ): ThreadPullRequestLink => ({
1356 host: "github.com",
1357 repository: "pingdotgg/t3code",
1358 number,
1359 url: `https://github.com/pingdotgg/t3code/pull/${number}`,
1360 source: "manual",
1361 linkedAt: "2026-09-09T00:00:00Z",
1362 snapshot: null,
1363 stack: null,
1364 ...overrides,
1365 });
1366 const surface = (
1367 number: number,
1368 overrides: Partial<Parameters<typeof pullRequestPanelContext>[1]> = {},
1369 ) => ({
1370 projectId: "proj-a",
1371 host: "github.com",
1372 repository: "pingdotgg/t3code",
1373 number,
1374 ...overrides,
1375 });
1376 const stackThread = {
1377 projectId: "proj-a",
1378 pullRequests: [
1379 link(10856),
1380 link(10832, { source: "stack" }),
1381 link(10677, { source: "stack" }),
1382 link(10854, { source: "stack" }),
1383 link(10855, { source: "stack" }),
1384 ],
1385 linkedPullRequest: {
1386 projectId: "proj-a",
1387 repository: "pingdotgg/t3code",
1388 number: 10856,
1389 url: "https://github.com/pingdotgg/t3code/pull/10856",
1390 },
1391 };
13781392
1379 it("matches on project, repository and number together", () => {1393 it("treats every layer of the thread's stack as its own, not only the one the legacy field names", () => {
1380 expect(1394 for (const number of [10856, 10832, 10677, 10854, 10855]) {
1381 isThreadOwnPullRequest({ projectId: "proj-a", repository: "acme/app", number: 7 }, surface),1395 expect(pullRequestPanelContext(stackThread, surface(number)), `#${number}`).toBe("thread");
1382 ).toBe(true);1396 }
1383 });1397 });
13841398
1385 it("rejects a second checkout of the same repository under another project", () => {1399 it("does not let the legacy field decide when the thread holds a link list", () => {
1386 expect(1400 // Every prior regression flipped here: a server-side change to which link the legacy field
1387 isThreadOwnPullRequest({ projectId: "proj-b", repository: "acme/app", number: 7 }, surface),1401 // resolves to must not turn the thread's own second link into a checkout-able stranger.
1388 ).toBe(false);1402 const thread = {
1403 projectId: "proj-a",
1404 pullRequests: [link(11101, { source: "created" }), link(11105, { source: "stack" })],
1405 linkedPullRequest: {
1406 projectId: "proj-a",
1407 repository: "pingdotgg/t3code",
1408 number: 11105,
1409 url: "https://github.com/pingdotgg/t3code/pull/11105",
1410 },
1411 };
1412 expect(pullRequestPanelContext(thread, surface(11101))).toBe("thread");
1413 expect(pullRequestPanelContext(thread, surface(11105))).toBe("thread");
1414 expect(pullRequestPanelContext({ ...thread, linkedPullRequest: null }, surface(11101))).toBe(
1415 "thread",
1416 );
1389 });1417 });
13901418
1391 it("rejects another repository or another number", () => {1419 it("is the page for a pull request the thread is not linked to", () => {
1392 expect(1420 expect(pullRequestPanelContext(stackThread, surface(12320))).toBe("page");
1393 isThreadOwnPullRequest({ projectId: "proj-a", repository: "acme/web", number: 7 }, surface),1421 expect(pullRequestPanelContext(stackThread, surface(10856, { repository: "acme/web" }))).toBe(
1394 ).toBe(false);1422 "page",
1395 expect(1423 );
1396 isThreadOwnPullRequest({ projectId: "proj-a", repository: "acme/app", number: 8 }, surface),1424 });
1397 ).toBe(false);1425
1426 it("is the page under another project's checkout of the same repository", () => {
1427 expect(pullRequestPanelContext(stackThread, surface(10856, { projectId: "proj-b" }))).toBe(
1428 "page",
1429 );
1430 });
1431
1432 it("recognizes an unsynced manual link, and matches host and repository case-insensitively", () => {
1433 const thread = { projectId: "proj-a", pullRequests: [link(7, { host: "GitHub.com" })] };
1434 expect(pullRequestPanelContext(thread, surface(7, { repository: "PingDotGG/T3Code" }))).toBe(
1435 "thread",
1436 );
1437 expect(pullRequestPanelContext(thread, surface(7, { host: undefined }))).toBe("thread");
1438 expect(pullRequestPanelContext(thread, surface(7, { host: "gitlab.com" }))).toBe("page");
1439 });
1440
1441 it("ignores a dismissed stack member the reader chose not to see", () => {
1442 const thread = {
1443 projectId: "proj-a",
1444 pullRequests: [link(1), link(2, { source: "stack-dismissed" })],
1445 };
1446 expect(pullRequestPanelContext(thread, surface(2))).toBe("page");
1398 });1447 });
13991448
1400 it("rejects a thread with no project or no pull request of its own", () => {1449 it("falls back to the legacy fields only for a thread with no link list", () => {
1450 const legacy = {
1451 projectId: "proj-a",
1452 repository: "pingdotgg/t3code",
1453 number: 3,
1454 url: "https://github.com/pingdotgg/t3code/pull/3",
1455 };
1401 expect(1456 expect(
1402 isThreadOwnPullRequest({ projectId: null, repository: "acme/app", number: 7 }, surface),1457 pullRequestPanelContext({ projectId: "proj-a", linkedPullRequest: legacy }, surface(3)),
1403 ).toBe(false);1458 ).toBe("thread");
1404 expect(1459 expect(
1405 isThreadOwnPullRequest(1460 pullRequestPanelContext({ projectId: "proj-a", branchPullRequest: legacy }, surface(3)),
1406 { projectId: "proj-a", repository: "acme/app", number: null },1461 ).toBe("thread");
1407 surface,1462 expect(pullRequestPanelContext({ projectId: "proj-a", pullRequests: [] }, surface(3))).toBe(
1408 ),1463 "page",
1409 ).toBe(false);1464 );
1465 expect(pullRequestPanelContext({ projectId: null }, surface(3))).toBe("page");
1410 });1466 });
1411});1467});
14121468
apps/web/src/components/pullRequest/pullRequestDetail.logic.ts+44-18
...@@ -21,8 +21,14 @@ import {...@@ -21,8 +21,14 @@ import {
21 type PullRequestState,21 type PullRequestState,
22 type PullRequestUpdateMethod,22 type PullRequestUpdateMethod,
23 type SourceControlProviderKind,23 type SourceControlProviderKind,
24 type ThreadLinkedPullRequest,
25 type ThreadPullRequestLink,
24 type VcsRef,26 type VcsRef,
25} from "@t3tools/contracts";27} from "@t3tools/contracts";
28import {
29 threadPullRequestKeysEqual,
30 visibleThreadPullRequests,
31} from "@t3tools/shared/threadPullRequests";
2632
27import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/reviewCommentContext";33import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/reviewCommentContext";
28import { reviewCommentContextId } from "~/lib/composerContextRecords";34import { reviewCommentContextId } from "~/lib/composerContextRecords";
...@@ -163,28 +169,55 @@ export function editPullRequestThreadComment<...@@ -163,28 +169,55 @@ export function editPullRequestThreadComment<
163 return comments.map((comment) => (comment.id === commentId ? { ...comment, body } : comment));169 return comments.map((comment) => (comment.id === commentId ? { ...comment, body } : comment));
164}170}
165171
172type LegacyLinkedPullRequest = Pick<ThreadLinkedPullRequest, "repository" | "number">;
173
166/**174/**
167 * Whether the pull request on a right-panel surface is the thread's own one. Repository and175 * How the detail panel behaves beside a thread: "thread" for a pull request the thread itself
168 * number are not enough: one environment can hold two checkouts of the same repository under176 * is linked to (any layer of its stack), "page" for any other one the reader opened there.
177 *
178 * Decided from the thread's full link list, never from the single legacy `linkedPullRequest`:
179 * that field is one server-chosen link out of many, and a thread's own second link or lower
180 * stack layer would otherwise be handed a checkout button for a branch it already works on. The
181 * legacy fields only answer for servers that predate link lists. Repository and number are not
182 * enough either way: one environment can hold two checkouts of the same repository under
169 * different projects, and the other project's checkout is somebody else's branch.183 * different projects, and the other project's checkout is somebody else's branch.
170 */184 */
171export function isThreadOwnPullRequest(185export function pullRequestPanelContext(
172 thread: {186 thread: {
173 readonly projectId: string | null;187 readonly projectId: string | null;
174 readonly repository: string | null;188 readonly pullRequests?: ReadonlyArray<ThreadPullRequestLink> | undefined;
175 readonly number: number | null;189 readonly linkedPullRequest?: LegacyLinkedPullRequest | null | undefined;
190 readonly branchPullRequest?: LegacyLinkedPullRequest | null | undefined;
176 },191 },
177 surface: {192 surface: {
178 readonly projectId: string;193 readonly projectId: string;
194 readonly host?: string | undefined;
179 readonly repository: string;195 readonly repository: string;
180 readonly number: number;196 readonly number: number;
181 },197 },
182): boolean {198): "page" | "thread" {
183 return (199 if (thread.projectId !== surface.projectId) return "page";
184 thread.projectId === surface.projectId &&200 const links = visibleThreadPullRequests(thread.pullRequests ?? []);
185 thread.repository === surface.repository &&201 if (links.length > 0) {
186 thread.number === surface.number202 const repository = surface.repository.toLowerCase();
187 );203 return links.some((link) =>
204 surface.host !== undefined
205 ? threadPullRequestKeysEqual(link, {
206 host: surface.host,
207 repository: surface.repository,
208 number: surface.number,
209 })
210 : link.number === surface.number && link.repository.toLowerCase() === repository,
211 )
212 ? "thread"
213 : "page";
214 }
215 const legacy = thread.linkedPullRequest ?? thread.branchPullRequest ?? null;
216 return legacy !== null &&
217 legacy.repository === surface.repository &&
218 legacy.number === surface.number
219 ? "thread"
220 : "page";
188}221}
189222
190/** Names where a pull-request task will land, without letting each surface guess independently. */223/** Names where a pull-request task will land, without letting each surface guess independently. */
...@@ -202,13 +235,6 @@ export function pullRequestHandoffLabels(inThisThread: boolean) {...@@ -202,13 +235,6 @@ export function pullRequestHandoffLabels(inThisThread: boolean) {
202 };235 };
203}236}
204237
205export function pullRequestComposerTarget<T>(
206 context: "page" | "thread",
207 target: T | null | undefined,
208): T | null {
209 return context === "thread" ? (target ?? null) : null;
210}
211
212/** Whether the open pull-request action group contains at least one action. */238/** Whether the open pull-request action group contains at least one action. */
213export function pullRequestActionMenuHasGroup(239export function pullRequestActionMenuHasGroup(
214 showsDraftToggle: boolean,240 showsDraftToggle: boolean,
apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts deleted-13
...@@ -1,13 +0,0 @@
1import { describe, expect, it } from "vite-plus/test";
2
3import { openOnHostLabel } from "./pullRequestLinkContextMenu";
4
5describe("pull request link context menu", () => {
6 it("names every host it knows, and says nothing false about one it does not", () => {
7 expect(openOnHostLabel("github")).toBe("Open on GitHub");
8 expect(openOnHostLabel("gitlab")).toBe("Open on GitLab");
9 expect(openOnHostLabel("bitbucket")).toBe("Open on Bitbucket");
10 expect(openOnHostLabel("azure-devops")).toBe("Open on Azure DevOps");
11 expect(openOnHostLabel("something-else")).toBe("Open on host");
12 });
13});
apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx deleted-28
...@@ -1,28 +0,0 @@
1import { act } from "react";
2import { create, type ReactTestRenderer } from "react-test-renderer";
3import { afterEach, expect, it, vi } from "vite-plus/test";
4
5import { PullRequestActorAvatar } from "./pullRequestPresentation";
6
7let renderer: ReactTestRenderer | undefined;
8
9afterEach(async () => {
10 await act(async () => renderer?.unmount());
11 vi.unstubAllGlobals();
12});
13
14it("falls back to the actor initial when a remote avatar fails", async () => {
15 vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);
16 await act(async () => {
17 renderer = create(
18 <PullRequestActorAvatar
19 actor={{ login: "octocat", name: null, avatarUrl: "https://ghe.example/octocat.png" }}
20 />,
21 );
22 });
23
24 await act(async () => renderer!.root.findByType("img").props.onError());
25
26 expect(renderer!.root.findAllByType("img")).toHaveLength(0);
27 expect(renderer!.root.findByType("span").children).toEqual(["O"]);
28});