Skip to content

Commit 8ad7a1c

Browse files
codebanditsssscodebanditssss
committed
fix: normalize pr attention links
- Normalize issue-shaped GitHub PR links before rendering dashboard actions - Add review files and merge conflict fallback links - Allow board card attention links to open directly
1 parent 9ae0573 commit 8ad7a1c

4 files changed

Lines changed: 160 additions & 12 deletions

File tree

‎frontend/src/renderer/components/SessionInspector.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { apiClient, apiErrorMessage } from "../lib/api-client";
1515
import { workspaceQueryKey } from "../hooks/useWorkspaceQuery";
1616
import { formatTimeCompact } from "../lib/format-time";
1717
import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary";
18-
import { prStatusRows, sessionPRDisplaySummaries, type PRDisplayTone } from "../lib/pr-display";
18+
import { prOpenUrl, prStatusRows, sessionPRDisplaySummaries, type PRDisplayTone } from "../lib/pr-display";
1919
import type { SessionStatus, WorkspaceSession } from "../types/workspace";
2020
import { sortedPRs, workerDisplayStatus } from "../types/workspace";
2121
import { BrowserPanelView } from "./BrowserPanel";
@@ -210,6 +210,7 @@ function SummaryView({ session }: { session: WorkspaceSession }) {
210210
}
211211

212212
function PRSummaryCard({ pr }: { pr: SessionPRSummary }) {
213+
const openUrl = prOpenUrl(pr) || pr.htmlUrl || pr.url;
213214
return (
214215
<div className="rounded-[7px] border border-border bg-surface px-3 py-2.5">
215216
<div className="flex items-center gap-2">
@@ -219,7 +220,7 @@ function PRSummaryCard({ pr }: { pr: SessionPRSummary }) {
219220
{pr.state}
220221
</Badge>
221222
<a
222-
href={pr.htmlUrl || pr.url}
223+
href={openUrl}
223224
target="_blank"
224225
rel="noopener noreferrer"
225226
className="ml-auto inline-flex items-center gap-0.5 text-[11px] font-medium text-accent hover:underline"

‎frontend/src/renderer/components/SessionsBoard.tsx‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,10 +264,16 @@ function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: (
264264
const showBranch = branch !== "" && !sameLabel(branch, session.title) && !sameLabel(branch, session.id);
265265
const prSummaries = sessionPRDisplaySummaries(session, useSessionScmSummary(session.id).data);
266266
return (
267-
<button
268-
className="w-full rounded-[7px] border border-border bg-surface text-left transition-colors hover:border-border-strong"
267+
<div
268+
className="w-full cursor-pointer rounded-[7px] border border-border bg-surface text-left transition-colors hover:border-border-strong focus:outline-none focus:ring-2 focus:ring-accent/40"
269269
onClick={onOpen}
270-
type="button"
270+
onKeyDown={(event) => {
271+
if (event.key !== "Enter" && event.key !== " ") return;
272+
event.preventDefault();
273+
onOpen();
274+
}}
275+
role="button"
276+
tabIndex={0}
271277
>
272278
<div className="flex items-center gap-2 px-[13px] pb-[9px] pt-3">
273279
<span className={cn("inline-flex items-center gap-1.5 text-[11px] font-medium", badge.className)}>
@@ -303,7 +309,7 @@ function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: (
303309
"no PR yet"
304310
)}
305311
</div>
306-
</button>
312+
</div>
307313
);
308314
}
309315

@@ -316,7 +322,7 @@ function BoardPRSummary({ className, pr }: { className?: string; pr: SessionPRSu
316322
</span>
317323
{diffSummary ? <span className="truncate">{diffSummary}</span> : null}
318324
<PRStatusStrip pr={pr} />
319-
<PRAttentionPanel className="mt-1.5 pt-1.5" interactiveLinks={false} maxItems={2} pr={pr} />
325+
<PRAttentionPanel className="mt-1.5 pt-1.5" maxItems={2} pr={pr} />
320326
</div>
321327
);
322328
}

‎frontend/src/renderer/lib/pr-display.test.ts‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, expect, it } from "vitest";
22
import type { SessionPRSummary } from "../hooks/useSessionScmSummary";
3-
import { prAttentionItems, prDiffSummary, prStatusRows } from "./pr-display";
3+
import { prAttentionItems, prDiffSummary, prOpenUrl, prStatusRows } from "./pr-display";
44

55
const summary = (overrides: Partial<SessionPRSummary> = {}): SessionPRSummary => ({
66
url: "https://github.com/acme/repo/pull/7",
@@ -100,6 +100,83 @@ describe("prAttentionItems", () => {
100100
});
101101
});
102102

103+
it("normalizes issue-shaped GitHub PR URLs for generic PR links", () => {
104+
expect(
105+
prOpenUrl(
106+
summary({
107+
url: "https://github.com/acme/repo/issues/7",
108+
htmlUrl: "https://github.com/acme/repo/issues/7",
109+
mergeability: { state: "mergeable", reasons: [], prUrl: "https://github.com/acme/repo/issues/7" },
110+
}),
111+
),
112+
).toBe("https://github.com/acme/repo/pull/7");
113+
});
114+
115+
it("falls back to the PR files view for requested changes without discussion links", () => {
116+
const items = prAttentionItems(
117+
summary({
118+
url: "https://github.com/acme/repo/issues/7",
119+
htmlUrl: "https://github.com/acme/repo/issues/7",
120+
review: {
121+
decision: "changes_requested",
122+
hasUnresolvedHumanComments: true,
123+
unresolvedBy: [],
124+
},
125+
mergeability: { state: "mergeable", reasons: [], prUrl: "https://github.com/acme/repo/issues/7" },
126+
}),
127+
);
128+
129+
expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({
130+
label: "Review files",
131+
href: "https://github.com/acme/repo/pull/7/files",
132+
});
133+
});
134+
135+
it("preserves direct review discussion links when available", () => {
136+
const items = prAttentionItems(
137+
summary({
138+
url: "https://github.com/acme/repo/issues/7",
139+
htmlUrl: "https://github.com/acme/repo/issues/7",
140+
review: {
141+
decision: "changes_requested",
142+
hasUnresolvedHumanComments: true,
143+
unresolvedBy: [
144+
{
145+
reviewerId: "alice",
146+
count: 1,
147+
links: [{ url: "https://github.com/acme/repo/pull/7#discussion_r1", file: "main.go", line: 12 }],
148+
},
149+
],
150+
},
151+
mergeability: { state: "mergeable", reasons: [], prUrl: "https://github.com/acme/repo/issues/7" },
152+
}),
153+
);
154+
155+
expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]?.href).toBe(
156+
"https://github.com/acme/repo/pull/7#discussion_r1",
157+
);
158+
});
159+
160+
it("falls back to GitHub conflict resolution for merge conflicts", () => {
161+
const items = prAttentionItems(
162+
summary({
163+
url: "https://github.com/acme/repo/issues/7",
164+
htmlUrl: "https://github.com/acme/repo/issues/7",
165+
mergeability: {
166+
state: "conflicting",
167+
reasons: [],
168+
prUrl: "https://github.com/acme/repo/issues/7",
169+
conflictFiles: [],
170+
},
171+
}),
172+
);
173+
174+
expect(items.find((item) => item.kind === "merge_conflict")?.links[0]).toMatchObject({
175+
label: "Resolve conflicts",
176+
href: "https://github.com/acme/repo/pull/7/conflicts",
177+
});
178+
});
179+
103180
it("suppresses attention once the PR is closed or merged", () => {
104181
expect(
105182
prAttentionItems(

‎frontend/src/renderer/lib/pr-display.ts‎

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,17 @@ export function prAttentionItems(pr: SessionPRSummary): PRAttentionItem[] {
177177
const reviewers = pr.review.unresolvedBy.slice(0, 3);
178178
const links = reviewers.map((reviewer) => ({
179179
label: reviewerLabel(reviewer),
180-
href: reviewer.links.find((link) => link.url)?.url || undefined,
180+
href: reviewer.links.find((link) => link.url)?.url || prFilesUrl(pr),
181181
title: `${reviewer.count} unresolved ${pluralize("comment", reviewer.count)}`,
182182
}));
183+
const fallbackFilesUrl = prFilesUrl(pr);
184+
if (links.length === 0 && fallbackFilesUrl) {
185+
links.push({
186+
label: "Review files",
187+
href: fallbackFilesUrl,
188+
title: "Open changed files",
189+
});
190+
}
183191
items.push({
184192
kind: "review_changes_requested",
185193
title: "Address requested changes",
@@ -200,6 +208,54 @@ export function prAttentionItems(pr: SessionPRSummary): PRAttentionItem[] {
200208
return items;
201209
}
202210

211+
export function prOpenUrl(pr: SessionPRSummary): string | undefined {
212+
return normalizeGitHubPRUrl(pr.htmlUrl || pr.url, pr.number) ?? normalizeGitHubPRUrl(pr.mergeability.prUrl, pr.number);
213+
}
214+
215+
function prFilesUrl(pr: SessionPRSummary): string | undefined {
216+
return appendPRPath(prOpenUrl(pr), "files");
217+
}
218+
219+
function prConflictsUrl(pr: SessionPRSummary): string | undefined {
220+
return appendPRPath(prOpenUrl(pr), "conflicts");
221+
}
222+
223+
function appendPRPath(url: string | undefined, segment: string): string | undefined {
224+
if (!url) return undefined;
225+
const parsed = parseURL(url);
226+
if (!parsed) return undefined;
227+
parsed.pathname = `${parsed.pathname.replace(/\/+$/, "")}/${segment}`;
228+
parsed.search = "";
229+
return parsed.toString();
230+
}
231+
232+
function normalizeGitHubPRUrl(url: string | undefined, number: number): string | undefined {
233+
const parsed = parseURL(url);
234+
if (!parsed || parsed.hostname !== "github.com") return url || undefined;
235+
236+
const parts = parsed.pathname.split("/").filter(Boolean);
237+
if (parts.length >= 4 && parts[2] === "pull" && Number(parts[3]) === number) {
238+
parsed.pathname = `/${parts[0]}/${parts[1]}/pull/${number}`;
239+
parsed.search = "";
240+
return parsed.toString();
241+
}
242+
if (parts.length >= 4 && parts[2] === "issues" && Number(parts[3]) === number) {
243+
parsed.pathname = `/${parts[0]}/${parts[1]}/pull/${number}`;
244+
parsed.search = "";
245+
return parsed.toString();
246+
}
247+
return url || undefined;
248+
}
249+
250+
function parseURL(url: string | undefined): URL | undefined {
251+
if (!url) return undefined;
252+
try {
253+
return new URL(url);
254+
} catch {
255+
return undefined;
256+
}
257+
}
258+
203259
function toCIState(value: string): SessionPRSummary["ci"]["state"] {
204260
return ciStates.has(value as SessionPRSummary["ci"]["state"])
205261
? (value as SessionPRSummary["ci"]["state"])
@@ -331,18 +387,26 @@ function mergeAttention(
331387
fallback: string,
332388
tone: PRDisplayTone,
333389
): PRAttentionItem {
334-
const fileLinks = (pr.mergeability.conflictFiles ?? []).slice(0, 3).map((file) => ({
390+
const fileLinks: PRAttentionLink[] = (pr.mergeability.conflictFiles ?? []).slice(0, 3).map((file) => ({
335391
label: file.path,
336-
href: file.url || pr.mergeability.prUrl || undefined,
392+
href: file.url || prConflictsUrl(pr),
337393
}));
338394
const reasonLinks =
339395
fileLinks.length > 0
340396
? []
341397
: pr.mergeability.reasons.slice(0, 3).map((reason) => ({
342398
label: mergeReasonLabel(reason),
343-
href: pr.mergeability.prUrl || undefined,
399+
href: prConflictsUrl(pr),
344400
}));
345401
const links = fileLinks.length > 0 ? fileLinks : reasonLinks;
402+
const conflictsUrl = prConflictsUrl(pr);
403+
if (links.length === 0 && kind === "merge_conflict" && conflictsUrl) {
404+
links.push({
405+
label: "Resolve conflicts",
406+
href: conflictsUrl,
407+
title: "Open GitHub conflict resolution",
408+
});
409+
}
346410
return {
347411
kind,
348412
title,

0 commit comments

Comments
 (0)