feat(FN-3287): document PR link visibility in dashboard README

Merges the documentation update for FN-3287, adding 2 lines to the dashboard README covering PR link visibility behavior. This completes Step 5 of the task.

Fusion-Task-Id: FN-3287
This commit is contained in:
Fusion
2026-05-05 07:17:10 -07:00
committed by gsxdsm
parent ad94ff3016
commit ca30babd97
10 changed files with 281 additions and 67 deletions

View File

@@ -22,7 +22,16 @@ interface MockTask {
title: string; title: string;
description: string; description: string;
worktree?: string; worktree?: string;
prInfo?: unknown; prInfo?: {
number: number;
url: string;
status: "open" | "closed" | "merged";
headBranch?: string;
baseBranch?: string;
title?: string;
commentCount?: number;
lastCheckedAt?: string;
};
column: string; column: string;
} }
@@ -43,6 +52,28 @@ function makeStore(task: MockTask, settings: Record<string, unknown> = {}) {
}); });
} }
function makeStatefulStore(task: MockTask, settings: Record<string, unknown> = {}) {
const emitter = new EventEmitter();
let state = structuredClone(task);
return Object.assign(emitter, {
getTask: vi.fn(async () => structuredClone(state)),
getSettings: vi.fn().mockResolvedValue({ requirePrApproval: false, ...settings }),
updateTask: vi.fn(async (_id: string, patch: Record<string, unknown>) => {
state = { ...state, ...patch };
}),
updatePrInfo: vi.fn(async (_id: string, prInfo: MockTask["prInfo"]) => {
state = { ...state, prInfo: prInfo ?? undefined };
return structuredClone(state);
}),
moveTask: vi.fn(async (_id: string, column: string) => {
state = { ...state, column };
}),
logEntry: vi.fn().mockResolvedValue(undefined),
getActiveMergingTask: vi.fn().mockReturnValue(null),
_getState: () => state,
});
}
describe("processPullRequestMergeTask", () => { describe("processPullRequestMergeTask", () => {
beforeEach(() => { beforeEach(() => {
execMock.mockReset(); execMock.mockReset();
@@ -240,6 +271,59 @@ describe("processPullRequestMergeTask", () => {
expect(store.moveTask).toHaveBeenCalledWith("FN-9004", "done"); expect(store.moveTask).toHaveBeenCalledWith("FN-9004", "done");
}); });
it("preserves PR number/url through create, refresh, and merge completion", async () => {
const task: MockTask = {
id: "FN-9103",
title: "test",
description: "desc",
column: "in-review",
};
const store = makeStatefulStore(task);
const createdPr = {
number: 123,
url: "https://github.com/x/y/pull/123",
status: "open" as const,
headBranch: "fusion/fn-9103",
baseBranch: "main",
title: "PR title",
commentCount: 0,
};
const mergedPr = {
...createdPr,
status: "merged" as const,
commentCount: 2,
};
const github = {
findPrForBranch: vi.fn(async () => null),
createPr: vi.fn(async () => createdPr),
getPrMergeStatus: vi.fn(async () => ({
prInfo: { ...createdPr, commentCount: 1 },
reviewDecision: "APPROVED",
checks: [],
mergeReady: true,
blockingReasons: [],
})),
mergePr: vi.fn(async () => mergedPr),
};
const result = await processPullRequestMergeTask(
store as never,
"/repo",
task.id,
github as never,
() => undefined,
);
expect(result).toBe("merged");
const persisted = (store as { _getState: () => MockTask })._getState();
expect(persisted.column).toBe("done");
expect(persisted.prInfo?.number).toBe(123);
expect(persisted.prInfo?.url).toBe("https://github.com/x/y/pull/123");
expect(store.updatePrInfo).toHaveBeenCalledTimes(3);
});
describe("requirePrApproval", () => { describe("requirePrApproval", () => {
function makeReadyMergeStatus(reviewDecision: string | null) { function makeReadyMergeStatus(reviewDecision: string | null) {
const prInfo = { const prInfo = {

View File

@@ -5721,6 +5721,29 @@ Task with acceptance criteria
expect(updated.log.some((l) => l.action === "PR linked" && l.outcome?.includes("#42"))).toBe(true); expect(updated.log.some((l) => l.action === "PR linked" && l.outcome?.includes("#42"))).toBe(true);
}); });
it("keeps PR number/url after moving task to done", async () => {
const task = await createTestTask();
const prInfo = {
url: "https://github.com/owner/repo/pull/42",
number: 42,
status: "open" as const,
title: "Fix the bug",
headBranch: "kb-001-fix-bug",
baseBranch: "main",
commentCount: 0,
};
await store.updatePrInfo(task.id, prInfo);
await store.moveTask(task.id, "todo");
await store.moveTask(task.id, "in-progress");
await store.moveTask(task.id, "in-review");
await store.moveTask(task.id, "done");
const updated = await store.getTask(task.id);
expect(updated.prInfo?.number).toBe(42);
expect(updated.prInfo?.url).toBe("https://github.com/owner/repo/pull/42");
});
it("updates existing PR info with new values", async () => { it("updates existing PR info with new values", async () => {
const task = await createTestTask(); const task = await createTestTask();
const prInfo1 = { const prInfo1 = {

View File

@@ -466,6 +466,8 @@ The dashboard exposes two automated completion strategies in Settings:
When the merge strategy is **Pull request**: When the merge strategy is **Pull request**:
- Task cards render the PR badge as a direct GitHub link (`#<number>`) that opens the PR in a new tab without triggering the card detail modal
- The task detail modal surfaces linked PR numbers as direct GitHub links for both in-review and completed tasks (not just inside the in-review PR section)
- The task's PR section shows whether kb is waiting on checks/reviews or has merged successfully - The task's PR section shows whether kb is waiting on checks/reviews or has merged successfully
- Required checks must pass before kb merges the PR; optional checks do not block auto-merge - Required checks must pass before kb merges the PR; optional checks do not block auto-merge
- A blocking review state (for example, active changes requested) prevents auto-merge until cleared - A blocking review state (for example, active changes requested) prevents auto-merge until cleared

View File

@@ -17,39 +17,31 @@ function getIssueModifierClass(state: string, stateReason?: string): string {
} }
export function GitHubBadge({ prInfo, issueInfo, onIssueRefresh: _onIssueRefresh }: GitHubBadgeProps) { export function GitHubBadge({ prInfo, issueInfo, onIssueRefresh: _onIssueRefresh }: GitHubBadgeProps) {
const handlePrClick = () => {
if (prInfo?.url) {
window.open(prInfo.url, "_blank", "noopener,noreferrer");
}
};
const handleIssueClick = () => {
if (issueInfo?.url) {
window.open(issueInfo.url, "_blank", "noopener,noreferrer");
}
};
return ( return (
<> <>
{prInfo && ( {prInfo && (
<span <a
className={`card-github-badge card-github-badge--${prInfo.status}`} className={`card-github-badge card-github-badge--${prInfo.status}`}
title={`PR #${prInfo.number}: ${prInfo.title}`} title={`PR #${prInfo.number}: ${prInfo.title}`}
onClick={handlePrClick} href={prInfo.url}
target="_blank"
rel="noopener noreferrer"
> >
<GitPullRequest size={12} /> <GitPullRequest size={12} />
<span>#{prInfo.number}</span> <span>#{prInfo.number}</span>
</span> </a>
)} )}
{issueInfo && ( {issueInfo && (
<span <a
className={`card-github-badge ${getIssueModifierClass(issueInfo.state, issueInfo.stateReason)}`} className={`card-github-badge ${getIssueModifierClass(issueInfo.state, issueInfo.stateReason)}`}
title={`Issue #${issueInfo.number}: ${issueInfo.title}`} title={`Issue #${issueInfo.number}: ${issueInfo.title}`}
onClick={handleIssueClick} href={issueInfo.url}
target="_blank"
rel="noopener noreferrer"
> >
<CircleDot size={12} /> <CircleDot size={12} />
<span>#{issueInfo.number}</span> <span>#{issueInfo.number}</span>
</span> </a>
)} )}
</> </>
); );

View File

@@ -56,7 +56,18 @@ export function MergeDetails({ task }: MergeDetailsProps) {
<div className="detail-log-entry"> <div className="detail-log-entry">
<div className="detail-log-header"> <div className="detail-log-header">
<span className="detail-log-action">PR</span> <span className="detail-log-action">PR</span>
{task.prInfo?.url ? (
<a
className="detail-source-link detail-log-outcome"
href={task.prInfo.url}
target="_blank"
rel="noopener noreferrer"
>
#{details.prNumber}
</a>
) : (
<span className="detail-log-outcome">#{details.prNumber}</span> <span className="detail-log-outcome">#{details.prNumber}</span>
)}
</div> </div>
</div> </div>
) : null} ) : null}

View File

@@ -1789,6 +1789,26 @@ export function TaskDetailContent({
</span> </span>
</div> </div>
)} )}
{(task.prInfo?.number || task.mergeDetails?.prNumber) && (
<div className="detail-provenance detail-pr-link-row">
<GitBranch aria-hidden="true" />
<span>
PR{" "}
{task.prInfo?.url ? (
<a
className="detail-provenance-link"
href={task.prInfo.url}
target="_blank"
rel="noopener noreferrer"
>
#{task.prInfo.number}
</a>
) : (
<span>#{task.prInfo?.number ?? task.mergeDetails?.prNumber}</span>
)}
</span>
</div>
)}
</div> </div>
</> </>
)} )}

View File

@@ -1,5 +1,5 @@
import { describe, it, expect, vi } from "vitest"; import { describe, it, expect, vi } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react"; import { render, screen } from "@testing-library/react";
import type { IssueInfo, PrInfo } from "@fusion/core"; import type { IssueInfo, PrInfo } from "@fusion/core";
import { GitHubBadge } from "../GitHubBadge"; import { GitHubBadge } from "../GitHubBadge";
@@ -141,56 +141,23 @@ describe("GitHubBadge", () => {
}); });
}); });
describe("Click behavior", () => { describe("Link behavior", () => {
it("opens PR URL in new tab when PR badge is clicked", () => { it("renders PR badge as a semantic link", () => {
const mockOpen = vi.fn();
vi.stubGlobal("open", mockOpen);
render(<GitHubBadge prInfo={mockPrInfo} />); render(<GitHubBadge prInfo={mockPrInfo} />);
const badge = screen.getByTitle("PR #42: Fix critical bug"); const badge = screen.getByRole("link", { name: "#42" });
fireEvent.click(badge); expect(badge).toHaveAttribute("href", "https://github.com/owner/repo/pull/42");
expect(badge).toHaveAttribute("target", "_blank");
expect(mockOpen).toHaveBeenCalledWith( expect(badge).toHaveAttribute("rel", "noopener noreferrer");
"https://github.com/owner/repo/pull/42",
"_blank",
"noopener,noreferrer"
);
vi.unstubAllGlobals();
}); });
it("opens Issue URL in new tab when Issue badge is clicked", () => { it("renders Issue badge as a semantic link", () => {
const mockOpen = vi.fn();
vi.stubGlobal("open", mockOpen);
render(<GitHubBadge issueInfo={mockIssueInfo} />); render(<GitHubBadge issueInfo={mockIssueInfo} />);
const badge = screen.getByTitle("Issue #123: Feature request: dark mode"); const badge = screen.getByRole("link", { name: "#123" });
fireEvent.click(badge); expect(badge).toHaveAttribute("href", "https://github.com/owner/repo/issues/123");
expect(badge).toHaveAttribute("target", "_blank");
expect(mockOpen).toHaveBeenCalledWith( expect(badge).toHaveAttribute("rel", "noopener noreferrer");
"https://github.com/owner/repo/issues/123",
"_blank",
"noopener,noreferrer"
);
vi.unstubAllGlobals();
});
it("does not open window when PR badge is clicked but URL is missing", () => {
const mockOpen = vi.fn();
vi.stubGlobal("open", mockOpen);
const prWithoutUrl: PrInfo = { ...mockPrInfo, url: "" };
render(<GitHubBadge prInfo={prWithoutUrl} />);
const badge = screen.getByTitle("PR #42: Fix critical bug");
fireEvent.click(badge);
expect(mockOpen).not.toHaveBeenCalled();
vi.unstubAllGlobals();
}); });
}); });

View File

@@ -59,6 +59,53 @@ describe("TaskCard", () => {
expect(screen.getByText("FN-001")).toBeDefined(); expect(screen.getByText("FN-001")).toBeDefined();
}); });
it("clicking PR badge link does not open the task detail modal", () => {
const onOpenDetail = vi.fn();
render(
<TaskCard
task={makeTask({
column: "in-review",
prInfo: {
url: "https://github.com/owner/repo/pull/42",
number: 42,
status: "open",
title: "PR",
headBranch: "fusion/fn-001",
baseBranch: "main",
commentCount: 0,
} as any,
})}
onOpenDetail={onOpenDetail}
addToast={noop}
/>,
);
fireEvent.click(screen.getByRole("link", { name: "#42" }));
expect(onOpenDetail).not.toHaveBeenCalled();
});
it("clicking issue badge text does not open the task detail modal", () => {
const onOpenDetail = vi.fn();
render(
<TaskCard
task={makeTask({
column: "in-review",
issueInfo: {
url: "https://github.com/owner/repo/issues/123",
number: 123,
state: "open",
title: "Issue",
} as any,
})}
onOpenDetail={onOpenDetail}
addToast={noop}
/>,
);
fireEvent.click(screen.getByText("#123"));
expect(onOpenDetail).not.toHaveBeenCalled();
});
it("renders the status badge when task.status is set", () => { it("renders the status badge when task.status is set", () => {
render( render(
<TaskCard <TaskCard

View File

@@ -2971,6 +2971,60 @@ describe("TaskDetailModal", () => {
expect(screen.queryByText("Merge & Close")).toBeNull(); expect(screen.queryByText("Merge & Close")).toBeNull();
}); });
it("shows linked PR number in detail metadata for in-review tasks", () => {
render(
<TaskDetailModal
task={makeTask({ column: "in-review" as Column, prInfo: {
url: "https://github.com/owner/repo/pull/42",
number: 42,
status: "open",
title: "Task",
headBranch: "fusion/fn-099",
baseBranch: "main",
commentCount: 0,
} })}
onClose={noop}
onMoveTask={noopMove}
onDeleteTask={noopDelete}
onMergeTask={noopMerge}
onOpenDetail={noopOpenDetail}
addToast={noop}
/>,
);
expect(screen.getByRole("link", { name: "#42" })).toHaveAttribute("href", "https://github.com/owner/repo/pull/42");
});
it("shows linked PR number in merge details for done tasks", () => {
render(
<TaskDetailModal
task={makeTask({
column: "done" as Column,
prInfo: {
url: "https://github.com/owner/repo/pull/42",
number: 42,
status: "merged",
title: "Task",
headBranch: "fusion/fn-099",
baseBranch: "main",
commentCount: 0,
},
mergeDetails: { prNumber: 42 },
})}
onClose={noop}
onMoveTask={noopMove}
onDeleteTask={noopDelete}
onMergeTask={noopMerge}
onOpenDetail={noopOpenDetail}
addToast={noop}
/>,
);
const links = screen.getAllByRole("link", { name: "#42" });
expect(links.length).toBeGreaterThan(0);
expect(links[0]).toHaveAttribute("href", "https://github.com/owner/repo/pull/42");
});
it("shows PR automation waiting label instead of Merge & Close when awaiting PR checks", () => { it("shows PR automation waiting label instead of Merge & Close when awaiting PR checks", () => {
render( render(
<TaskDetailModal <TaskDetailModal

View File

@@ -1802,8 +1802,22 @@ input[type="range"]:focus-visible {
font-weight: 600; font-weight: 600;
padding: 2px 6px; padding: 2px 6px;
border-radius: var(--radius-pill); border-radius: var(--radius-pill);
border: 1px solid transparent;
color: inherit;
text-decoration: none;
cursor: pointer; cursor: pointer;
white-space: nowrap; white-space: nowrap;
transition: box-shadow var(--transition-fast), border-color var(--transition-fast), filter var(--transition-fast);
}
.card-github-badge:hover {
filter: brightness(1.08);
}
.card-github-badge:focus-visible {
outline: none;
border-color: currentColor;
box-shadow: var(--focus-ring-strong);
} }
/* GitHub badge theme-aware colors */ /* GitHub badge theme-aware colors */
@@ -1824,8 +1838,8 @@ input[type="range"]:focus-visible {
color: var(--in-progress); color: var(--in-progress);
} }
.card-github-badge--not-planned { .card-github-badge--not-planned {
background: color-mix(in srgb, var(--text-muted, #8b949e) 15%, transparent); background: color-mix(in srgb, var(--text-muted) 15%, transparent);
color: var(--text-muted, #8b949e); color: var(--text-muted);
} }
.pr-number { .pr-number {