fix(web): open review panes in selected split (#2710)

This commit is contained in:
José Almeida
2026-08-16 15:40:11 +01:00
committed by GitHub
parent 63c21686ff
commit 8d006c4011
17 changed files with 978 additions and 95 deletions
@@ -39,6 +39,7 @@ import { TaskMRLinkDialog } from "./task-mr-link-dialog";
import { MRAutomationControls } from "./mr-automation-controls";
import { MRCIPopover } from "./mr-ci-popover";
import { useDockviewStore } from "@/lib/state/dockview-store";
import type { ReviewPanelOptions } from "@/lib/state/dockview-panel-actions";
import { reviewItemId } from "@/components/task/review-selection";
import { mrTaskKey } from "./mr-detail-panel";
import { useTranslation } from "react-i18next";
@@ -86,7 +87,7 @@ export function openMobileMRReview(
}
export function openDesktopMRReview(
addMRPanel: (mrKey: string) => void,
addMRPanel: (mrKey: string, opts?: ReviewPanelOptions) => void,
mr: TaskMR,
schedule: (callback: FrameRequestCallback) => number = requestAnimationFrame,
) {
@@ -1,17 +1,28 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from "@kandev/ui/dropdown-menu";
import { mrTaskKey } from "@/components/gitlab/mr-detail-panel";
import type { TaskPR } from "@/lib/types/github";
import type { TaskMR } from "@/lib/types/gitlab";
import type { ReviewItemSummary } from "@/lib/plugins/types";
import { reviewPanelId } from "@/lib/state/dockview-review-panel-id";
import { AddPanelMenuItems, type AddPanelMenuState } from "./dockview-add-panel-items";
import { pluginRegistry } from "@/lib/plugins/registry";
import { reviewItemId } from "./review-selection";
const { mockAddPRPanel, mockAddTodosPanel } = vi.hoisted(() => ({
mockAddPRPanel: vi.fn(),
mockAddTodosPanel: vi.fn(),
}));
const { mockAddMRPanel, mockAddPRPanel, mockAddReviewPanel, mockAddTodosPanel } = vi.hoisted(
() => ({
mockAddMRPanel: vi.fn(),
mockAddPRPanel: vi.fn(),
mockAddReviewPanel: vi.fn(),
mockAddTodosPanel: vi.fn(),
}),
);
const mockDockviewStore = vi.hoisted(() => ({
api: null,
api: null as null | {
getPanel: (id: string) => { params?: Record<string, unknown> } | undefined;
},
centerGroupId: "group-center",
addBrowserPanel: vi.fn(),
addVscodePanel: vi.fn(),
@@ -21,9 +32,16 @@ const mockDockviewStore = vi.hoisted(() => ({
addChangesPanel: vi.fn(),
addFilesPanel: vi.fn(),
addPRPanel: mockAddPRPanel,
addMRPanel: vi.fn(),
addMRPanel: mockAddMRPanel,
addReviewPanel: mockAddReviewPanel,
addTerminalPanel: vi.fn(),
}));
const CENTER_GROUP_ID = mockDockviewStore.centerGroupId;
const SECONDARY_GROUP_ID = "group-secondary";
const mockNormalizedReviews = vi.hoisted(() => ({
reviews: [] as ReviewItemSummary[],
}));
const mockAppState = vi.hoisted(() => ({
workspaces: { activeId: "workspace-1" },
@@ -54,8 +72,14 @@ vi.mock("@/hooks/domains/workspace/use-repository-scripts", () => ({
useRepositoryScripts: () => ({ scripts: [] }),
}));
vi.mock("./review-panel-provider", () => ({
useNormalizedTaskReviews: () => mockNormalizedReviews.reviews,
}));
const PR_SUBMENU_TEST_ID = "add-panel-pr-submenu";
const PR_ITEM_TEST_ID_PREFIX = "add-panel-pr-item-";
const GITLAB_ORIGIN = "https://gitlab.example";
const TEST_TIMESTAMP = "2026-07-31T00:00:00Z";
function makePR(id: string, number: number, repo = "kandev"): TaskPR {
return {
@@ -81,15 +105,63 @@ function makePR(id: string, number: number, repo = "kandev"): TaskPR {
checks_passing: 0,
additions: 0,
deletions: 0,
created_at: "2026-07-31T00:00:00Z",
created_at: TEST_TIMESTAMP,
merged_at: null,
closed_at: null,
last_synced_at: null,
updated_at: "2026-07-31T00:00:00Z",
updated_at: TEST_TIMESTAMP,
};
}
function renderMenu(state: Partial<AddPanelMenuState> = {}) {
function makeMR(id: string, number: number, projectPath = "acme/kandev"): TaskMR {
return {
id,
task_id: "task-1",
host: GITLAB_ORIGIN,
project_path: projectPath,
mr_iid: number,
mr_url: `${GITLAB_ORIGIN}/${projectPath}/-/merge_requests/${number}`,
mr_title: `MR ${number}`,
head_branch: `feature/${number}`,
base_branch: "main",
author_username: "alice",
state: "open",
approval_state: "",
pipeline_state: "",
merge_status: "",
draft: false,
approval_count: 0,
required_approvals: 0,
pipeline_jobs_total: 0,
pipeline_jobs_pass: 0,
reviewer_count: 0,
unapproved_reviewers: 0,
unresolved_discussions: 0,
created_at: TEST_TIMESTAMP,
updated_at: TEST_TIMESTAMP,
};
}
function makeRegisteredReview(number: number): ReviewItemSummary {
return {
providerId: "bitbucket",
reviewKey: `acme/kandev/${number}`,
title: `Change request ${number}`,
url: `https://bitbucket.example/acme/kandev/pull-requests/${number}`,
connectionScope: "https://bitbucket.example",
repositoryId: "acme/kandev",
changeRequestNumber: number,
state: "open",
};
}
function setOpenPanels(panels: Record<string, Record<string, unknown>>) {
mockDockviewStore.api = {
getPanel: (id) => (Object.hasOwn(panels, id) ? { params: panels[id] } : undefined),
};
}
function renderMenu(state: Partial<AddPanelMenuState> = {}, groupId = CENTER_GROUP_ID) {
const fullState: AddPanelMenuState = {
taskId: null,
isPassthrough: false,
@@ -104,7 +176,7 @@ function renderMenu(state: Partial<AddPanelMenuState> = {}) {
<DropdownMenuTrigger>open</DropdownMenuTrigger>
<DropdownMenuContent>
<AddPanelMenuItems
groupId="group-center"
groupId={groupId}
state={fullState}
onNewSession={() => {}}
onAddTerminal={() => {}}
@@ -123,7 +195,11 @@ async function openPRSubmenu() {
}
beforeEach(() => {
mockDockviewStore.api = null;
mockNormalizedReviews.reviews = [];
mockAddMRPanel.mockClear();
mockAddPRPanel.mockClear();
mockAddReviewPanel.mockClear();
mockAddTodosPanel.mockClear();
});
@@ -163,17 +239,153 @@ describe("AddPanelMenuItems — linked PR rows", () => {
});
it("opens the selected PR panel with its task key and the active session", async () => {
renderMenu({ prs: [makePR("pr-1", 42, "web"), makePR("pr-2", 77, "api")] });
renderMenu({ prs: [makePR("pr-1", 42, "web"), makePR("pr-2", 77, "api")] }, SECONDARY_GROUP_ID);
await openPRSubmenu();
fireEvent.click(screen.getByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-api-77`));
expect(mockAddPRPanel).toHaveBeenCalledWith("acme/api/77");
expect(mockAddPRPanel).toHaveBeenCalledWith("acme/api/77", {
groupId: SECONDARY_GROUP_ID,
});
});
it("keeps the inline row clickable when exactly one PR is linked", () => {
renderMenu({ prs: [makePR("pr-1", 42)] });
fireEvent.click(screen.getByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-kandev-42`));
expect(mockAddPRPanel).toHaveBeenCalledWith("acme/kandev/42");
expect(mockAddPRPanel).toHaveBeenCalledWith("acme/kandev/42", {
groupId: CENTER_GROUP_ID,
});
});
it("hides a canonical open PR and renders the sole missing PR inline", () => {
setOpenPanels({ "pr-detail": { prKey: "acme/web/42" } });
renderMenu({ prs: [makePR("pr-1", 42, "web"), makePR("pr-2", 77, "api")] });
expect(screen.queryByTestId(PR_SUBMENU_TEST_ID)).toBeNull();
expect(screen.queryByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-web-42`)).toBeNull();
expect(screen.getByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-api-77`)).toBeTruthy();
});
it("hides a keyed open PR by exact task key", () => {
setOpenPanels({ "pr-detail|acme/web/42": {} });
renderMenu({ prs: [makePR("pr-1", 42, "web"), makePR("pr-2", 77, "api")] });
expect(screen.queryByTestId(PR_SUBMENU_TEST_ID)).toBeNull();
expect(screen.queryByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-web-42`)).toBeNull();
expect(screen.getByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-api-77`)).toBeTruthy();
});
it("uses the filtered PR count for the submenu and keeps other reviews available", async () => {
setOpenPanels({ "pr-detail": { prKey: "acme/web/42" } });
renderMenu({
prs: [makePR("pr-1", 42, "web"), makePR("pr-2", 77, "api"), makePR("pr-3", 91, "docs")],
});
const trigger = screen.getByTestId(PR_SUBMENU_TEST_ID);
expect(trigger.getAttribute("data-pr-count")).toBe("2");
fireEvent.click(trigger);
expect(await screen.findByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-api-77`)).toBeTruthy();
expect(screen.getByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-docs-91`)).toBeTruthy();
expect(screen.queryByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-web-42`)).toBeNull();
});
it("offers a PR on the next menu mount after its exact panel closes", () => {
setOpenPanels({ "pr-detail|acme/kandev/42": {} });
const firstMenu = renderMenu({ prs: [makePR("pr-1", 42)] });
expect(screen.queryByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-kandev-42`)).toBeNull();
firstMenu.unmount();
setOpenPanels({});
renderMenu({ prs: [makePR("pr-1", 42)] });
expect(screen.getByTestId(`${PR_ITEM_TEST_ID_PREFIX}acme-kandev-42`)).toBeTruthy();
});
});
describe("AddPanelMenuItems — open review identities", () => {
it("opens a missing GitLab MR in the invoking group", () => {
const mr = makeMR("mr-1", 42);
renderMenu({ mrs: [mr] }, SECONDARY_GROUP_ID);
fireEvent.click(screen.getByTestId("add-panel-mr-item-mr-1"));
expect(mockAddMRPanel).toHaveBeenCalledWith(mrTaskKey(mr), {
groupId: SECONDARY_GROUP_ID,
});
});
it("opens a missing registered-provider review in the invoking group", () => {
const review = makeRegisteredReview(42);
mockNormalizedReviews.reviews = [review];
renderMenu({}, SECONDARY_GROUP_ID);
fireEvent.click(screen.getByTestId(`add-panel-review-item-${reviewItemId(review)}`));
expect(mockAddReviewPanel).toHaveBeenCalledWith(review, {
groupId: SECONDARY_GROUP_ID,
});
});
it("hides canonical and keyed GitLab MRs by exact task key", () => {
const canonicalMR = makeMR("mr-1", 42, "acme/web");
const keyedMR = makeMR("mr-2", 77, "acme/api");
const missingMR = makeMR("mr-3", 91, "acme/docs");
setOpenPanels({
"pr-detail": { mrKey: mrTaskKey(canonicalMR) },
[`mr-detail|${mrTaskKey(keyedMR)}`]: {},
});
renderMenu({ mrs: [canonicalMR, keyedMR, missingMR] });
expect(screen.queryByTestId("add-panel-mr-item-mr-1")).toBeNull();
expect(screen.queryByTestId("add-panel-mr-item-mr-2")).toBeNull();
expect(screen.getByTestId("add-panel-mr-item-mr-3")).toBeTruthy();
});
it("uses the single-MR label when filtering leaves one visible MR", () => {
const openMR = makeMR("mr-1", 42, "acme/web");
const missingMR = makeMR("mr-2", 77, "acme/api");
setOpenPanels({ "pr-detail": { mrKey: mrTaskKey(openMR) } });
renderMenu({ mrs: [openMR, missingMR] });
expect(screen.getByTestId("add-panel-mr-item-mr-2").textContent).toContain("Merge Request !77");
});
it("hides registered-provider reviews with exact canonical and keyed identities", () => {
const canonicalReview = makeRegisteredReview(42);
const keyedReview = makeRegisteredReview(77);
const missingReview = makeRegisteredReview(91);
setOpenPanels({
"pr-detail": {
providerId: canonicalReview.providerId,
connectionScope: canonicalReview.connectionScope,
repositoryId: canonicalReview.repositoryId,
changeRequestNumber: canonicalReview.changeRequestNumber,
},
[reviewPanelId(keyedReview)]: {},
});
mockNormalizedReviews.reviews = [canonicalReview, keyedReview, missingReview];
renderMenu();
expect(
screen.queryByTestId(`add-panel-review-item-${reviewItemId(canonicalReview)}`),
).toBeNull();
expect(screen.queryByTestId(`add-panel-review-item-${reviewItemId(keyedReview)}`)).toBeNull();
expect(screen.getByTestId(`add-panel-review-item-${reviewItemId(missingReview)}`)).toBeTruthy();
});
it("does not hide an individual review for a provider-neutral canonical selector", () => {
const review = makeRegisteredReview(42);
setOpenPanels({ "pr-detail": {} });
mockNormalizedReviews.reviews = [review];
renderMenu();
expect(screen.getByTestId(`add-panel-review-item-${reviewItemId(review)}`)).toBeTruthy();
});
});
@@ -205,7 +417,7 @@ describe("AddPanelMenuItems — plugin task panels (AC1)", () => {
"kandev-plugin-notes",
"notes",
"Notes",
{ groupId: "group-center" },
{ groupId: CENTER_GROUP_ID },
);
});
});
@@ -261,7 +473,7 @@ describe("AddPanelMenuItems — Todos", () => {
expect(item).toBeTruthy();
fireEvent.click(item);
expect(mockAddTodosPanel).toHaveBeenCalledWith({ groupId: "group-center" });
expect(mockAddTodosPanel).toHaveBeenCalledWith({ groupId: CENTER_GROUP_ID });
});
it("hides the Todos row for a passthrough session, matching the Plan row's guard", () => {
@@ -18,10 +18,13 @@ import {
DropdownMenuSubContent,
DropdownMenuSubTrigger,
} from "@kandev/ui/dropdown-menu";
import type { DockviewApi } from "dockview-react";
import { prPanelLabel, prIdentitySlug, prTaskKey } from "@/components/github/pr-utils";
import { useDockviewStore } from "@/lib/state/dockview-store";
import { reviewPanelId } from "@/lib/state/dockview-review-panel-id";
import { pluginRegistry, usePluginRegistry } from "@/lib/plugins/registry";
import { resolvePluginIcon } from "@/lib/plugins/icons";
import type { ReviewItemSummary } from "@/lib/plugins/types";
import type { TaskPR } from "@/lib/types/github";
import type { TaskMR } from "@/lib/types/gitlab";
import { mrTaskKey } from "@/components/gitlab/mr-detail-panel";
@@ -57,6 +60,76 @@ export const MENU_ITEM_CLASS = "cursor-pointer text-xs";
const PR_SUBMENU_TEST_ID = "add-panel-pr-submenu";
type ReviewMenuIdentity = Pick<ReviewItemSummary, "providerId" | "reviewKey"> &
Partial<Pick<ReviewItemSummary, "connectionScope" | "repositoryId" | "changeRequestNumber">>;
type ReviewPanelLookup = Pick<DockviewApi, "getPanel">;
function panelParams(api: ReviewPanelLookup, id: string): Record<string, unknown> | undefined {
return api.getPanel(id)?.params as Record<string, unknown> | undefined;
}
function matchesBuiltInReview(
params: Record<string, unknown> | undefined,
providerId: "github" | "gitlab",
reviewKey: string,
legacyKey: "prKey" | "mrKey",
): boolean {
return (
params?.[legacyKey] === reviewKey ||
((params?.providerId === providerId || params?.provider === providerId) &&
params.reviewKey === reviewKey)
);
}
function matchesRegisteredReview(
params: Record<string, unknown> | undefined,
review: Required<ReviewMenuIdentity>,
): boolean {
return (
params?.providerId === review.providerId &&
params.connectionScope === review.connectionScope &&
params.repositoryId === review.repositoryId &&
params.changeRequestNumber !== undefined &&
String(params.changeRequestNumber) === String(review.changeRequestNumber)
);
}
/** True only when this exact review already has a canonical or keyed live panel. */
export function isReviewPanelOpen(
api: ReviewPanelLookup | null,
review: ReviewMenuIdentity,
): boolean {
if (!api) return false;
const canonicalParams = panelParams(api, "pr-detail");
if (review.providerId === "github") {
return (
Boolean(api.getPanel(`pr-detail|${review.reviewKey}`)) ||
matchesBuiltInReview(canonicalParams, "github", review.reviewKey, "prKey")
);
}
if (review.providerId === "gitlab") {
return (
Boolean(api.getPanel(`mr-detail|${review.reviewKey}`)) ||
matchesBuiltInReview(canonicalParams, "gitlab", review.reviewKey, "mrKey") ||
matchesBuiltInReview(panelParams(api, "mr-detail"), "gitlab", review.reviewKey, "mrKey")
);
}
const { connectionScope, repositoryId, changeRequestNumber } = review;
if (
connectionScope === undefined ||
repositoryId === undefined ||
changeRequestNumber === undefined
) {
return false;
}
const registeredReview = { ...review, connectionScope, repositoryId, changeRequestNumber };
return (
Boolean(api.getPanel(reviewPanelId(registeredReview))) ||
matchesRegisteredReview(canonicalParams, registeredReview)
);
}
/**
* Linked GitHub PR entries for the dockview "+" menu. A single PR renders as
* one inline row; multiple PRs collapse behind a "Pull requests" sub-menu so
@@ -135,6 +208,20 @@ function PluginTaskPanelMenuItems({ groupId }: { groupId: string }) {
);
}
function missingBuiltInReviews(
api: ReviewPanelLookup | null,
state: Pick<AddPanelMenuState, "prs" | "mrs">,
): Pick<AddPanelMenuState, "prs" | "mrs"> {
return {
prs: state.prs.filter(
(pr) => !isReviewPanelOpen(api, { providerId: "github", reviewKey: prTaskKey(pr) }),
),
mrs: state.mrs.filter(
(mr) => !isReviewPanelOpen(api, { providerId: "gitlab", reviewKey: mrTaskKey(mr) }),
),
};
}
export function AddPanelMenuItems({
groupId,
state,
@@ -150,9 +237,7 @@ export function AddPanelMenuItems({
const addTodosPanel = useDockviewStore((s) => s.addTodosPanel);
const addFilesPanel = useDockviewStore((s) => s.addFilesPanel);
const addChangesPanel = useDockviewStore((s) => s.addChangesPanel);
const addPRPanel = useDockviewStore((s) => s.addPRPanel);
const addMRPanel = useDockviewStore((s) => s.addMRPanel);
const api = useDockviewStore((s) => s.api);
return (
<>
{state.taskId && (
@@ -213,40 +298,52 @@ export function AddPanelMenuItems({
{t("task:files")}
</DropdownMenuItem>
)}
<PRPanelMenuItems prs={state.prs} onOpenPR={(pr) => addPRPanel(prTaskKey(pr))} />
{state.mrs.map((mr) => (
<DropdownMenuItem
key={mr.id}
onClick={() => addMRPanel(mrTaskKey(mr))}
className={MENU_ITEM_CLASS}
data-testid={`add-panel-mr-item-${mr.id}`}
>
<IconGitPullRequest className={`${MENU_ICON_CLASS} text-orange-500`} />
{state.mrs.length > 1
? `MR !${mr.mr_iid} - ${mr.project_path}`
: t("task:mergeRequest", { mriid: mr.mr_iid })}
</DropdownMenuItem>
))}
<PluginReviewPanelMenuItems taskId={state.taskId} />
<ReviewPanelMenuItems groupId={groupId} state={state} api={api} />
<RepositoryScriptsMenuItems onRunScript={onRunScript} onRunDevScript={onRunDevScript} />
</>
);
}
function PluginReviewPanelMenuItems({ taskId }: { taskId: string | null }) {
function ReviewPanelMenuItems({
groupId,
state,
api,
}: Pick<AddPanelMenuItemsProps, "groupId" | "state"> & { api: ReviewPanelLookup | null }) {
const { t } = useTranslation();
const addPRPanel = useDockviewStore((s) => s.addPRPanel);
const addMRPanel = useDockviewStore((s) => s.addMRPanel);
const addReviewPanel = useDockviewStore((s) => s.addReviewPanel);
const reviews = useNormalizedTaskReviews(taskId).filter(
(review) => review.providerId !== "github" && review.providerId !== "gitlab",
const { prs, mrs } = missingBuiltInReviews(api, state);
const reviews = useNormalizedTaskReviews(state.taskId)
.filter((review) => review.providerId !== "github" && review.providerId !== "gitlab")
.filter((review) => !isReviewPanelOpen(api, review));
return (
<>
<PRPanelMenuItems prs={prs} onOpenPR={(pr) => addPRPanel(prTaskKey(pr), { groupId })} />
{mrs.map((mr) => (
<DropdownMenuItem
key={mr.id}
onClick={() => addMRPanel(mrTaskKey(mr), { groupId })}
className={MENU_ITEM_CLASS}
data-testid={`add-panel-mr-item-${mr.id}`}
>
<IconGitPullRequest className={`${MENU_ICON_CLASS} text-orange-500`} />
{mrs.length > 1
? `MR !${mr.mr_iid} - ${mr.project_path}`
: t("task:mergeRequest", { mriid: mr.mr_iid })}
</DropdownMenuItem>
))}
{reviews.map((review) => (
<DropdownMenuItem
key={reviewItemId(review)}
onClick={() => addReviewPanel(review, { groupId })}
className={MENU_ITEM_CLASS}
data-testid={`add-panel-review-item-${reviewItemId(review)}`}
>
<IconGitPullRequest className={MENU_ICON_CLASS} />
{review.title}
</DropdownMenuItem>
))}
</>
);
return reviews.map((review) => (
<DropdownMenuItem
key={reviewItemId(review)}
onClick={() => addReviewPanel(review)}
className={MENU_ITEM_CLASS}
data-testid={`add-panel-review-item-${reviewItemId(review)}`}
>
<IconGitPullRequest className={MENU_ICON_CLASS} />
{review.title}
</DropdownMenuItem>
));
}
@@ -4,9 +4,9 @@ import { SessionPage } from "../../pages/session-page";
test.describe("PR detail panel — manual open", () => {
/**
* Regression: when the user removes the layout-owned PR Details panel and
* opens a pull request manually from the topbar, the keyed review tab must
* use the center fallback — not create a separate split.
* Regression: after the user removes the layout-owned PR Details panel, a
* topbar open keeps its center fallback while a group's add-panel menu uses
* that invoking group.
*
* The manual path uses `addPRPanel(prKey)` which historically anchored on
* the store's `centerGroupId`. That value can be stale across layout
@@ -17,7 +17,7 @@ test.describe("PR detail panel — manual open", () => {
* Inbox → Working (auto_start, on_turn_complete → Done) → Done
* Task A (with PR #501)
*/
test("uses the center fallback when PR Details is absent", async ({
test("distinguishes topbar fallback from add-menu split placement", async ({
testPage,
apiClient,
seedData,
@@ -133,5 +133,30 @@ test.describe("PR detail panel — manual open", () => {
expect(location.error, location.error).toBeUndefined();
expect(location.canonicalExists).toBe(false);
expect(location.keyedGroupId).toBe(location.sessionGroupId);
// Remove the keyed topbar result, then reopen the same missing review from
// the Terminal split's own + menu. The new tab must join Terminal rather
// than reuse the topbar's center fallback.
await session.prDetailTab().hover();
await session.prDetailTab().locator(".dv-default-tab-action").click();
await expect(session.prDetailTab()).toHaveCount(0);
const terminalGroup = testPage.locator(".dv-groupview").filter({
has: testPage.locator(".dv-default-tab").filter({ hasText: /^Terminal$/ }),
});
const agentGroup = testPage.locator('.dv-groupview:has([data-testid^="session-tab-"])');
await terminalGroup.getByTestId("dockview-add-panel-btn").click();
await testPage.getByTestId("add-panel-pr-item-testorg-testrepo-501").click();
const reopenedTab = testPage.locator(".dv-default-tab").filter({ hasText: /^PR #501$/ });
await expect(
terminalGroup.locator(".dv-default-tab").filter({ hasText: /^PR #501$/ }),
).toBeVisible({
timeout: 10_000,
});
await expect(
agentGroup.locator(".dv-default-tab").filter({ hasText: /^PR #501$/ }),
).toHaveCount(0);
await expect(reopenedTab).toHaveCount(1);
});
});
+19 -12
View File
@@ -153,23 +153,23 @@ async function openTaskAndWait(
return session;
}
async function selectPRFromAddPanel(
async function selectOnlyMissingPRFromAddPanel(
testPage: import("@playwright/test").Page,
session: SessionPage,
itemTestId: string,
openItemTestId: string,
) {
const item = testPage.getByTestId(itemTestId);
await expect(async () => {
if ((await session.prDetailTab().count()) >= 2) return;
// Radix submenu content can be replaced by a PR refresh between the
// Linked-review refreshes can replace Radix menu content between the
// visibility check and the click. Reopen the short-lived menu for each
// bounded attempt and force the click once the exact row is visible.
// bounded attempt and force the click once the missing row is visible.
await testPage.keyboard.press("Escape").catch(() => undefined);
await session.addPanelButton().click({ force: true });
const submenu = testPage.getByTestId("add-panel-pr-submenu");
await expect(submenu).toBeVisible({ timeout: 2_000 });
await submenu.click({ force: true });
await expect(testPage.getByTestId("add-panel-pr-submenu")).toHaveCount(0);
await expect(testPage.getByTestId(openItemTestId)).toHaveCount(0);
await expect(item).toBeVisible({ timeout: 2_000 });
await item.click({ force: true });
await expect(session.prDetailTab()).toHaveCount(2, { timeout: 2_000 });
@@ -371,12 +371,19 @@ test.describe("Multi-PR CI popover", () => {
// The layout-owned canonical panel follows the primary/first-associated PR (web#42).
await expect(session.prDetailTab()).toHaveCount(1, { timeout: 15_000 });
// Regression: with multiple linked PRs the "+" add-panel menu collapses
// the PR rows behind a "Pull requests" submenu. Selecting the OTHER PR
// from inside it must open a second, distinct tab instead of repurposing
// the layout-owned canonical one. (Dedup when re-selecting the same PR is
// covered by the runAutoPRPanelEffect / addPRPanel unit tests.)
await selectPRFromAddPanel(testPage, session, `add-panel-pr-item-${OWNER}-api-77`);
// The canonical web PR is already open, so the picker hides it and renders
// the sole missing API PR inline. Selecting that row opens a second tab.
await selectOnlyMissingPRFromAddPanel(
testPage,
session,
`add-panel-pr-item-${OWNER}-api-77`,
`add-panel-pr-item-${OWNER}-web-42`,
);
await expect(session.prDetailTab()).toHaveCount(2, { timeout: 15_000 });
// Both linked PRs now have exact panels, leaving no PR entry to add again.
await session.addPanelButton().click();
await expect(testPage.getByTestId(/^add-panel-pr-item-/)).toHaveCount(0);
await expect(testPage.getByTestId("add-panel-pr-submenu")).toHaveCount(0);
});
});
@@ -324,6 +324,20 @@ test.describe("Task layout profile defaults", () => {
prDetailsGroupId: "group-right-top",
rightGroupOrder: ["files", "changes", "pr-detail"],
});
const agentGroup = testPage.locator('.dv-groupview:has([data-testid^="session-tab-"])');
await agentGroup.getByTestId("dockview-add-panel-btn").click();
await expect(
testPage.getByTestId(`add-panel-pr-item-testorg-testrepo-${PR_NUMBER}`),
).toHaveCount(0);
await expect(testPage.getByTestId("add-panel-pr-submenu")).toHaveCount(0);
await testPage.keyboard.press("Escape");
await expect
.poll(() => dockviewSnapshot(testPage))
.toMatchObject({
prDetailsGroupId: "group-right-top",
rightGroupOrder: ["files", "changes", "pr-detail"],
});
});
test("fresh tasks use the no-terminal default while existing tasks wait for Reset Layout", async ({
@@ -82,6 +82,31 @@ describe("useWorkspaceContentSearch request lifecycle", () => {
expect(result.current.isSearching).toBe(true);
});
it("publishes the first available results while later repositories are still retrying", async () => {
const primaryResult = {
repository_name: "primary",
path: "src/primary.ts",
line: 1,
column: 1,
preview: "primary needle",
match_ranges: [],
};
mockSearchWorkspaceContent.mockResolvedValue({ results: [primaryResult] });
const { result } = renderHook(() =>
useWorkspaceContentSearch({
enabled: true,
query: "needle",
sessionId: "session-1",
}),
);
await act(async () => vi.advanceTimersByTimeAsync(250));
expect(mockSearchWorkspaceContent).toHaveBeenCalledTimes(1);
expect(result.current.results).toEqual([primaryResult]);
expect(result.current.isSearching).toBe(true);
});
it("clears results without sending when search is disabled", async () => {
mockSearchWorkspaceContent.mockResolvedValue({ results: [] });
const { result, rerender } = renderHook(
@@ -30,6 +30,7 @@ async function searchWorkspaceContentWithRetries(
sessionId: string,
query: string,
isCancelled: () => boolean,
onResults: (results: WorkspaceContentSearchResult[]) => void,
): Promise<WorkspaceContentSearchResult[]> {
const client = getWebSocketClient();
if (!client) throw new Error("websocket transport unavailable");
@@ -52,6 +53,7 @@ async function searchWorkspaceContentWithRetries(
for (const result of response.results ?? []) {
results.set(contentSearchResultKey(result), result);
}
if (!isCancelled()) onResults([...results.values()]);
} catch (error) {
lastError = error;
}
@@ -107,6 +109,12 @@ export function useWorkspaceContentSearch({
sessionId,
normalizedQuery,
() => cancelled,
(partialResults) => {
if (!cancelled) {
setResults(partialResults);
setError(null);
}
},
);
if (!cancelled) {
setResults(nextResults);
@@ -96,6 +96,7 @@ describe("addPRPanel — layout-owned group placement", () => {
const SESSION_PANEL_ID = `session:${SESSION_ID}`;
const SESSION_GROUP = "group-session-host";
const PR_DETAILS_GROUP = "group-configured-pr-details";
const REQUESTED_GROUP = "group-invoking-add-menu";
function buildExtra(api: DockviewApi) {
const store = makeStore(api);
@@ -143,6 +144,26 @@ describe("addPRPanel — layout-owned group placement", () => {
expect(pr.group.id).toBe(CENTER_GROUP);
});
it("opens in the explicitly requested group when no PR Details panel exists", () => {
const { api, actions } = buildExtra(makeApi({ extraGroupIds: [REQUESTED_GROUP] }));
actions.addPRPanel(PR_KEY, { groupId: REQUESTED_GROUP });
const pr = api.getPanel(KEYED_PR_ID) as unknown as MockPanel;
expect(pr).toBeDefined();
expect(pr.group.id).toBe(REQUESTED_GROUP);
});
it("prefers the explicitly requested group over another canonical group", () => {
const { api, actions } = buildExtra(makeApi({ extraGroupIds: [REQUESTED_GROUP] }));
seedCanonicalPRDetails(api);
actions.addPRPanel(PR_KEY, { groupId: REQUESTED_GROUP });
const pr = api.getPanel(KEYED_PR_ID) as unknown as MockPanel;
expect(pr.group.id).toBe(REQUESTED_GROUP);
});
it("focuses an existing keyed PR tab without relocating it", () => {
const { api, actions } = buildExtra(makeApi());
seedCanonicalPRDetails(api);
@@ -210,6 +231,16 @@ describe("addMRPanel — layout-owned review tabs", () => {
expect(keyed.group.id).toBe("group-configured-pr-details");
expect(api.panels.filter((panel) => panel.id.startsWith(LEGACY_MR_ID))).toHaveLength(2);
});
it("opens a new MR panel in the explicitly requested group", () => {
const requestedGroup = "group-invoking-add-menu";
const { api, actions } = buildExtra(makeApi({ extraGroupIds: [requestedGroup] }));
actions.addMRPanel(MR_KEY, { groupId: requestedGroup });
const panel = api.getPanel(`${LEGACY_MR_ID}|${MR_KEY}`) as unknown as MockPanel;
expect(panel.group.id).toBe(requestedGroup);
});
});
const PLUGIN_ID = "kandev-plugin-notes";
@@ -327,6 +358,17 @@ describe("addReviewPanel", () => {
expect(api.getPanel(panelId)).toBeUndefined();
});
it("opens a new provider-neutral review in the explicitly requested group", () => {
const requestedGroup = "group-invoking-add-menu";
const api = makeApi({ extraGroupIds: [requestedGroup] });
const actions = buildExtraPanelActions(makeStore(api).get);
actions.addReviewPanel(review, { groupId: requestedGroup });
const panel = api.getPanel(panelId) as unknown as MockPanel;
expect(panel.group.id).toBe(requestedGroup);
});
it("does not collide when provider IDs or review keys contain panel delimiters", () => {
const api = makeApi();
const actions = buildExtraPanelActions(makeStore(api).get);
+10 -9
View File
@@ -58,6 +58,7 @@ function openBrowserPanel(api: DockviewApi, centerGroupId: string, url: string):
}
type SidePanelOpts = { groupId?: string; quiet?: boolean; inCenter?: boolean };
export type ReviewPanelOptions = { groupId?: string };
/**
* Shared placement logic for a single-instance "side" panel (Plan, a plugin
@@ -531,11 +532,11 @@ export function removeSessionPanel(api: DockviewApi, sessionId: string): void {
function buildReviewPanelActions(get: StoreGet) {
return {
/**
* Focus an existing PR tab in place, or add a keyed tab beside the
* layout-owned canonical PR Details panel. Fall back to the center group
* when the current layout intentionally omits PR Details.
* Focus an existing PR tab in place, or add a keyed tab in the explicitly
* requested group. Callers without a group use the layout-owned canonical
* PR Details panel, then the center fallback.
*/
addPRPanel: (prKey?: string) => {
addPRPanel: (prKey?: string, opts?: ReviewPanelOptions) => {
const { api, centerGroupId } = get();
if (!api) return;
// Multi-repo: each TaskPR opens in its own panel keyed by
@@ -550,7 +551,7 @@ function buildReviewPanelActions(get: StoreGet) {
return;
}
if (prKey && focusMatchingLegacyPanel(api, id, "pr-detail", "prKey", prKey)) return;
const targetGroupId = api.getPanel("pr-detail")?.group.id ?? centerGroupId;
const targetGroupId = opts?.groupId ?? api.getPanel("pr-detail")?.group.id ?? centerGroupId;
focusOrAddPanel(api, {
id,
component: "pr-detail",
@@ -559,7 +560,7 @@ function buildReviewPanelActions(get: StoreGet) {
params: prKey ? { prKey } : undefined,
});
},
addMRPanel: (mrKey: string) => {
addMRPanel: (mrKey: string, opts?: ReviewPanelOptions) => {
const { api, centerGroupId } = get();
if (!api) return;
const id = `mr-detail|${mrKey}`;
@@ -578,11 +579,11 @@ function buildReviewPanelActions(get: StoreGet) {
id,
component: "mr-detail",
title: panelTitle("mr-detail"),
position: { referenceGroup: canonical?.group.id ?? centerGroupId },
position: { referenceGroup: opts?.groupId ?? canonical?.group.id ?? centerGroupId },
params: { mrKey },
});
},
addReviewPanel: (review: ReviewPanelTarget) => {
addReviewPanel: (review: ReviewPanelTarget, opts?: ReviewPanelOptions) => {
const { api, centerGroupId } = get();
if (!api) return;
const canonical = api.getPanel("pr-detail");
@@ -605,7 +606,7 @@ function buildReviewPanelActions(get: StoreGet) {
id,
component: "review-detail",
title: review.title,
position: { referenceGroup: canonical?.group.id ?? centerGroupId },
position: { referenceGroup: opts?.groupId ?? canonical?.group.id ?? centerGroupId },
params: {
providerId: review.providerId,
reviewKey: review.reviewKey,
+4 -2
View File
@@ -48,6 +48,7 @@ import {
buildExtraPanelActions,
type OpenPanelOpts,
type PreviewType,
type ReviewPanelOptions,
} from "./dockview-panel-actions";
import { preserveChatScrollDuringLayout } from "./dockview-scroll-preserve";
import { measureDockviewContainer } from "./dockview-measure";
@@ -172,9 +173,9 @@ type DockviewStore = {
closePluginPanels: (pluginId: string) => void;
addTodosPanel: (opts?: { groupId?: string; quiet?: boolean; inCenter?: boolean }) => void;
/** Open a PR detail panel. prKey (owner/repo/pr_number) gives multi-repo tasks one tab per PR. */
addPRPanel: (prKey?: string) => void;
addPRPanel: (prKey?: string, opts?: ReviewPanelOptions) => void;
/** Open a GitLab merge request detail panel keyed by host/project/iid. */
addMRPanel: (mrKey: string) => void;
addMRPanel: (mrKey: string, opts?: ReviewPanelOptions) => void;
/** Open a provider-neutral review detail panel. */
addReviewPanel: (
review: Pick<
@@ -186,6 +187,7 @@ type DockviewStore = {
| "changeRequestNumber"
| "title"
>,
opts?: ReviewPanelOptions,
) => void;
addTerminalPanel: (
terminalId?: string,
+191
View File
@@ -0,0 +1,191 @@
---
spec: docs/specs/ui/task-layout-profiles.md
created: 2026-08-14
status: completed
---
# Fix Plan: Dockview Review Add Menu
## Overview
Make each task Dockview `+` menu list only reviews whose panel is missing from
the live layout. A matching canonical or keyed review tab in any split removes
that review from every add-panel menu; it stays where the user placed it. This
matches the Layout editor's existing missing-panel behavior and preserves the
current contract that explicit review opens focus existing tabs in place rather
than relocating them. When a review is missing, selecting it from a group's `+`
menu creates it in that invoking group.
The durable behavior is recorded in
[`task-layout-profiles.md`](../../specs/ui/task-layout-profiles.md), with the
GitHub list and submenu details amended in
[`add-panel-pr-submenu.md`](../../specs/ui/add-panel-pr-submenu.md).
## Confirmed Root Cause
`AddPanelMenuItems` renders every linked PR, MR, and registered-provider review
without consulting the live Dockview panels. Its state only tracks whether the
single-instance Files and Changes panels exist. In contrast,
`addPRPanel`, `addMRPanel`, and `addReviewPanel` deliberately deduplicate a
matching canonical or keyed review and focus it in place. The menu therefore
offers an add action whose click cannot add to the invoking group or move the
existing tab.
The mismatch became user-visible when `cc6eb4dd5` restored automatic canonical
PR Details insertion for linked reviews but did not update the older add-panel
rows. A throwaway Vitest regression placed canonical `pr-detail` for
`acme/kandev/42` in `group-right`, rendered another group's menu, and expected
`add-panel-pr-item-acme-kandev-42` to be absent. Current code failed exactly at
that assertion: the menu item remained present. The throwaway test was removed;
the worktree was clean before these design artifacts were written.
User retesting confirmed a second placement defect after the missing-only menu
repair: `AddPanelMenuItems` receives the invoking `groupId`, but its GitHub,
GitLab, and registered-provider review callbacks discard it. The corresponding
store actions accept no placement option and fall back to `centerGroupId` when
no canonical `pr-detail` panel exists. The focused RED regression requested
`group-invoking-add-menu` and observed `group-center`.
## Frontend
### Live review-panel availability
- Update `apps/web/components/task/dockview-add-panel-items.tsx` with one pure
exact-identity predicate over the live `DockviewApi`.
- For GitHub, recognize both canonical `pr-detail` parameters for the same
`prKey` and keyed `pr-detail|<prKey>` panels.
- For GitLab, recognize canonical `pr-detail` or legacy `mr-detail` parameters
for the same `mrKey` and keyed `mr-detail|<mrKey>` panels.
- For registered providers, compare the canonical provider-neutral identity
(`providerId`, `connectionScope`, `repositoryId`, and
`changeRequestNumber`) and the existing `reviewPanelId(review)` keyed panel.
- A mixed-provider canonical selector without one exact review identity does
not hide any individual review. Only an exact matching review panel counts as
open.
- Read the current store API when the dropdown content mounts. Do not add
persistence or cached availability state; closing a panel must make its row
available the next time the menu opens.
### Missing-only menu rows
- Filter `state.prs`, `state.mrs`, and registered-provider reviews through the
predicate before rendering their menu rows.
- Feed only missing GitHub PRs into `PRPanelMenuItems`, so zero missing PRs
render nothing, one renders inline, and two or more use the existing submenu.
- Keep labels, icons, stable test IDs, ordering, and the existing add actions
unchanged for reviews that remain available.
- Forward the invoking `groupId` for each available GitHub, GitLab, and
registered-provider row. Extend the three review actions with an optional
placement option so an explicit group wins for newly created tabs, while
callers that omit it retain canonical/center fallback behavior.
- For an exact open match, omit the row instead of invoking its existing
focus-in-place action; do not add relocation logic. Topbar and other
explicit-open surfaces retain their current focus-in-place behavior.
No backend, API, persistence, feature-flag, localization, or layout-editor code
changes are required.
## Public Documentation
- Update the PR Details paragraph in
`docs/public/sessions-and-review.md` to state that a group's `+` menu lists a
linked review only while its exact panel is missing, and that moving an open
review between splits uses Dockview's tab and layout controls.
- Keep the page's primary how-to purpose and existing terminology. No new page,
navigation entry, screenshot, or media change is required.
## Tests
- **What:** an exact canonical PR in another group and an exact keyed PR are
omitted, while a different linked PR remains available.
**File:** `apps/web/components/task/dockview-add-panel-items.test.tsx`.
**How:** first add the failing render/helper cases against unchanged
production code, then implement the predicate and rerun them.
- **What:** filtering occurs before the inline/submenu threshold.
**File:** the same Vitest file.
**How:** cover two linked PRs with one open (one inline row, no submenu) and
three linked PRs with one open (submenu containing only the two missing
rows).
- **What:** GitLab and registered-provider exact matches use the same rule, and
a mixed canonical selector does not suppress individual rows.
**File:** the same Vitest file.
**How:** exercise the exported pure identity predicate with small Dockview API
fakes and assert matching versus non-matching identities.
- **What:** an add-menu request creates a missing review in the invoking group,
while existing focus-in-place and non-menu fallback behavior remain
unchanged.
**File:** `apps/web/lib/state/dockview-panel-actions-extra.test.ts`.
**How:** first assert the explicit-group failure, then cover GitHub, GitLab,
and registered-provider placement plus the existing fallback guards.
## E2E Tests
- **Scenario:** **GIVEN** the Layout editor places canonical PR Details in the
Files/Changes split and a fresh task links that PR, **WHEN** the user opens
the Agent split's `+` menu, **THEN** that PR row is absent and the canonical
panel remains in the configured split.
**File:** `apps/web/e2e/tests/settings/layout-profiles.spec.ts`.
- **Scenario:** **GIVEN** two linked PRs with the primary PR already represented
by canonical PR Details, **WHEN** the user opens the `+` menu, **THEN** the
primary is absent, the secondary is the only inline PR row, and selecting it
opens one keyed tab. Reopening the menu after both are open shows no PR rows.
**File:** `apps/web/e2e/tests/pr/pr-multi-popover.spec.ts`.
Both scenarios assert user-visible menu state and live Dockview placement. API
calls remain setup only.
## Mobile Design Contract
- Desktop entry: the per-group Dockview `+` menu and empty-group watermark.
- Phone/tablet task entry: unchanged. `SessionMobileLayout` uses its existing
review destination and does not render Dockview's add-panel menu.
- Nearest shipped mobile behavior: the dedicated mobile task review surface in
`apps/web/components/task/task-layout.tsx`; it already exposes linked review
content without split-group placement.
- Mobile settings behavior: unchanged. The responsive Layout editor already
lists only globally missing reusable panels and remains the touch path for
configuring future PR Details placement.
- No mobile Playwright case is added because the changed menu has no mobile
task-layout entry point and no composition, scrolling, navigation, safe-area,
or touch behavior changes.
## Verification Results
- RED: the focused add-panel unit file ran 18 tests with the five new hiding
assertions failing as expected and 13 existing/guard assertions passing.
- GREEN: the final add-panel and panel-action regression suites passed 41 tests
across 2 files.
- Targeted ESLint and frontend TypeScript checks passed. `i18n:check` passed
with only the repository's advisory locale-parity report, and the new-code
i18n ratchet reported 0 added plus 3 modified production files clean.
- Playwright listed exactly the two intended Chromium scenarios. The first
production-build run passed the multi-PR case and exposed an unstable Agent
text locator in the layout case; after switching to the existing session-tab
group selector, a fresh disposable-backend rerun against the unchanged build
passed both scenarios in 22.0 seconds.
- The placement repair's final production build passed its focused Chromium
scenario: topbar opening retained the center fallback, then the Terminal
split's `+` menu opened the missing review in Terminal (1 test, 12.3 seconds).
- Public-doc validation passed 61 tests and validated 41 published pages.
`git diff --check` passed.
## Implementation Task
- [x] [Task 01: Filter open review menu entries](task-01-filter-open-review-menu-entries.md)
- [x] [Task 02: Place reviews in invoking group](task-02-place-reviews-in-invoking-group.md)
Execution is sequential in the primary conversation. This single task is not a
parallel candidate and does not authorize subagent use.
## Risks and Boundaries
- Canonical panels have legacy built-in keys and newer provider-neutral params;
matching must accept both without treating a mixed-review selector as one
exact review.
- Filter before computing the PR submenu threshold or a single remaining PR
will stay hidden behind a misleading multi-PR trigger.
- Read live Dockview state when the menu opens; a cached list could stay stale
after close, restore, or environment switch.
- Review linking, ordering, topbar behavior, relocation of existing panels,
saved layouts, mobile review navigation, and non-review add-panel rows remain
out of scope.
@@ -0,0 +1,145 @@
---
id: "01-filter-open-review-menu-entries"
title: "Filter open review menu entries"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/ui/task-layout-profiles.md"
---
# Task 01: Filter Open Review Menu Entries
## Acceptance
- Every task Dockview `+` menu and empty-group watermark omits a GitHub,
GitLab, or registered-provider review that already has an exact canonical or
keyed panel anywhere in the live layout.
- Other linked reviews remain available. GitHub chooses no row, one inline row,
or a submenu from the filtered count, not the task's total linked PR count.
- Closing an exact review panel makes its row available on the next menu open;
a mixed-review canonical selector does not hide individual reviews.
- Existing review actions still focus matching tabs in place outside the add
menu and retain current placement for newly opened keyed tabs.
- Desktop E2E proves both layout-editor placement in another split and the
multi-PR filtered-menu flow.
- Public session/review guidance explains the missing-only `+` menu and the
explicit path for moving an existing review tab.
## TDD Sequence
1. Add the focused unit cases for canonical/keyed identities, filtered PR
counts, provider parity, and mixed-review behavior.
2. Run the unit file against unchanged production code and record the expected
failures.
3. Add the minimal live-API predicate and filter the three review menu paths.
4. Rerun the unit suite and existing review panel-action regression suite.
5. Update the two existing Chromium E2E scenarios, run them against a fresh
production build, and record exact results.
## Verification
Bootstrap only if this worktree's dependencies are absent:
```bash
cd apps && pnpm install --frozen-lockfile
```
From the repository root, run:
```bash
cd apps && pnpm --filter @kandev/web exec vitest run components/task/dockview-add-panel-items.test.tsx lib/state/dockview-panel-actions-extra.test.ts
cd apps/web && pnpm exec eslint components/task/dockview-add-panel-items.tsx components/task/dockview-add-panel-items.test.tsx && pnpm run typecheck
cd apps/web && pnpm e2e:run tests/settings/layout-profiles.spec.ts tests/pr/pr-multi-popover.spec.ts -- --grep 'moves PR Details|selecting a different PR'
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
git diff --check
```
Confirm Playwright discovers both intended Chromium tests before treating the
focused E2E command as evidence.
## Files Likely Touched
- `apps/web/components/task/dockview-add-panel-items.tsx`
- `apps/web/components/task/dockview-add-panel-items.test.tsx`
- `apps/web/e2e/tests/settings/layout-profiles.spec.ts`
- `apps/web/e2e/tests/pr/pr-multi-popover.spec.ts`
- `docs/public/sessions-and-review.md`
- `docs/specs/ui/task-layout-profiles.md`
- `docs/specs/ui/add-panel-pr-submenu.md`
- `docs/plans/dockview-review-add-menu/plan.md`
- `docs/plans/dockview-review-add-menu/task-01-filter-open-review-menu-entries.md`
## Dependencies
None.
## Parallelism
`sequential`. Unit behavior, production filtering, and E2E expectations share
one RED-GREEN cycle and overlapping files.
## Inputs
- `docs/specs/ui/task-layout-profiles.md`, especially review placement and the
missing-only add-menu scenario.
- `docs/specs/ui/add-panel-pr-submenu.md`, especially filtered PR count and
inline/submenu scenarios.
- `apps/web/components/task/dockview-add-panel-items.tsx` for all three review
menu paths.
- `apps/web/lib/state/dockview-panel-actions.ts` and
`apps/web/lib/state/dockview-review-panel-id.ts` for canonical/keyed identity
and focus-in-place behavior.
- `apps/web/components/task/dockview-review-panel-sync.ts` for canonical
provider-neutral parameters and mixed-review state.
- Existing E2E setup in `layout-profiles.spec.ts` and
`pr-multi-popover.spec.ts`.
## Output Contract
Report the RED failures, GREEN command results and test counts, files changed,
any identity compatibility risk, and cleanup evidence. Mark this task `done`,
replace `## Results` below, check the task in `plan.md`, and synchronize the
plan's `## Verification Results` in the same primary conversation.
## Results
RED results:
- `dockview-add-panel-items.test.tsx` ran 18 tests: the five new canonical,
keyed, filtered-count, GitLab, and registered-provider assertions failed as
expected; 13 existing and mixed-selector guard assertions passed.
GREEN results:
- `pnpm --filter @kandev/web exec vitest run
components/task/dockview-add-panel-items.test.tsx
lib/state/dockview-panel-actions-extra.test.ts` passed 35 tests across 2
files.
- Targeted ESLint completed without diagnostics and `pnpm run typecheck`
passed.
- `pnpm exec playwright test --list ... --grep ...` discovered exactly the 2
intended Chromium tests. The managed production-build run passed the
multi-PR scenario; its layout scenario timed out on a new Agent-text test
locator before reaching a product assertion. Replacing that locator with
the repository's stable session-tab group selector and rerunning both tests
in a fresh disposable backend against the unchanged production build passed
2 tests in 22.0 seconds.
- `pnpm run i18n:check` passed with advisory pre-existing locale parity output;
`pnpm run i18n:ratchet` reported 0 added plus 1 modified production file
clean.
- Public-doc validation passed 61 tests and validated all 41 published pages.
- `git diff --check` passed.
The live-menu predicate recognizes exact GitHub and GitLab legacy/current keys,
registered-provider canonical identities, and keyed provider panels. A
provider-neutral canonical selector remains non-matching, and remounting the
menu after a panel closes restores the row. Existing topbar review actions and
panel placement code were not changed.
Public docs updated: `docs/public/sessions-and-review.md` remains a how-to guide
and now explains that `+` lists only missing review panels and that an open tab
must be dragged to move it between splits. No mobile E2E was added because the
phone/tablet task composition does not render Dockview's add-panel menu; its
dedicated Review navigation is unchanged.
@@ -0,0 +1,75 @@
---
id: "02-place-reviews-in-invoking-group"
title: "Place reviews in invoking group"
status: done
wave: 2
depends_on: ["01-filter-open-review-menu-entries"]
plan: "plan.md"
spec: "../../specs/ui/task-layout-profiles.md"
---
# Task 02: Place Reviews in Invoking Group
## Confirmed Root Cause
`AddPanelMenuItems` knows which Dockview group opened the menu, but the review
callbacks call `addPRPanel`, `addMRPanel`, and `addReviewPanel` without that
group. Those actions therefore use the canonical PR Details group or
`centerGroupId`; with no canonical tab, a click from another split lands in the
default center group.
## Acceptance
- A missing GitHub, GitLab, or registered-provider review selected from a
group's add-panel menu opens in that group.
- A matching open review remains omitted and is never relocated.
- Topbar and host callers that do not request a group retain canonical/center
fallback behavior.
- Action-level placement and component callback wiring have regression tests.
- The isolated Tailscale test environment demonstrates the non-default split
flow without changing the main instance on `:9998`.
## TDD Sequence
1. Add an action regression requesting a non-default group and confirm it fails
with the panel in `group-center`.
2. Add menu callback assertions for the invoking group and provider parity.
3. Extend review actions with optional placement and forward `groupId` from the
menu callbacks.
4. Run focused unit suites, then the targeted Chromium Dockview scenario.
5. Run frontend static/i18n checks, docs validation, and diff hygiene.
## Verification
```bash
cd apps/web && pnpm exec vitest run components/task/dockview-add-panel-items.test.tsx lib/state/dockview-panel-actions-extra.test.ts
cd apps/web && pnpm exec eslint components/task/dockview-add-panel-items.tsx components/task/dockview-add-panel-items.test.tsx lib/state/dockview-panel-actions.ts lib/state/dockview-panel-actions-extra.test.ts lib/state/dockview-store.ts
cd apps/web && pnpm run typecheck
cd apps/web && pnpm run i18n:check && pnpm run i18n:ratchet
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
git diff --check
```
## Parallelism
`sequential`. The action contract, menu callbacks, tests, and behavioral specs
are one coupled RED-GREEN repair.
## Results
RED confirmed: the action regression requested
`group-invoking-add-menu` but the new panel was created in `group-center`.
GREEN confirmed:
- Review actions now accept optional placement, and the add-panel menu forwards
its invoking `groupId` for GitHub, GitLab, and registered providers.
- The focused component and action suites passed 41 tests across 2 files.
- Targeted ESLint, frontend TypeScript, i18n checks, and diff hygiene passed.
- A final production build passed the focused Chromium scenario (1 test in
12.3 seconds), covering both unchanged topbar fallback and split-menu
placement.
- Interactive testing in the isolated Tailscale environment opened PR #842 in
`group-right-bottom` after using that split's `+` menu. The main instance on
`:9998` remained untouched.
+2
View File
@@ -119,6 +119,8 @@ Open **Settings > General > Layouts** to configure reusable desktop workbench pr
**PR Details** is a reusable Layouts panel whose visibility follows the active task's review association. Without a linked GitHub pull request or GitLab merge request, the tab stays hidden, even when the selected layout includes it. Once a review is linked, Kandev adds PR Details as an inactive tab: beside **Agent** for the built-in Default, or in the group and tab position you configured in the Layouts editor. Closing that tab prevents it from reappearing automatically in the same session. Changing the default applies to task environments without a saved task-specific layout and **Reset Layout**, not a layout already saved for a task. Removing Terminal from the Default layout also prevents Kandev from creating its initial user shell.
Each desktop split's **+** menu lists a linked review only while that exact review panel is missing from the live layout. Select a missing review there to open it in the split whose **+** you used. To move an open review to another split, drag its Dockview tab. Use **Layouts** only to change the default placement for new or reset task layouts.
All panels for a task point at the same task environment. In a multi-repository task, check the repository label before editing, committing, or reviewing. A preview also requires the application to listen on a reachable interface and expose a forwarded port.
Structured shell-command activity keeps the command, working directory, status, and output size in the chat row. Expand **Output** to fetch the transcript; Kandev continues refreshing an open, running command and stops when it reaches a terminal state. The disclosure separates standard output and errors, reports truncation and the exit code when known, and offers **Retry** when the transcript request fails. Historical command transcripts are loaded only when opened, which keeps long conversations responsive without discarding the stored output.
+51 -18
View File
@@ -16,13 +16,20 @@ viewport or requires scrolling through a wall of identical rows.
## What
- The task-view "+" add-panel menu (dockview group header and empty-group
watermark) keeps its current behavior for tasks with zero or one linked
GitHub PR: a single PR renders as one inline menu row, and no PR renders
nothing.
- When a task has more than one linked GitHub PR, the menu shows a single
watermark) lists only linked GitHub PRs that do not already have a matching
canonical or keyed review panel anywhere in the current Dockview layout.
- A matching open panel is identified by the PR's stable task key, regardless
of which split group owns it. The add-panel menu never offers that PR as a
way to focus or relocate the existing tab; users move existing tabs through
Dockview's tab and split controls.
- The menu keeps its current behavior when zero or one linked PR remains after
filtering: one available PR renders as one inline menu row, and no available
PR renders nothing.
- When more than one linked PR remains after filtering, the menu shows a single
**Pull requests** sub-menu trigger instead of inline PR rows.
- Opening the sub-menu reveals one row per linked PR. Each row opens that PR's
dockview panel when selected, exactly as the current inline row does.
- Opening the sub-menu reveals one row per available linked PR. Each row opens
that PR's Dockview panel in the split whose add-panel menu was used. The
single inline PR row uses the same invoking-split placement.
- Per-PR rows inside the sub-menu keep the current disambiguating label used for
multi-PR tasks (`PR #42 — owner/repo`), while single-PR tasks keep the plain
`PR #42` label on the inline row.
@@ -30,6 +37,11 @@ viewport or requires scrolling through a wall of identical rows.
an icon consistent with the PR rows.
- Per-PR rows keep their existing stable test identifiers so automated tests can
select them from within the sub-menu.
- Closing a matching canonical or keyed review panel makes that PR available in
the add-panel menu again. Other linked PRs remain independently available or
hidden according to their own open-panel state.
- GitLab and registered-provider review rows use the same already-open filter,
invoking-split placement, and existing inline presentation and labels.
- The sub-menu must work with the existing Radix dropdown primitives on desktop
(pointer hover and keyboard). The feature is desktop-only: mobile renders
`SessionMobileLayout`, which has no "+" add-panel entry point, so the sub-menu
@@ -42,15 +54,35 @@ viewport or requires scrolling through a wall of identical rows.
- **GIVEN** a task with exactly one linked GitHub PR, **WHEN** the user opens
the "+" add-panel menu, **THEN** one inline PR row labeled `PR #N` is shown
and no Pull requests sub-menu trigger is shown.
- **GIVEN** a task with two linked GitHub PRs, **WHEN** the user opens the "+"
add-panel menu, **THEN** the menu shows a Pull requests sub-menu trigger and
no inline PR rows.
- **GIVEN** a task with two linked GitHub PRs and the Pull requests sub-menu
open, **WHEN** the user selects a PR row, **THEN** that PR's dockview panel
opens and the row is labeled `PR #N — owner/repo`.
- **GIVEN** a task with three linked GitHub PRs, **WHEN** the user opens the "+"
add-panel menu, **THEN** the main menu height is bounded by the single Pull
requests trigger instead of three PR rows.
- **GIVEN** a task whose only linked GitHub PR is already rendered by the
canonical PR Details panel in another split, **WHEN** the user opens the "+"
add-panel menu in any group, **THEN** no row or sub-menu trigger for that PR
is shown and the existing panel is neither focused nor moved.
- **GIVEN** a task with two linked GitHub PRs whose primary PR is already open,
**WHEN** the user opens the "+" add-panel menu, **THEN** only the other PR is
shown as an inline row and no Pull requests sub-menu trigger is shown.
- **GIVEN** a task with three linked GitHub PRs and one matching panel already
open, **WHEN** the user opens the "+" add-panel menu, **THEN** the Pull
requests sub-menu contains only the two missing PRs.
- **GIVEN** a linked PR was omitted because its matching panel was open,
**WHEN** the user closes that panel and reopens the "+" add-panel menu,
**THEN** the PR is offered again.
- **GIVEN** a GitLab or registered-provider review already has a matching panel
anywhere in Dockview, **WHEN** the user opens another group's add-panel menu,
**THEN** that review row is omitted under the same identity rule.
- **GIVEN** a task with two linked GitHub PRs and neither is already open,
**WHEN** the user opens the "+" add-panel menu, **THEN** the menu shows a Pull
requests sub-menu trigger and no inline PR rows.
- **GIVEN** a task with two available linked GitHub PRs and the Pull requests
sub-menu open, **WHEN** the user selects a PR row, **THEN** that PR's dockview
panel opens in the split whose add-panel menu was used and the row is labeled
`PR #N — owner/repo`.
- **GIVEN** no matching PR Details tab is open, **WHEN** the user selects a
linked GitHub PR from a non-default split's add-panel menu, **THEN** the new
review tab opens in that non-default split rather than the default group.
- **GIVEN** a task with three available linked GitHub PRs, **WHEN** the user
opens the "+" add-panel menu, **THEN** the main menu height is bounded by the
single Pull requests trigger instead of three PR rows.
## Out of scope
@@ -58,10 +90,11 @@ viewport or requires scrolling through a wall of identical rows.
- Grouping GitLab merge requests into a sub-menu; MR rows keep their current
inline rendering.
- Changing the PR top-bar button, the multi-PR CI popover, or the PR picker
dialog.
dialog. Those surfaces may continue to focus an already-open review in place.
- Mobile presentation of the sub-menu: `SessionMobileLayout` has no "+"
add-panel entry point, so there is no mobile sub-menu to style.
## Implementation plan
## Implementation plans
[Task Add-Panel PR Submenu implementation plan](../../plans/add-panel-pr-submenu/plan.md)
- [Task Add-Panel PR Submenu implementation plan](../../plans/add-panel-pr-submenu/plan.md)
- [Dockview Review Add Menu repair plan](../../plans/dockview-review-add-menu/plan.md)
+5 -2
View File
@@ -23,7 +23,8 @@ Users can arrange and save the desktop task workbench only while a task is open,
- After review data hydrates, Kandev removes the canonical PR Details panel whenever the active task has no linked review, including panels materialized from a selected profile or restored task layout. The saved layout data remains unchanged so it can continue to provide placement when a review is linked.
- Closing a conditionally added PR Details panel suppresses automatic re-creation for that session. The user can still reopen a specific review explicitly or configure future linked-review placement through the layout editor.
- Runtime review-data synchronization updates the canonical panel's provider and review identity without moving an existing panel. Explicit layout placement therefore remains user-owned even while review content changes.
- Explicitly opening a specific pull request or merge request focuses a matching tab in place. A new task-specific review tab joins the canonical PR Details panel's configured group when that panel exists and otherwise falls back to the existing center group; opening never relocates an already-open tab.
- Explicitly opening a specific pull request or merge request focuses a matching tab in place. A new task-specific review tab requested from a split's add-panel menu opens in that invoking split. Callers without an explicit split join the canonical PR Details panel's configured group when that panel exists and otherwise fall back to the existing center group; opening never relocates an already-open tab.
- Each task Dockview add-panel menu offers only linked reviews without a matching canonical or keyed panel anywhere in the live layout. An already-open review is omitted from every split's add menu; moving that tab remains an explicit Dockview layout action rather than a side effect of adding it elsewhere.
- Selecting a tab makes it active and shows contextual controls next to its split. Users can reorder or remove the tab, move it between groups, create splits, and move, merge, or resize the selected split. Adding a missing panel remains a separate floating action. Every editor action provides a hover/focus description.
- Layout changes use the shared Settings floating save control and navigation guard. The page does not render its own Save or Cancel buttons.
- Removing Terminal from the effective default prevents the default terminal panel and its backing user shell from being created when a fresh task environment is first opened.
@@ -106,7 +107,9 @@ The frontend treats the returned settings payload as authoritative after each su
- **GIVEN** Kandev conditionally added PR Details for a linked review, **WHEN** the user closes that tab and review data resynchronizes in the same session, **THEN** Kandev does not recreate the tab automatically.
- **GIVEN** Kandev showed PR Details for one task, **WHEN** the active task changes to one without a linked review, **THEN** no canonical PR Details panel remains visible even when the incoming layout configures its future placement.
- **GIVEN** the canonical PR Details panel is configured in a non-Agent group, **WHEN** the user explicitly opens a second pull request or merge request, **THEN** the new keyed review tab joins that group; reopening an existing keyed tab focuses it without moving it.
- **GIVEN** a layout without the canonical PR Details panel, **WHEN** the user explicitly opens a specific pull request or merge request, **THEN** the keyed review tab opens in the center fallback group without creating a new split.
- **GIVEN** a linked review already has a canonical or keyed panel in one split, **WHEN** the user opens another split's add-panel menu, **THEN** that review is absent from the menu and its existing panel remains in place.
- **GIVEN** a layout without the canonical PR Details panel, **WHEN** the user selects a specific pull request or merge request from a split's add-panel menu, **THEN** the keyed review tab opens in that invoking split without creating another split.
- **GIVEN** a caller without an explicit split opens a specific pull request or merge request and no canonical PR Details panel exists, **THEN** the keyed review tab opens in the center fallback group.
- **GIVEN** a default profile without Terminal and a task environment with no saved layout, **WHEN** the user first opens that task, **THEN** the workbench has no Terminal tab and no default user shell is created.
- **GIVEN** an existing task with a task-specific layout and no pending inactive-task changes, **WHEN** the user changes the default profile and returns to that task, **THEN** the task-specific layout and saved active panel are unchanged.
- **GIVEN** an existing desktop task with a saved active panel and a Changes panel outside the Agent group, **WHEN** a meaningful Git or commit update is detected while the task is inactive and the user returns to it, **THEN** the task-specific geometry is restored and Changes becomes the active panel after restoration completes.