From 11b8dae74512bea4a983209f405526e81d8c15dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sun, 2 Aug 2026 20:07:13 +0100 Subject: [PATCH] fix: restore task tab focus (#2176) --- .../web/e2e/tests/pr/pr-detail-layout.spec.ts | 64 +++ .../dockview-env-switch-active-views.test.ts | 385 ++++++++++++++++++ .../state/dockview-env-switch-active-views.ts | 159 ++++++++ .../web/lib/state/dockview-env-switch.test.ts | 67 --- apps/web/lib/state/dockview-env-switch.ts | 70 +--- .../dockview-active-tab-restoration/plan.md | 198 +++++++++ .../task-01-restore-semantic-active-views.md | 133 ++++++ docs/specs/ui/task-layout-profiles.md | 6 + 8 files changed, 960 insertions(+), 122 deletions(-) create mode 100644 apps/web/lib/state/dockview-env-switch-active-views.test.ts create mode 100644 apps/web/lib/state/dockview-env-switch-active-views.ts create mode 100644 docs/plans/dockview-active-tab-restoration/plan.md create mode 100644 docs/plans/dockview-active-tab-restoration/task-01-restore-semantic-active-views.md diff --git a/apps/web/e2e/tests/pr/pr-detail-layout.spec.ts b/apps/web/e2e/tests/pr/pr-detail-layout.spec.ts index 734426a2b0..a671da193a 100644 --- a/apps/web/e2e/tests/pr/pr-detail-layout.spec.ts +++ b/apps/web/e2e/tests/pr/pr-detail-layout.spec.ts @@ -104,6 +104,12 @@ async function linkPR(apiClient: ApiClient, taskId: string): Promise { }); } +function sessionTabWrapper(page: Page, sessionId: string) { + return page.locator(".dv-tab", { + has: page.getByTestId(`session-tab-${sessionId}`), + }); +} + test.describe("PR Details layout panel", () => { test("keeps the Default panel beside Agent and syncs linked review content", async ({ testPage, @@ -169,4 +175,62 @@ test.describe("PR Details layout panel", () => { keyedPanelIds: [], }); }); + + test("restores each task's selected center tab after a PR Details round trip", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await seedMockPR(apiClient); + const taskA = await createTaskWithSession(apiClient, seedData, "Active Agent PR round trip A"); + const taskB = await createTaskWithSession(apiClient, seedData, "Active Agent PR round trip B"); + await linkPR(apiClient, taskA.id); + + const session = await openTask(testPage, taskA.id); + const agentTab = sessionTabWrapper(testPage, taskA.session_id!); + await expect(agentTab).toHaveClass(/dv-active-tab/); + + // Make Agent the last selected center tab after explicitly visiting PR Details. + await session.prDetailTab().click(); + await expect(session.prDetailPanel()).toBeVisible(); + await session.clickSessionChatTab(); + await expect(agentTab).toHaveClass(/dv-active-tab/); + + await session.clickTaskInSidebar("Active Agent PR round trip B"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskB.id), { + timeout: 15_000, + }); + await session.waitForDockviewReady(); + + await session.clickTaskInSidebar("Active Agent PR round trip A"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), { + timeout: 15_000, + }); + await session.waitForDockviewReady(); + await expect(agentTab).toHaveClass(/dv-active-tab/, { timeout: 15_000 }); + await expect(testPage.getByTestId(`session-tab-${taskA.session_id}`)).toHaveCount(1); + + // A deliberate PR Details selection remains deliberate across the same round trip. + await session.prDetailTab().click(); + const reviewTab = testPage.locator(".dv-tab", { has: session.prDetailTab() }); + await expect(reviewTab).toHaveClass(/dv-active-tab/); + await session.clickTaskInSidebar("Active Agent PR round trip B"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskB.id), { + timeout: 15_000, + }); + await session.waitForDockviewReady(); + await session.clickTaskInSidebar("Active Agent PR round trip A"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), { + timeout: 15_000, + }); + await session.waitForDockviewReady(); + await expect(testPage.locator(".dv-tab", { has: session.prDetailTab() })).toHaveClass( + /dv-active-tab/, + { + timeout: 15_000, + }, + ); + await expect(agentTab).not.toHaveClass(/dv-active-tab/); + }); }); diff --git a/apps/web/lib/state/dockview-env-switch-active-views.test.ts b/apps/web/lib/state/dockview-env-switch-active-views.test.ts new file mode 100644 index 0000000000..bed2a07cee --- /dev/null +++ b/apps/web/lib/state/dockview-env-switch-active-views.test.ts @@ -0,0 +1,385 @@ +import { beforeEach, expect, it, vi } from "vitest"; +import { performEnvSwitch, type EnvSwitchParams } from "./dockview-env-switch"; + +const { CANONICAL_CENTER_GROUP_ID, RIGHT_TOP_GROUP_ID, RIGHT_BOTTOM_GROUP_ID } = vi.hoisted(() => ({ + CANONICAL_CENTER_GROUP_ID: "group-center", + RIGHT_TOP_GROUP_ID: "group-right-top", + RIGHT_BOTTOM_GROUP_ID: "group-right-bottom", +})); + +vi.mock("@/lib/local-storage", () => ({ + getEnvLayout: vi.fn(() => null), + getManualRightWidth: vi.fn(() => null), +})); + +vi.mock("./dockview-layout-builders", () => ({ + applyLayoutFixups: vi.fn(() => ({ + sidebarGroupId: "g1", + centerGroupId: "g2", + rightTopGroupId: "g3", + rightBottomGroupId: "g4", + })), +})); + +vi.mock("./layout-manager", () => ({ + fromDockviewApi: vi.fn(() => ({ columns: [] })), + savedLayoutMatchesLive: vi.fn(() => false), + layoutStructuresMatch: vi.fn(() => false), + getRootSplitview: vi.fn(() => null), + getPinnedWidth: vi.fn(() => 350), + isCenterCandidateGroupId: vi.fn( + (groupId: string) => + groupId !== "group-sidebar" && + groupId !== RIGHT_TOP_GROUP_ID && + groupId !== RIGHT_BOTTOM_GROUP_ID, + ), + setPinnedTarget: vi.fn(), + CENTER_GROUP: CANONICAL_CENTER_GROUP_ID, + RIGHT_TOP_GROUP: RIGHT_TOP_GROUP_ID, + RIGHT_BOTTOM_GROUP: RIGHT_BOTTOM_GROUP_ID, +})); + +import { getEnvLayout } from "@/lib/local-storage"; +import { fromDockviewApi, layoutStructuresMatch, savedLayoutMatchesLive } from "./layout-manager"; + +const NEW_SESSION_ID = "new-session"; +const OLD_SESSION_PANEL_ID = "session:old-session"; +const NEW_SESSION_PANEL_ID = `session:${NEW_SESSION_ID}`; +const PR_DETAILS_ID = "pr-detail"; +const PR_DETAILS_TITLE = "PR Details"; + +type TestPanel = { + id: string; + api: { component?: string; setActive?: ReturnType; close?: () => void }; + group?: TestGroup; +}; + +type TestGroup = { id: string; panels: TestPanel[] }; + +function makeMockApi() { + return { + panels: [], + groups: [], + layout: vi.fn(), + fromJSON: vi.fn(), + getPanel: vi.fn(() => null), + addPanel: vi.fn(), + } as unknown as EnvSwitchParams["api"]; +} + +function makeParams(overrides?: Partial): EnvSwitchParams { + return { + api: makeMockApi(), + oldEnvId: "old-env", + newEnvId: "new-env", + activeSessionId: NEW_SESSION_ID, + safeWidth: 800, + safeHeight: 600, + buildDefault: vi.fn(), + getDefaultLayout: vi.fn(() => ({ columns: [] })), + ...overrides, + }; +} + +function makeTwoLeafSavedLayout( + leaves: Array<{ id: string; views: string[]; activeView: string }>, + activeGroup: string, +): ReturnType { + return { + grid: { + root: { + type: "branch" as const, + data: leaves.map((leaf) => ({ type: "leaf", data: leaf })), + }, + height: 600, + width: 800, + orientation: "HORIZONTAL" as const, + }, + panels: { chat: { contentComponent: "chat" } }, + activeGroup, + } as unknown as ReturnType; +} + +function makeAgentReviewApi( + setActiveAgent: ReturnType, + setActiveReview: ReturnType, + groupId = "center", +) { + const group: TestGroup = { id: groupId, panels: [] }; + group.panels = [ + { id: NEW_SESSION_PANEL_ID, api: { component: "chat", setActive: setActiveAgent }, group }, + { id: PR_DETAILS_ID, api: { component: PR_DETAILS_ID, setActive: setActiveReview }, group }, + ]; + return { + group, + api: { + ...makeMockApi(), + panels: group.panels, + groups: [group], + getPanel: vi.fn((id: string) => group.panels.find((panel) => panel.id === id) ?? null), + } as unknown as EnvSwitchParams["api"], + }; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +it("rebinds a saved Chat selection to the incoming Agent panel", () => { + const setActiveAgent = vi.fn(); + const setActiveReview = vi.fn(); + const savedLayout = makeTwoLeafSavedLayout( + [{ id: "center", views: ["chat", PR_DETAILS_ID], activeView: "chat" }], + "center", + ); + const { api } = makeAgentReviewApi(setActiveAgent, setActiveReview); + vi.mocked(getEnvLayout).mockReturnValueOnce(savedLayout); + vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(true); + + performEnvSwitch(makeParams({ api })); + + expect(setActiveAgent).toHaveBeenCalledOnce(); + expect(setActiveReview).not.toHaveBeenCalled(); +}); + +it("rebinds a stale saved session selection to the incoming Agent panel", () => { + const setActiveAgent = vi.fn(); + const setActiveReview = vi.fn(); + const savedLayout = makeTwoLeafSavedLayout( + [ + { + id: "center", + views: [OLD_SESSION_PANEL_ID, PR_DETAILS_ID], + activeView: OLD_SESSION_PANEL_ID, + }, + ], + "center", + ); + const { api } = makeAgentReviewApi(setActiveAgent, setActiveReview); + vi.mocked(getEnvLayout).mockReturnValueOnce(savedLayout); + vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(true); + + performEnvSwitch(makeParams({ api })); + + expect(setActiveAgent).toHaveBeenCalledOnce(); + expect(setActiveReview).not.toHaveBeenCalled(); +}); + +it("uses the effective default Agent selection when the target has no saved layout", () => { + const setActiveAgent = vi.fn(); + const setActiveReview = vi.fn(); + const liveGroupId = "generated-center-group"; + const { api } = makeAgentReviewApi(setActiveAgent, setActiveReview, liveGroupId); + vi.mocked(layoutStructuresMatch).mockReturnValueOnce(true); + const liveLayout = { + columns: [ + { + id: "center", + groups: [ + { + id: liveGroupId, + panels: [ + { id: NEW_SESSION_PANEL_ID, component: "chat", title: "Agent" }, + { id: PR_DETAILS_ID, component: PR_DETAILS_ID, title: PR_DETAILS_TITLE }, + ], + }, + ], + }, + ], + }; + vi.mocked(fromDockviewApi).mockReturnValueOnce({ columns: [] }).mockReturnValueOnce(liveLayout); + + performEnvSwitch( + makeParams({ + api, + getDefaultLayout: vi.fn(() => ({ + columns: [ + { + id: "center", + groups: [ + { + activePanel: "chat", + panels: [ + { id: "chat", component: "chat", title: "Agent" }, + { id: PR_DETAILS_ID, component: PR_DETAILS_ID, title: PR_DETAILS_TITLE }, + ], + }, + ], + }, + ], + })), + }), + ); + + expect(setActiveAgent).toHaveBeenCalledOnce(); + expect(setActiveReview).not.toHaveBeenCalled(); +}); + +it("does not rebind a default Agent selection when no session exists", () => { + const setActiveReview = vi.fn(); + const groupId = "generated-center-without-session"; + const group: TestGroup = { + id: groupId, + panels: [{ id: PR_DETAILS_ID, api: { component: PR_DETAILS_ID, setActive: setActiveReview } }], + }; + const api = { + ...makeMockApi(), + groups: [group], + panels: group.panels, + } as unknown as EnvSwitchParams["api"]; + vi.mocked(layoutStructuresMatch).mockReturnValueOnce(true); + const liveLayout = { + columns: [ + { + id: "center", + groups: [ + { + id: groupId, + panels: [{ id: PR_DETAILS_ID, component: PR_DETAILS_ID, title: PR_DETAILS_TITLE }], + }, + ], + }, + ], + }; + vi.mocked(fromDockviewApi).mockReturnValueOnce({ columns: [] }).mockReturnValueOnce(liveLayout); + + performEnvSwitch( + makeParams({ + api, + activeSessionId: null, + getDefaultLayout: vi.fn(() => ({ + columns: [ + { + id: "center", + groups: [ + { + activePanel: "chat", + panels: [ + { id: "chat", component: "chat", title: "Agent" }, + { id: PR_DETAILS_ID, component: PR_DETAILS_ID, title: PR_DETAILS_TITLE }, + ], + }, + ], + }, + ], + })), + }), + ); + + expect(setActiveReview).not.toHaveBeenCalled(); +}); + +it("rebinds a stale saved session selection after slow-path replacement", () => { + const setActiveAgent = vi.fn(); + const setActiveReview = vi.fn(); + const group: TestGroup = { id: "saved-center", panels: [] }; + const panels: TestPanel[] = []; + const removePanel = (id: string) => { + const index = panels.findIndex((panel) => panel.id === id); + if (index >= 0) panels.splice(index, 1); + const groupIndex = group.panels.findIndex((panel) => panel.id === id); + if (groupIndex >= 0) group.panels.splice(groupIndex, 1); + }; + const stale: TestPanel = { + id: OLD_SESSION_PANEL_ID, + api: { component: "chat", close: () => removePanel(OLD_SESSION_PANEL_ID) }, + group, + }; + const review: TestPanel = { + id: PR_DETAILS_ID, + api: { component: PR_DETAILS_ID, setActive: setActiveReview }, + group, + }; + panels.push(stale, review); + group.panels.push(stale, review); + const addPanel = vi.fn((options: { id: string; component: string }) => { + const incoming: TestPanel = { + id: options.id, + api: { component: options.component, setActive: setActiveAgent }, + group, + }; + panels.unshift(incoming); + group.panels.unshift(incoming); + return incoming; + }); + const savedLayout = makeTwoLeafSavedLayout( + [ + { + id: group.id, + views: [OLD_SESSION_PANEL_ID, PR_DETAILS_ID], + activeView: OLD_SESSION_PANEL_ID, + }, + ], + group.id, + ); + vi.mocked(getEnvLayout).mockReturnValueOnce(savedLayout).mockReturnValueOnce(savedLayout); + vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(false); + const api = { + ...makeMockApi(), + panels, + groups: [group], + addPanel, + getPanel: vi.fn((id: string) => panels.find((panel) => panel.id === id) ?? null), + } as unknown as EnvSwitchParams["api"]; + + performEnvSwitch(makeParams({ api })); + + expect(api.fromJSON).toHaveBeenCalledOnce(); + expect(setActiveAgent).toHaveBeenCalledOnce(); + expect(setActiveReview).not.toHaveBeenCalled(); +}); + +it("keeps a deliberately selected PR Details panel selected", () => { + const setActiveAgent = vi.fn(); + const setActiveReview = vi.fn(); + const savedLayout = makeTwoLeafSavedLayout( + [{ id: "center", views: ["chat", PR_DETAILS_ID], activeView: PR_DETAILS_ID }], + "center", + ); + const { api } = makeAgentReviewApi(setActiveAgent, setActiveReview); + vi.mocked(getEnvLayout).mockReturnValueOnce(savedLayout); + vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(true); + + performEnvSwitch(makeParams({ api })); + + expect(setActiveReview).toHaveBeenCalledOnce(); + expect(setActiveAgent).not.toHaveBeenCalled(); +}); + +it("restores saved per-group active tabs on the fast path", () => { + const setActiveRight = vi.fn(); + const setActiveCenter = vi.fn(); + const rightGroup: TestGroup = { + id: "right", + panels: [ + { id: "plan", api: { setActive: setActiveRight } }, + { id: "files", api: { setActive: vi.fn() } }, + ], + }; + const centerGroup: TestGroup = { + id: "center", + panels: [{ id: NEW_SESSION_PANEL_ID, api: { setActive: setActiveCenter } }], + }; + const savedLayout = makeTwoLeafSavedLayout( + [ + { id: "center", views: ["chat"], activeView: "chat" }, + { id: "right", views: ["plan", "files"], activeView: "plan" }, + ], + "right", + ); + vi.mocked(getEnvLayout).mockReturnValueOnce(savedLayout).mockReturnValueOnce(savedLayout); + vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(true); + const api = { + ...makeMockApi(), + groups: [centerGroup, rightGroup], + getPanel: vi.fn((id: string) => (id === NEW_SESSION_PANEL_ID ? centerGroup.panels[0] : null)), + } as unknown as EnvSwitchParams["api"]; + + performEnvSwitch(makeParams({ api })); + + expect(setActiveCenter).toHaveBeenCalledOnce(); + expect(setActiveRight).toHaveBeenCalled(); + const lastRightCall = setActiveRight.mock.invocationCallOrder.at(-1) ?? 0; + const lastCenterCall = setActiveCenter.mock.invocationCallOrder.at(-1) ?? 0; + expect(lastRightCall).toBeGreaterThan(lastCenterCall); +}); diff --git a/apps/web/lib/state/dockview-env-switch-active-views.ts b/apps/web/lib/state/dockview-env-switch-active-views.ts new file mode 100644 index 0000000000..3e3678447c --- /dev/null +++ b/apps/web/lib/state/dockview-env-switch-active-views.ts @@ -0,0 +1,159 @@ +import type { DockviewApi, SerializedDockview } from "dockview-react"; +import type { LayoutNode, LayoutState } from "./layout-manager"; +import { CENTER_GROUP } from "./layout-manager"; + +type ActiveViewEntry = { groupId: string; activeView: string }; + +type ActiveViewTarget = { + entries: ActiveViewEntry[]; + activeGroup?: string; +}; + +type LayoutGroup = LayoutState["columns"][number]["groups"][number]; + +function collectSavedActiveViews( + saved: SerializedDockview, +): Array<{ groupId: string; activeView: string }> { + const out: Array<{ groupId: string; activeView: string }> = []; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const walk = (node: any): void => { + if (!node) return; + if (Array.isArray(node.data)) { + for (const child of node.data) walk(child); + return; + } + const data = node.data; + if (data?.id && data.activeView) out.push({ groupId: data.id, activeView: data.activeView }); + }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + walk((saved as any).grid?.root); + return out; +} + +function collectLayoutGroups(layout: LayoutState): LayoutGroup[] { + const groups: LayoutGroup[] = []; + const visit = (node: LayoutNode): void => { + if (node.type === "leaf") { + groups.push(node.group); + return; + } + for (const child of node.children) visit(child); + }; + + for (const column of layout.columns) { + if (column.tree) { + visit(column.tree); + continue; + } + groups.push(...column.groups); + } + return groups; +} + +function collectDefaultActiveViews(layout: LayoutState, liveLayout: LayoutState): ActiveViewTarget { + const liveGroups = collectLayoutGroups(liveLayout); + const liveGroupIds = new Set(liveGroups.flatMap((group) => (group.id ? [group.id] : []))); + const entries: Array = []; + let explicitActiveGroup: string | undefined; + let groupIndex = 0; + + const addGroup = (group: LayoutGroup): void => { + const liveGroup = liveGroups[groupIndex++]; + const groupId = + (group.id && liveGroupIds.has(group.id) ? group.id : undefined) ?? liveGroup?.id; + const activeView = group.activePanel ?? group.panels[0]?.id; + if (!groupId || !activeView) return; + const explicit = !!group.activePanel; + entries.push({ groupId, activeView, explicit }); + if (explicit && !explicitActiveGroup) explicitActiveGroup = groupId; + }; + + for (const group of collectLayoutGroups(layout)) addGroup(group); + + return { + entries, + activeGroup: + explicitActiveGroup ?? entries.find((entry) => entry.groupId === CENTER_GROUP)?.groupId, + }; +} + +function isSemanticAgentView(activeView: string): boolean { + return activeView === "chat" || activeView.startsWith("session:"); +} + +function resolveTargetPanel( + group: DockviewApi["groups"][number], + activeView: string, + activeSessionId: string | null, +): DockviewApi["panels"][number] | undefined { + if (activeSessionId && isSemanticAgentView(activeView)) { + const incomingSessionPanel = group.panels.find( + (panel) => panel.id === `session:${activeSessionId}`, + ); + if (incomingSessionPanel) return incomingSessionPanel; + } + return group.panels.find((panel) => panel.id === activeView); +} + +/** + * Restore each group's selected panel from the target layout. Agent panel IDs + * are dynamic, so saved `chat` or stale `session:*` selections resolve to the + * incoming session panel after session reconciliation. The fast path does not + * call `fromJSON`, and the slow path replaces stale sessions after it does; + * both paths therefore need this same post-reconciliation replay. + * + * The target `activeGroup` is applied last so the resulting global focus + * matches what was persisted (or the effective default's explicit Agent + * selection). + */ +function restoreTargetActiveViews( + api: DockviewApi, + target: ActiveViewTarget, + activeSessionId: string | null, +): void { + const ordered = target.activeGroup + ? [ + ...target.entries.filter((entry) => entry.groupId !== target.activeGroup), + ...target.entries.filter((entry) => entry.groupId === target.activeGroup), + ] + : target.entries; + for (const { groupId, activeView } of ordered) { + const group = api.groups.find((g) => g.id === groupId); + if (!group) continue; + const panel = resolveTargetPanel(group, activeView, activeSessionId); + if (panel) { + try { + panel.api.setActive(); + } catch { + /* panel may be in a transient state */ + } + } + } +} + +export function restoreSavedActiveViews( + api: DockviewApi, + saved: SerializedDockview, + activeSessionId: string | null, +): void { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const activeGroup = (saved as any).activeGroup as string | undefined; + restoreTargetActiveViews( + api, + { entries: collectSavedActiveViews(saved), activeGroup }, + activeSessionId, + ); +} + +export function restoreDefaultActiveViews( + api: DockviewApi, + defaultLayout: LayoutState, + liveLayout: LayoutState, + activeSessionId: string | null, +): void { + restoreTargetActiveViews( + api, + collectDefaultActiveViews(defaultLayout, liveLayout), + activeSessionId, + ); +} diff --git a/apps/web/lib/state/dockview-env-switch.test.ts b/apps/web/lib/state/dockview-env-switch.test.ts index 0894634745..dd3f35766f 100644 --- a/apps/web/lib/state/dockview-env-switch.test.ts +++ b/apps/web/lib/state/dockview-env-switch.test.ts @@ -94,25 +94,6 @@ function makeParams(overrides?: Partial): EnvSwitchParams { }; } -function makeTwoLeafSavedLayout( - leaves: Array<{ id: string; views: string[]; activeView: string }>, - activeGroup: string, -): ReturnType { - return { - grid: { - root: { - type: "branch" as const, - data: leaves.map((leaf) => ({ type: "leaf", data: leaf })), - }, - height: 600, - width: 800, - orientation: "HORIZONTAL" as const, - }, - panels: { chat: { contentComponent: "chat" } }, - activeGroup, - } as unknown as ReturnType; -} - describe("performEnvSwitch", () => { beforeEach(() => { vi.clearAllMocks(); @@ -650,51 +631,3 @@ describe("performEnvSwitch missing Agent restoration", () => { ); }); }); - -describe("performEnvSwitch fast-path active view restoration", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("restores saved per-group active tabs on the fast path", () => { - // Regression: the fast path skips fromJSON, so per-group active tabs - // from the outgoing env would persist into the incoming env. The saved - // layout's activeView for each group must be reapplied. - const setActiveRight = vi.fn(); - const setActiveCenter = vi.fn(); - const rightGroup = { - id: "right", - panels: [ - { id: "plan", api: { setActive: setActiveRight } }, - { id: "files", api: { setActive: vi.fn() } }, - ], - }; - const centerGroup = { - id: "center", - panels: [{ id: NEW_SESSION_PANEL_ID, api: { setActive: setActiveCenter } }], - }; - const savedLayout = makeTwoLeafSavedLayout( - [ - { id: "center", views: ["chat"], activeView: "chat" }, - { id: "right", views: ["plan", "files"], activeView: "plan" }, - ], - "right", - ); - vi.mocked(getEnvLayout).mockReturnValueOnce(savedLayout).mockReturnValueOnce(savedLayout); - vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(true); - const api = { - ...makeMockApi(), - groups: [centerGroup, rightGroup], - getPanel: vi.fn((id: string) => (id === NEW_SESSION_PANEL_ID ? centerGroup.panels[0] : null)), - } as unknown as EnvSwitchParams["api"]; - - performEnvSwitch(makeParams({ api })); - - expect(setActiveRight).toHaveBeenCalled(); - // The saved activeGroup ("right") is applied last, so its setActive must - // be the most recent — otherwise center would steal global focus. - const lastRightCall = setActiveRight.mock.invocationCallOrder.at(-1) ?? 0; - const lastCenterCall = setActiveCenter.mock.invocationCallOrder.at(-1) ?? 0; - expect(lastRightCall).toBeGreaterThan(lastCenterCall); - }); -}); diff --git a/apps/web/lib/state/dockview-env-switch.ts b/apps/web/lib/state/dockview-env-switch.ts index 04b806b2b4..d9a02b68ed 100644 --- a/apps/web/lib/state/dockview-env-switch.ts +++ b/apps/web/lib/state/dockview-env-switch.ts @@ -25,6 +25,10 @@ import { } from "./layout-manager"; import { resolveResponsiveRightWidth } from "./layout-manager/right-width"; import type { LayoutState, LayoutGroupIds } from "./layout-manager"; +import { + restoreDefaultActiveViews, + restoreSavedActiveViews, +} from "./dockview-env-switch-active-views"; import { ENV_SCOPED_DOCKVIEW_COMPONENTS } from "./dockview-env-scoped-components"; import { createDebugLogger, isDebug } from "@/lib/debug/log"; import { @@ -76,60 +80,6 @@ function savedLayoutHasEphemeralPanels(serialized: SerializedDockview): boolean return Object.values(panels).some((p) => EPHEMERAL_COMPONENTS.has(p.contentComponent ?? "")); } -/** Walk the serialized grid tree collecting (groupId, activeView) for each leaf. */ -function collectSavedActiveViews( - saved: SerializedDockview, -): Array<{ groupId: string; activeView: string }> { - const out: Array<{ groupId: string; activeView: string }> = []; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const walk = (node: any): void => { - if (!node) return; - if (Array.isArray(node.data)) { - for (const child of node.data) walk(child); - return; - } - const data = node.data; - if (data?.id && data.activeView) out.push({ groupId: data.id, activeView: data.activeView }); - }; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - walk((saved as any).grid?.root); - return out; -} - -/** - * Restore each group's `activeView` from the saved layout. The fast path - * doesn't call `fromJSON`, so per-group active tabs would otherwise carry - * over from the outgoing env (e.g. Task B left "changes" focused in the - * right group, and switching back to Task A would still show "changes" - * even though Task A had "plan" active when it was last saved). - * - * The saved `activeGroup` is applied last so the resulting global focus - * matches what was persisted. - */ -function restoreSavedActiveViews(api: DockviewApi, saved: SerializedDockview): void { - const entries = collectSavedActiveViews(saved); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const savedActiveGroup = (saved as any).activeGroup as string | undefined; - const ordered = savedActiveGroup - ? [ - ...entries.filter((e) => e.groupId !== savedActiveGroup), - ...entries.filter((e) => e.groupId === savedActiveGroup), - ] - : entries; - for (const { groupId, activeView } of ordered) { - const group = api.groups.find((g) => g.id === groupId); - if (!group) continue; - const panel = group.panels.find((p) => p.id === activeView); - if (panel) { - try { - panel.api.setActive(); - } catch { - /* panel may be in a transient state */ - } - } - } -} - export type EnvSwitchParams = { api: DockviewApi; oldEnvId: string | null; @@ -367,7 +317,16 @@ function tryFastEnvSwitch(params: EnvSwitchParams): LayoutGroupIds | null { // The fast path skips `fromJSON`, so per-group active tabs from the // outgoing env would otherwise persist into the incoming env. Reapply // them from the saved layout to match what `fromJSON` would have done. - if (saved) restoreSavedActiveViews(api, saved as SerializedDockview); + if (saved) { + restoreSavedActiveViews(api, saved as SerializedDockview, activeSessionId); + } else { + restoreDefaultActiveViews( + api, + params.getDefaultLayout(), + fromDockviewApi(api), + activeSessionId, + ); + } // Column widths from the outgoing env stay live across the switch because // we skipped fromJSON. Apply the target env's widths explicitly: @@ -612,6 +571,7 @@ export function performEnvSwitch(params: EnvSwitchParams): LayoutGroupIds { // useAutoSessionTab will still no-op if the panel was just added here. replaceStaleSessionPanels(api, activeSessionId, currentSessionIds); if (activeSessionId) restoreMissingSessionPanel(api, activeSessionId); + restoreSavedActiveViews(api, saved as SerializedDockview, activeSessionId); api.layout(safeWidth, safeHeight); if (isDebug()) { debug("performEnvSwitch: completed via slow path (fromJSON)", { diff --git a/docs/plans/dockview-active-tab-restoration/plan.md b/docs/plans/dockview-active-tab-restoration/plan.md new file mode 100644 index 0000000000..74ec7bb25c --- /dev/null +++ b/docs/plans/dockview-active-tab-restoration/plan.md @@ -0,0 +1,198 @@ +--- +spec: docs/specs/ui/task-layout-profiles.md +created: 2026-08-02 +status: completed +--- + +# Implementation Plan: Restore Dockview Active Tabs Across Tasks + +## Overview + +Repair desktop task-environment restoration so a selected Agent tab survives +the dynamic replacement of `chat` or a stale `session:` with the incoming +task's live `session:`. The implementation will restore every group's +selection from the target task's saved layout, or from the effective default +for a fresh environment, while preserving exact non-Agent selections and +applying the saved global group last. + +## Confirmed Root Cause + +Task selection correctly resolves and focuses the incoming primary session. +The observed failure happens later, while Dockview restores the target +environment: + +- `performEnvSwitch` reconciles the serialized task layout with the incoming + session's dynamic panel ID; +- the fast path replays saved `activeView` values only by exact panel ID; +- a saved Agent selection such as `chat` or a stale `session:` therefore + does not match the newly materialized `session:` panel; +- the slow path calls `fromJSON` before replacing stale session panels and does + not replay the selection after replacement; and +- `useAutoSessionTab` then correctly declines to steal focus on a cross-task + restore, leaving the neighboring PR Details panel active. + +Changing the later `shouldActivateSessionPanel` decision to always select Agent +would violate the existing contract for tasks whose last selected tab really +was PR Details, Plan, Files, Changes, or another valid non-Agent panel. The +repair belongs in environment restoration, where the target layout and the +incoming session identity are both available. + +## Frontend + +Likely file: + +- `apps/web/lib/state/dockview-env-switch.ts` + +Introduce one post-reconciliation active-view restoration path that: + +- obtains the target selections from the healthy saved serialized layout, or + from the materialized effective default when no task layout exists; +- recognizes `chat` and `session:*` as the semantic Agent slot only when that + selected view represents a session panel; +- maps that Agent selection to `session:${activeSessionId}` after stale and + missing session panels have been reconciled; +- restores stable non-Agent panel IDs only by exact match; +- restores each group's selected panel without changing group structure or tab + order; and +- applies the target globally active group last so per-group restoration does + not leave focus in the wrong split. + +Use the same restoration helper after both the structure-preserving fast path +and the `fromJSON` slow path. Keep the pending-inactive-task Changes override in +its existing later lifecycle position, so meaningful updates can still +deliberately supersede the saved selection. + +## Tests + +### Unit RED/GREEN + +File: + +- `apps/web/lib/state/dockview-env-switch.test.ts` + +Add focused regressions for: + +- fast-path restoration of saved `activeView: "chat"` to the live incoming + session panel beside PR Details; +- fast-path restoration of a stale `session:` selection to the live + incoming session panel; +- slow-path `fromJSON` restoration after stale-session replacement; +- a deliberately selected `pr-detail` remaining selected exactly; +- selections in more than one group being restored while the serialized + `activeGroup` is activated last; and +- a fresh environment using the effective default's Agent selection instead + of inheriting PR Details from the outgoing environment. + +Retain the existing session-tab activation tests in the targeted command to +guard the downstream policy that intentionally avoids stealing restored focus. + +## E2E Tests + +### Desktop PR Details round trip + +File: + +- `apps/web/e2e/tests/pr/pr-detail-layout.spec.ts` + +Create two desktop tasks with settled sessions. In the first task, use the +Default center group containing Agent and PR Details, select PR Details and then +Agent so Agent is the last real selection, switch to the second task, and return +through the normal task UI. Assert that: + +- the first task's live Agent tab and content are active; +- PR Details remains present but inactive; +- the current session panel exists exactly once. + +Add a companion round trip where PR Details is deliberately left selected and +prove it remains selected. This distinguishes semantic Agent rebinding from an +unconditional Agent activation. + +Run the scenario through Kandev's managed production-build Playwright runner; +use API helpers only for task/session preconditions and verify focus through +the rendered workbench plus the exposed Dockview API where group-local identity +is not observable from content alone. + +## Mobile And Tablet + +No mobile or tablet regression is required. Phone and tablet task detail use +`SessionMobileLayout` and `SessionTabletLayout`, not `DockviewDesktopLayout`, +and do not execute the environment-switch restoration being repaired. This +change introduces no rendered UI, copy, responsive composition, touch target, +scroll owner, or mobile navigation behavior. + +## Implementation Waves + +Wave 1 (sequential): + +- [x] [Task 01 — Restore semantic active views](task-01-restore-semantic-active-views.md) + +## Verification + +Targeted unit RED/GREEN and downstream activation policy: + +```bash +cd apps && pnpm --filter @kandev/web test -- --run \ + lib/state/dockview-env-switch.test.ts \ + components/task/dockview-session-tab-activation.test.ts \ + components/task/dockview-session-tabs.test.ts +``` + +Frontend typecheck: + +```bash +cd apps/web && pnpm run typecheck +``` + +Managed production-build desktop E2E: + +```bash +cd apps/web && pnpm e2e:run tests/pr/pr-detail-layout.spec.ts \ + -- --grep "restores each task's selected center tab" +``` + +## Risks + +- Dockview's group-local selected panel and globally active panel are distinct; + restoring one without ordering the other can produce a correct-looking + center tab while moving keyboard focus to the wrong split. +- Treating every missing panel ID as Agent would resurrect removed panels or + overwrite legitimate PR Details, Plan, preview, or file selections. Only + reusable Chat and serialized session identities may rebind. +- Replaying selections before stale-session replacement repeats the bug because + the target live Agent ID does not yet exist. +- The no-saved-layout fast path must take selection intent from the effective + default without applying the default's entire structure over the live layout. +- Pending inactive-task Changes attention must remain a deliberate later + override and must not be folded into the generic restoration helper. + +## Out Of Scope + +- Changing task/session selection or primary-session resolution. +- Always activating Agent during cross-task navigation. +- Changing Dockview versions, saved-layout schemas, or storage keys. +- Altering pending Changes attention behavior. +- Changing mobile or tablet task-detail composition. +- Adding new user-facing copy or public configuration. + +## Documentation Impact + +No public documentation changes are required. This repair makes the desktop +workbench conform to its existing task-layout persistence contract and changes +no command, configuration key, API, or user-facing terminology. + +## Implementation Results + +- RED unit run reproduced the bug: saved `chat`, stale `session:`, and + fresh-default Agent selections were not activated, while the neighboring PR + Details panel remained eligible to win focus. +- GREEN implementation centralizes target active-view replay, resolves only + semantic Agent identities to `session:${activeSessionId}`, and runs after + reconciliation on both fast and slow environment-switch paths. +- Targeted unit suite: 70 tests passed. +- Frontend typecheck: passed. +- Managed production-build Chromium E2E: 1 test passed, covering Agent-last and + deliberate PR-Details-last round trips. +- Targeted ESLint: no errors; two max-lines warnings remain in the large + Dockview environment-switch test file. +- No mobile/tablet code or tests changed because those layouts do not execute + Dockview environment restoration. diff --git a/docs/plans/dockview-active-tab-restoration/task-01-restore-semantic-active-views.md b/docs/plans/dockview-active-tab-restoration/task-01-restore-semantic-active-views.md new file mode 100644 index 0000000000..803cd96d34 --- /dev/null +++ b/docs/plans/dockview-active-tab-restoration/task-01-restore-semantic-active-views.md @@ -0,0 +1,133 @@ +--- +id: "01-restore-semantic-active-views" +title: "Restore semantic Dockview active views" +status: done +wave: 1 +depends_on: [] +parallelism: sequential +plan: "plan.md" +spec: "../../specs/ui/task-layout-profiles.md" +--- + +# Task 01: Restore Semantic Dockview Active Views + +## Acceptance + +- A saved or default Agent selection represented by `chat` or a stale + `session:` selects the incoming task's live Agent panel after environment + restoration. +- A deliberately selected, still-live non-Agent panel such as PR Details, + Plan, Files, or Changes remains selected by exact identity. +- Every live group's target selection is restored, with the serialized global + active group applied last. +- Fast-path, slow-path, saved-layout, and fresh-default task switches follow the + same selection semantics. +- Pending inactive-task Changes attention, session-tab ordering, group + structure, layout geometry, and mobile/tablet behavior remain unchanged. + +## TDD Sequence + +1. RED: add fast-path unit cases where `chat` and a stale session ID are saved + as active beside PR Details; prove exact-ID replay leaves PR Details active. +2. RED: add the slow-path stale-session replacement case, exact PR Details + preservation, multi-group/global-focus ordering case, and fresh-default + selection case. +3. RED: add the production-build desktop round trips proving both Agent-last + and PR-Details-last behavior across two tasks. +4. GREEN: normalize the target layout's selected views, rebind only semantic + Agent identities to the incoming session panel, and replay selections after + session reconciliation on both environment-switch paths. +5. REFACTOR: keep target-selection extraction, semantic identity resolution, + and ordered Dockview activation explicit; do not widen the downstream + session-tab activation policy. +6. Run the exact targeted unit, typecheck, and production-build E2E commands. + +## Verification + +If this worktree has not installed dependencies yet, bootstrap once: + +```bash +cd apps && pnpm install --frozen-lockfile +``` + +Then run: + +```bash +cd apps && pnpm --filter @kandev/web test -- --run \ + lib/state/dockview-env-switch.test.ts \ + components/task/dockview-session-tab-activation.test.ts \ + components/task/dockview-session-tabs.test.ts +cd apps/web && pnpm run typecheck +cd apps/web && pnpm e2e:run tests/pr/pr-detail-layout.spec.ts \ + -- --grep "restores each task's selected center tab" +``` + +## Files Likely Touched + +- `apps/web/lib/state/dockview-env-switch.ts` +- `apps/web/lib/state/dockview-env-switch.test.ts` +- `apps/web/e2e/tests/pr/pr-detail-layout.spec.ts` +- `apps/web/e2e/pages/session-page.ts` only if the scenario needs a reusable + active-session-tab locator or Dockview-state helper. + +## Dependencies + +None. + +## Parallelism + +Sequential. The unit cases, post-reconciliation ordering, and browser round +trip describe one timing-sensitive Dockview state transition and should remain +in a single Red-Green-Refactor cycle. + +## Inputs + +- `docs/specs/ui/task-layout-profiles.md`, especially desktop restoration, + persistence guarantees, and the Agent/PR Details round-trip scenarios. +- `docs/plans/dockview-active-tab-restoration/plan.md`. +- `apps/web/lib/state/dockview-env-switch.ts`, including + `restoreSavedActiveViews`, `tryFastEnvSwitch`, `replaceStaleSessionPanels`, + `restoreMissingSessionPanel`, and the slow `fromJSON` path. +- `apps/web/lib/state/layout-manager/session-panels.ts` for the existing + reusable-Chat-to-live-session materialization semantics. +- Existing task-switch coverage in + `apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts` and PR Details + helpers in `apps/web/e2e/tests/pr/pr-detail-layout.spec.ts`. + +## Risks + +- A final `api.activePanel` assertion alone does not prove every group's local + selected tab; unit and E2E checks must inspect both concepts. +- Activation order matters because each `panel.api.setActive()` can update the + global active group. Apply the intended global group last. +- A stale selected ID may represent a removed non-Agent task-specific panel; + do not map arbitrary missing IDs to Agent. +- Default selection extraction must materialize only the semantic Agent ID and + must not rebuild or overwrite the fast-path layout structure. + +## Output Contract + +Report the RED failures, semantic mapping rule, minimum production change, +exact files changed, unit/typecheck/E2E results, plan/task status updates, and +any remaining ambiguity in Dockview's group-local versus global focus state. + +## Results + +- RED unit run: 4 expected failures showed exact-ID replay skipped saved + `chat`/stale session selections and did not restore the default Agent view. +- GREEN implementation: `restoreTargetActiveViews` rebinds semantic Agent IDs + after session replacement, preserves exact non-Agent IDs, and orders the + serialized active group last. The same rule now covers saved and fresh + default layouts on both fast and slow paths. +- Unit verification: 70 tests passed across the environment-switch and session + activation suites. +- Typecheck: `pnpm run typecheck` passed. +- E2E verification: the managed production-build Chromium scenario passed + (1/1), including Agent-last restoration, exactly one live session tab, and + deliberate PR Details preservation. +- Targeted ESLint: no errors; two max-lines warnings remain in + `dockview-env-switch.test.ts`. +- Formatting: Prettier check passed for all changed code and plan/task files; + `git diff --check` passed. +- Mobile/tablet: no rendered or responsive path changed; those viewports do + not mount `DockviewDesktopLayout` or execute this restoration hook. diff --git a/docs/specs/ui/task-layout-profiles.md b/docs/specs/ui/task-layout-profiles.md index 10e3db9921..72ad87d5db 100644 --- a/docs/specs/ui/task-layout-profiles.md +++ b/docs/specs/ui/task-layout-profiles.md @@ -29,6 +29,7 @@ Users can arrange and save the desktop task workbench only while a task is open, - Returning to a desktop task restores its task-specific layout, then activates Changes when a meaningful Git or commit update was detected while that task was inactive. This pending-change attention overrides the saved active panel, but an ordinary reload with already-known changes preserves the saved active panel. - Switching between tasks whose sessions share one task environment atomically replaces task-owned Agent tabs in their existing group. The handoff preserves the live split tree and proportions instead of briefly emptying the group or rebuilding the environment layout. - Desktop Agent-tab reconciliation preserves both the selected tab in each existing group and the globally focused panel unless pending inactive-task changes deliberately activate Changes. Replacing a selected Chat placeholder with the active session keeps Agent selected in that group without transiently selecting a neighboring Plan tab, while focus in another group such as Files or Changes remains unchanged. A valid non-Agent tab that was already selected in the Agent group remains selected. +- Desktop task-environment restoration finalizes each group's selected tab only after the incoming task's Agent panels have been reconciled. A saved or default Agent selection is semantic: `chat` or a stale `session:` resolves to the incoming active session's live Agent panel in that group. Stable non-Agent selections such as PR Details, Plan, Files, Changes, or a surviving task-specific panel remain exact, and the restored globally focused group is applied last. - The existing workbench layout menu continues to apply built-in and custom profiles and save the current workbench as a custom profile. Profile mutations from either surface remain consistent after the user-settings response is received. - Layout-profile editing is usable with pointer, keyboard, and touch input. On narrow settings viewports, profile management and all editor commands remain reachable without horizontal page scrolling. - Layout profiles configure the desktop Dockview workbench only. Mobile and tablet task-detail layouts retain their existing behavior. @@ -85,6 +86,7 @@ The frontend treats the returned settings payload as authoritative after each su - A task handoff within the same environment does not persist an intermediate panel-removal state or change the root split orientation. - Completing a normal, non-maximized desktop task-layout restore re-establishes a live Agent panel for the active session before the workbench is revealed. If the restored center group is empty, Agent is inserted there and activated; if another valid saved center tab such as Plan is active, that tab remains active. - Replacing the generic Chat placeholder with a session-owned Agent panel preserves the placeholder group's selected content and the workbench's globally focused panel; internal tab insertion, removal, or ordering does not acknowledge an unseen Plan. +- Task-environment selection state survives dynamic Agent panel IDs. When the selected Agent identity in a saved task layout or fresh effective default no longer exists verbatim, Kandev rebinds that selection to the incoming active session instead of retaining a neighboring panel selected by the outgoing task or by Dockview reconciliation. Exact non-Agent selections are never rewritten as Agent selections. ## Scenarios @@ -108,6 +110,9 @@ The frontend treats the returned settings payload as authoritative after each su - **GIVEN** two tasks whose active sessions share one task environment and a desktop workbench with Agent in the center and Files or Changes above Terminal on the right, **WHEN** the user switches between those tasks, **THEN** the incoming Agent replaces the outgoing Agent in the same group, the right column remains vertically split, the root remains horizontally split, and the same geometry survives reload. - **GIVEN** a normal desktop task layout restore completes while the active session's Agent panel is absent, **WHEN** the center group is empty, **THEN** the Agent panel is restored into that group and activated before the workbench is shown; when Plan or another valid saved center tab is active, restoring Agent does not steal focus from it, and a deliberately maximized non-Agent group remains unchanged. - **GIVEN** a desktop task layout whose Agent group has Chat selected beside an unseen Plan while Files or Changes owns global focus, **WHEN** session reconciliation replaces Chat with the active session's Agent panel, **THEN** Agent remains selected in its group, the globally focused Files or Changes panel remains focused, Plan is never selected or marked seen, and the user is not switched to Plan. +- **GIVEN** a desktop task layout with Agent selected beside PR Details and a different task currently has PR Details selected, **WHEN** the user returns and the saved Agent ID is `chat` or belongs to an earlier session, **THEN** the returning task's live Agent panel is selected and PR Details remains a background tab. +- **GIVEN** a desktop task layout with PR Details deliberately selected beside Agent, **WHEN** the user switches away and returns without pending inactive-task changes, **THEN** PR Details remains selected rather than being rewritten to Agent. +- **GIVEN** a desktop task layout with an Agent selection in one group and Files or Changes selected in another globally focused group, **WHEN** dynamic session-panel reconciliation completes during task restoration, **THEN** both group-local selections are restored and the saved global group remains focused. - **GIVEN** a task environment whose right column has never been manually resized, **WHEN** its desktop workbench moves from a large monitor to a laptop-sized workbench and back, **THEN** the Files, Changes, and Terminal column follows the default ratio at each width and returns to the large-workbench ratio. - **GIVEN** a task environment whose right column was manually resized through its desktop sash, **WHEN** the workbench moves between large and laptop-sized displays, **THEN** that requested width is restored whenever it fits and is temporarily clamped only to preserve the current screen's minimum center width. - **GIVEN** a custom default profile, **WHEN** the user deletes it and confirms, **THEN** the built-in Default becomes effective. @@ -119,6 +124,7 @@ The frontend treats the returned settings payload as authoritative after each su - Customizing mobile or tablet task-detail layouts. - Auto-activating Changes merely because a task has an already-known non-zero change count. +- Forcing Agent active on every task switch when the task's saved non-Agent selection is valid. - Changing the global app sidebar width or other layout-profile split proportions. - Forcing a changed default onto existing task-specific layouts without Reset Layout. - Configuring task-specific panels such as individual files, diffs, commits, keyed pull-request or merge-request tabs, extra sessions, or extra terminals. The canonical PR Details panel is reusable and remains in scope.