diff --git a/apps/web/components/task/dockview-session-tabs.ts b/apps/web/components/task/dockview-session-tabs.ts index 1b26cfbaaa..ed1628a189 100644 --- a/apps/web/components/task/dockview-session-tabs.ts +++ b/apps/web/components/task/dockview-session-tabs.ts @@ -3,6 +3,7 @@ import type { DockviewReadyEvent, AddPanelOptions } from "dockview-react"; import type { StoreApi } from "zustand"; import type { AppState } from "@/lib/state/store"; import { useDockviewStore } from "@/lib/state/dockview-store"; +import { focusOrAddPanel } from "@/lib/state/dockview-layout-builders"; import { useAppStore } from "@/components/state-provider"; import { wasPRPanelOffered, markPRPanelOffered } from "@/lib/local-storage"; @@ -138,12 +139,14 @@ export function useAutoPRPanel() { if (decision === "add") { const { centerGroupId } = useDockviewStore.getState(); - const centerGroupExists = centerGroupId && api.groups.some((g) => g.id === centerGroupId); - api.addPanel({ + // Route through focusOrAddPanel so a stale centerGroupId falls back + // to another non-sidebar group rather than letting dockview place + // the panel in the active group (which may be the sidebar). + focusOrAddPanel(api, { id: "pr-detail", component: "pr-detail", title: "Pull Request", - position: centerGroupExists ? { referenceGroup: centerGroupId } : undefined, + position: { referenceGroup: centerGroupId }, inactive: true, }); markPRPanelOffered(sessionId); diff --git a/apps/web/lib/state/dockview-layout-builders.test.ts b/apps/web/lib/state/dockview-layout-builders.test.ts new file mode 100644 index 0000000000..9dd87b09eb --- /dev/null +++ b/apps/web/lib/state/dockview-layout-builders.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect } from "vitest"; +import type { DockviewApi } from "dockview-react"; +import { fallbackGroupPosition } from "./dockview-layout-builders"; +import { SIDEBAR_GROUP, CENTER_GROUP, RIGHT_TOP_GROUP, RIGHT_BOTTOM_GROUP } from "./layout-manager"; + +type MockGroup = { id: string }; +type MockPanel = { id: string; group: { id: string } }; + +function makeApi(groupIds: string[], panels: MockPanel[] = []): DockviewApi { + const groups: MockGroup[] = groupIds.map((id) => ({ id })); + return { + groups, + panels, + getPanel: (id: string) => panels.find((p) => p.id === id) ?? undefined, + } as unknown as DockviewApi; +} + +describe("fallbackGroupPosition", () => { + it("returns the center group when it exists", () => { + const api = makeApi([SIDEBAR_GROUP, CENTER_GROUP, "group-other"]); + + expect(fallbackGroupPosition(api)).toEqual({ referenceGroup: CENTER_GROUP }); + }); + + it("returns the chat panel's group even when right groups iterate first", () => { + // Drag-to-split can replace the well-known center group ID with a generated one. + // Right groups appear before the chat group in iteration order; the fallback + // must still prefer the chat group over right-column groups. + const chatGroupId = "group-3"; + const api = makeApi( + [SIDEBAR_GROUP, RIGHT_TOP_GROUP, RIGHT_BOTTOM_GROUP, chatGroupId], + [{ id: "chat", group: { id: chatGroupId } }], + ); + + expect(fallbackGroupPosition(api)).toEqual({ referenceGroup: chatGroupId }); + }); + + it("returns the session:* panel's group when no chat panel exists", () => { + // Active session: chat is replaced with session:. CENTER_GROUP id was lost. + // Right groups iterate first; fallback must still prefer the session group. + const sessionGroupId = "group-7"; + const api = makeApi( + [SIDEBAR_GROUP, RIGHT_TOP_GROUP, sessionGroupId], + [{ id: "session:abc", group: { id: sessionGroupId } }], + ); + + expect(fallbackGroupPosition(api)).toEqual({ referenceGroup: sessionGroupId }); + }); + + it("does not return a right-column group when no center-like group exists", () => { + // Right-column groups (Changes/Files/Terminal) are tool columns — placing + // a diff or PR panel there is the same UX bug as placing it in the sidebar. + // With only sidebar+right groups present and no chat/session panels, the + // fallback must drop the position so dockview doesn't pick a right group. + const api = makeApi([SIDEBAR_GROUP, RIGHT_TOP_GROUP, RIGHT_BOTTOM_GROUP]); + + expect(fallbackGroupPosition(api)).toBeUndefined(); + }); + + it("returns undefined when only the sidebar group exists", () => { + // Must NOT return the sidebar group — panels added to the locked sidebar + // would leak there, which is the bug we're fixing. + const api = makeApi([SIDEBAR_GROUP]); + + expect(fallbackGroupPosition(api)).toBeUndefined(); + }); + + it("returns undefined when no groups exist", () => { + const api = makeApi([]); + + expect(fallbackGroupPosition(api)).toBeUndefined(); + }); +}); diff --git a/apps/web/lib/state/dockview-layout-builders.ts b/apps/web/lib/state/dockview-layout-builders.ts index a5ca6552d4..4b70217e7d 100644 --- a/apps/web/lib/state/dockview-layout-builders.ts +++ b/apps/web/lib/state/dockview-layout-builders.ts @@ -43,17 +43,34 @@ export function applyLayoutFixups(api: DockviewApi): LayoutGroupIds { /** * Resolve a fallback group position when the intended reference is stale. - * Tries the well-known center group, then any non-sidebar group, then the - * first group. Returns undefined if no groups exist (drops the position). + * + * Tries to land in the center column, in this order: + * 1. Well-known CENTER_GROUP id. + * 2. Group containing the `chat` panel (post-drag, the well-known id may be + * gone but the chat panel still marks the center column). + * 3. Group containing any `session:*` panel (active session: chat is removed + * and replaced with per-session tabs). + * 4. Any group that is NOT the sidebar AND NOT a right-column group + * (Changes/Files/Terminal). Returning a right-column group would leak the + * panel into the narrow tools column — same UX bug as the sidebar leak. + * + * Returns undefined if no center-like group exists. The caller drops the + * position so dockview picks a default. Never returns the sidebar. */ -function fallbackGroupPosition(api: DockviewApi): { referenceGroup: string } | undefined { +export function fallbackGroupPosition(api: DockviewApi): { referenceGroup: string } | undefined { const centerGroup = api.groups.find((g) => g.id === CENTER_GROUP); if (centerGroup) return { referenceGroup: centerGroup.id }; - const nonSidebarGroup = api.groups.find((g) => g.id !== SIDEBAR_GROUP); - if (nonSidebarGroup) return { referenceGroup: nonSidebarGroup.id }; + const chatGroupId = api.getPanel("chat")?.group?.id; + if (chatGroupId) return { referenceGroup: chatGroupId }; - if (api.groups.length > 0) return { referenceGroup: api.groups[0].id }; + const sessionGroupId = api.panels.find((p) => p.id.startsWith("session:"))?.group?.id; + if (sessionGroupId) return { referenceGroup: sessionGroupId }; + + const centerish = api.groups.find( + (g) => g.id !== SIDEBAR_GROUP && g.id !== RIGHT_TOP_GROUP && g.id !== RIGHT_BOTTOM_GROUP, + ); + if (centerish) return { referenceGroup: centerish.id }; return undefined; } diff --git a/apps/web/lib/state/layout-manager/applier.test.ts b/apps/web/lib/state/layout-manager/applier.test.ts new file mode 100644 index 0000000000..b8c5f88bda --- /dev/null +++ b/apps/web/lib/state/layout-manager/applier.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect } from "vitest"; +import type { DockviewApi } from "dockview-react"; +import { resolveGroupIds } from "./applier"; +import { SIDEBAR_GROUP, CENTER_GROUP, RIGHT_TOP_GROUP, RIGHT_BOTTOM_GROUP } from "./constants"; + +type MockGroup = { id: string }; +type MockPanel = { id: string; group: { id: string } }; + +function makeApi(groups: MockGroup[], panels: MockPanel[] = []): DockviewApi { + return { + groups, + panels, + getPanel: (id: string) => panels.find((p) => p.id === id) ?? undefined, + } as unknown as DockviewApi; +} + +describe("resolveGroupIds", () => { + it("returns well-known IDs when all groups exist", () => { + const api = makeApi([ + { id: SIDEBAR_GROUP }, + { id: CENTER_GROUP }, + { id: RIGHT_TOP_GROUP }, + { id: RIGHT_BOTTOM_GROUP }, + ]); + + const ids = resolveGroupIds(api); + + expect(ids.sidebarGroupId).toBe(SIDEBAR_GROUP); + expect(ids.centerGroupId).toBe(CENTER_GROUP); + expect(ids.rightTopGroupId).toBe(RIGHT_TOP_GROUP); + expect(ids.rightBottomGroupId).toBe(RIGHT_BOTTOM_GROUP); + }); + + it("falls back to chat panel's group when CENTER_GROUP id missing", () => { + // Simulates post-drag state where center group has a dockview-generated ID + const chatGroupId = "group-5"; + const api = makeApi( + [{ id: SIDEBAR_GROUP }, { id: chatGroupId }], + [{ id: "chat", group: { id: chatGroupId } }], + ); + + const ids = resolveGroupIds(api); + + expect(ids.centerGroupId).toBe(chatGroupId); + }); + + it("falls back to session:* panel's group when no chat panel exists", () => { + // Active session: chat panel was removed, replaced with session: + // CENTER_GROUP id was lost (e.g. drag-to-split). This is the bug scenario. + const sessionGroupId = "group-7"; + const api = makeApi( + [{ id: SIDEBAR_GROUP }, { id: sessionGroupId }], + [{ id: "session:abc123", group: { id: sessionGroupId } }], + ); + + const ids = resolveGroupIds(api); + + expect(ids.centerGroupId).toBe(sessionGroupId); + }); + + it("returns the CENTER_GROUP constant as last-resort when nothing matches", () => { + // Last-resort fallback: returns the well-known constant even when no live + // group carries that ID. The caller (focusOrAddPanel) detects the stale ID + // and applies its own fallback via fallbackGroupPosition. + const api = makeApi([{ id: SIDEBAR_GROUP }], []); + + const ids = resolveGroupIds(api); + + expect(ids.centerGroupId).toBe(CENTER_GROUP); + }); +}); diff --git a/apps/web/lib/state/layout-manager/applier.ts b/apps/web/lib/state/layout-manager/applier.ts index b644b82045..9023403bfa 100644 --- a/apps/web/lib/state/layout-manager/applier.ts +++ b/apps/web/lib/state/layout-manager/applier.ts @@ -32,10 +32,23 @@ function findGroupId(api: DockviewApi, knownId: string, fallbackPanelId: string) return pnl?.group?.id ?? knownId; } +/** Find the center group, preferring the well-known ID, then "chat", then any + * "session:*" panel's group. When a session is active, "chat" is removed and + * replaced with per-session tabs — without the session fallback the returned + * ID would be a stale constant that doesn't match any live group. */ +function findCenterGroupId(api: DockviewApi): string { + if (api.groups.some((g) => g.id === CENTER_GROUP)) return CENTER_GROUP; + const chat = api.getPanel("chat"); + if (chat?.group?.id) return chat.group.id; + const sessionPanel = api.panels.find((p) => p.id.startsWith("session:")); + if (sessionPanel?.group?.id) return sessionPanel.group.id; + return CENTER_GROUP; +} + export function resolveGroupIds(api: DockviewApi): LayoutGroupIds { return { sidebarGroupId: findGroupId(api, SIDEBAR_GROUP, "sidebar"), - centerGroupId: findGroupId(api, CENTER_GROUP, "chat"), + centerGroupId: findCenterGroupId(api), // Always use the well-known constant — do NOT fall back to the "changes" // panel's current group. In plan mode the "changes" panel moves into the // center group; a panel-based fallback would return the center group ID and