From 46f20e0dfe723f7fef02e86f1c9e8fe047c5ba0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Mon, 8 Jun 2026 17:33:54 +0100 Subject: [PATCH] fix(web): keep dockview group alive on task switch to stop layout collapse (#1308) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): keep dockview group alive on task switch to stop layout collapse On a fast-path env switch, the incoming session chat was added only after removeEphemeralPanels closed the outgoing chat. When that chat was the last surviving panel in its group (e.g. the default layout's chat column), removing it first emptied — and so destroyed — the group. The captured group id then no longer existed, and post-#1165 the old `referenceGroup: "sidebar"` fallback is dead, so addPanel ran with an undefined position: dockview dropped the incoming chat into the active (terminal) group and collapsed the grid root to a vertical stack. The corrupted layout was then persisted, so it survived reloads. Add the incoming session panel before removing the outgoing one, anchored at the outgoing panel's current index, so the group stays alive throughout the swap. Removing the panels ahead of the insert point shifts the new panel into the same final slot, so computeSurvivingIndex is no longer needed. * docs(web): note the raw-index equivalence assumption in env switch --- .../web/lib/state/dockview-env-switch.test.ts | 46 ++++++++++++++++ apps/web/lib/state/dockview-env-switch.ts | 52 ++++++++----------- 2 files changed, 68 insertions(+), 30 deletions(-) diff --git a/apps/web/lib/state/dockview-env-switch.test.ts b/apps/web/lib/state/dockview-env-switch.test.ts index 8bbfc3c501..67a39f58c2 100644 --- a/apps/web/lib/state/dockview-env-switch.test.ts +++ b/apps/web/lib/state/dockview-env-switch.test.ts @@ -206,6 +206,52 @@ describe("performEnvSwitch", () => { }); }); +describe("performEnvSwitch fast-path group survival", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("adds the incoming session panel before closing the outgoing chat (keeps its group alive)", () => { + // Regression: when the outgoing session chat is the only surviving panel in + // its group (e.g. the default layout's chat column), removing it FIRST + // empties — and so destroys — the group. The captured group id then no + // longer exists, and post-#1165 the old `referenceGroup: "sidebar"` fallback + // is dead, so `addPanel` runs with an undefined position and dockview drops + // the incoming chat into whatever group is active (the terminal), + // collapsing the grid root to a vertical stack. Adding the incoming panel + // BEFORE removing the outgoing one keeps the group alive throughout. + vi.mocked(layoutStructuresMatch).mockReturnValueOnce(true); + const order: string[] = []; + const closeOutgoing = vi.fn(() => order.push("close")); + const groupId = "center-group"; + const outgoingPanels = [ + { + id: "session:old-session", + api: { component: "chat", isActive: true, close: closeOutgoing }, + }, + ]; + const outgoing = { ...outgoingPanels[0], group: { id: groupId, panels: outgoingPanels } }; + const addPanel = vi.fn(() => order.push("add")); + const api = { + ...makeMockApi(), + panels: [outgoing], + groups: [{ id: groupId }], + getPanel: vi.fn(() => null), + addPanel, + } as unknown as EnvSwitchParams["api"]; + + performEnvSwitch(makeParams({ api })); + + expect(addPanel).toHaveBeenCalledWith( + expect.objectContaining({ + id: NEW_SESSION_PANEL_ID, + position: { referenceGroup: groupId, index: 0 }, + }), + ); + expect(order).toEqual(["add", "close"]); + }); +}); + describe("performEnvSwitch slow-path stale session strip", () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/apps/web/lib/state/dockview-env-switch.ts b/apps/web/lib/state/dockview-env-switch.ts index e9dd000410..51aeca4464 100644 --- a/apps/web/lib/state/dockview-env-switch.ts +++ b/apps/web/lib/state/dockview-env-switch.ts @@ -139,8 +139,7 @@ export type EnvSwitchParams = { * Ephemeral panels (file-editors, diffs, commit-details, etc.) are env-scoped * and never carry across switches. When `keepSessionId` is provided, chat * panels for any other session are also removed so the old env's session tab - * doesn't bleed into the new env. Pulled out so `computeSurvivingIndex` can - * reuse the same survival rules without duplicating them. + * doesn't bleed into the new env. */ function shouldRemoveDuringSwitch( panel: { id: string; api: { component: string } }, @@ -237,26 +236,6 @@ function replaceStaleSessionPanels(api: DockviewApi, keepSessionId: string | nul } } -/** - * Given the panels of a group and the id of the panel being replaced, return - * the target tab index for the replacement among the siblings that will - * survive `removeEphemeralPanels`. Returns -1 if the panel isn't in the group. - */ -function computeSurvivingIndex( - groupPanels: readonly { id: string; api: { component: string } }[], - outgoingPanelId: string | undefined, - keepSessionId: string | null, -): number { - if (!outgoingPanelId) return -1; - const idx = groupPanels.findIndex((p) => p.id === outgoingPanelId); - if (idx < 0) return -1; - let count = 0; - for (let i = 0; i < idx; i++) { - if (!shouldRemoveDuringSwitch(groupPanels[i], keepSessionId)) count++; - } - return count; -} - /** * Fast path: check if we can skip `api.fromJSON()` because the layout * structure hasn't changed. Returns group IDs if the fast path was taken, @@ -308,18 +287,31 @@ function tryFastEnvSwitch(params: EnvSwitchParams): LayoutGroupIds | null { api.panels.find((p) => isSessionPanel(p) && p.api.isActive) ?? api.panels.find(isSessionPanel); const outgoingGroup = outgoingSessionPanel?.group; const outgoingGroupId = outgoingGroup?.id; - // Capture the session's index among siblings that will survive - // `removeEphemeralPanels`, so the new session panel lands in the same tab - // slot. Without this, dockview appends and the agent tab drifts to the end - // of the group on every cross-task fast-path switch. - const outgoingIndex = outgoingGroup - ? computeSurvivingIndex(outgoingGroup.panels, outgoingSessionPanel?.id, activeSessionId) - : -1; + // The outgoing session panel's current index in its group. We insert the + // incoming session at this same slot *before* removing the outgoing one, so + // removing the panels ahead of it shifts the new panel into the right final + // position (equivalent to the old "survivor index" math, minus the group + // death described below). This equivalence relies on every panel that + // `removeEphemeralPanels` removes *ahead of* the insert position being + // ephemeral/stale (they'd have been excluded by the old survivor count too). + // If `shouldRemoveDuringSwitch` ever retains a non-stale panel before the + // outgoing slot, this raw index would land the new panel off by one. + const outgoingIndex = + outgoingGroup && outgoingSessionPanel + ? outgoingGroup.panels.findIndex((p) => p.id === outgoingSessionPanel.id) + : -1; - removeEphemeralPanels(api, activeSessionId); + // Add the incoming session panel BEFORE removing the outgoing chat. Removing + // first can leave the outgoing group empty, at which point dockview destroys + // it: `outgoingGroupId` no longer exists, and post-#1165 the old + // `referenceGroup: "sidebar"` fallback is dead, so `addPanel` runs with an + // undefined position and dockview drops the incoming chat into whatever group + // is active (e.g. the terminal) — collapsing the grid root to a vertical + // stack. Adding first keeps the group alive throughout the swap. if (activeSessionId && !api.getPanel(`session:${activeSessionId}`)) { addIncomingSessionPanel(api, activeSessionId, outgoingGroupId, outgoingIndex); } + removeEphemeralPanels(api, activeSessionId); // The fast path skips `fromJSON`, so per-group active tabs from the // outgoing env would otherwise persist into the incoming env. Reapply