From 5a2c504b9e789aa6b0ecb09493ac60f842c349b4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Jul 2026 14:53:57 +0000 Subject: [PATCH] fix(desktop): keep the pane layout when hiding a workspace from the sidebar "Remove from sidebar" replaced the workspace's paneLayout with an empty one, so re-adding the workspace brought it back with no tabs or splits. Releasing a terminal runtime does not kill its PTY, so any agents running in those panes stayed alive with nothing referencing them. Tombstoning now only changes sidebar placement. The runtimes are still released (nothing renders a hidden workspace), but the layout stays on the row, so re-adding remounts the same panes and reattaches to the still running terminal sessions. Refs #5989 --- .../RemoveFromSidebarMount.tsx | 5 +- .../sidebarMutations.test.ts | 134 +++++++++++++++++- .../sidebarMutations.ts | 10 +- .../useDashboardSidebarState.ts | 8 +- 4 files changed, 151 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/renderer/commandPalette/ui/RemoveFromSidebarMount/RemoveFromSidebarMount.tsx b/apps/desktop/src/renderer/commandPalette/ui/RemoveFromSidebarMount/RemoveFromSidebarMount.tsx index 0d704aa61c5..ba255802023 100644 --- a/apps/desktop/src/renderer/commandPalette/ui/RemoveFromSidebarMount/RemoveFromSidebarMount.tsx +++ b/apps/desktop/src/renderer/commandPalette/ui/RemoveFromSidebarMount/RemoveFromSidebarMount.tsx @@ -5,8 +5,9 @@ import { useRemoveFromSidebarIntent } from "renderer/stores/remove-workspace-fro /** * Headless effect that unpins a workspace from the sidebar as soon as a - * removal is requested. Unpinning is reversible — the workspace itself is not - * deleted and can be re-pinned from the Workspaces page or sidebar — so it runs + * removal is requested. Unpinning is reversible — the workspace is not deleted, + * its pane layout is kept on the hidden row, and re-pinning it from the + * Workspaces page or sidebar brings back its tabs and splits — so it runs * immediately with no confirmation dialog. Lives here (rather than at each call * site) because callers like the command palette fire imperatively and can't * use the router/collections hooks the removal needs. diff --git a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.test.ts b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.test.ts index 02abd24f2c3..1e12ae8f445 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.test.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.test.ts @@ -4,6 +4,7 @@ import { type SidebarWorkspaceRow, tombstoneSidebarWorkspaceRecord, } from "./sidebarMutations"; +import { ensureSidebarWorkspaceRecord } from "./useDashboardSidebarState"; /** * Minimal in-memory stand-in for a TanStack DB collection, implementing only @@ -33,6 +34,12 @@ function makeCollection(getKey: (item: T) => string) { }; } +type PaneLayout = { + version: number; + tabs: unknown[]; + activeTabId: string | null; +}; + type LocalStateRow = { workspaceId: string; createdAt: Date; @@ -43,13 +50,56 @@ type LocalStateRow = { isHidden: boolean; pinnedAt: number | null; }; - paneLayout: { version: number; tabs: unknown[]; activeTabId: string | null }; + paneLayout: PaneLayout; }; +/** A layout with real content: two tabs, one of them split across two panes. */ +function populatedPaneLayout(workspaceId: string): PaneLayout { + return { + version: 1, + tabs: [ + { + id: `${workspaceId}-tab-1`, + title: "agent", + panes: { + "pane-1": { + id: "pane-1", + kind: "terminal", + data: { terminalId: `${workspaceId}-term-1` }, + }, + "pane-2": { + id: "pane-2", + kind: "terminal", + data: { terminalId: `${workspaceId}-term-2` }, + }, + }, + layout: { + type: "split", + direction: "row", + first: { type: "pane", paneId: "pane-1" }, + second: { type: "pane", paneId: "pane-2" }, + }, + activePaneId: "pane-1", + }, + { + id: `${workspaceId}-tab-2`, + title: "docs", + panes: { + "pane-3": { id: "pane-3", kind: "browser", data: {} }, + }, + layout: { type: "pane", paneId: "pane-3" }, + activePaneId: "pane-3", + }, + ], + activeTabId: `${workspaceId}-tab-1`, + }; +} + function localStateRow( workspaceId: string, projectId: string, overrides: Partial = {}, + paneLayout: PaneLayout = { version: 1, tabs: [], activeTabId: null }, ): LocalStateRow { return { workspaceId, @@ -62,7 +112,7 @@ function localStateRow( pinnedAt: null, ...overrides, }, - paneLayout: { version: 1, tabs: [], activeTabId: null }, + paneLayout, }; } @@ -95,6 +145,11 @@ function asTombstoneArg(collections: Collections) { typeof tombstoneSidebarWorkspaceRecord >[0]; } +function asEnsureArg(collections: Collections) { + return collections as unknown as Parameters< + typeof ensureSidebarWorkspaceRecord + >[0]; +} const noopCleanup = () => {}; @@ -212,6 +267,35 @@ describe("removeProjectFromSidebarState", () => { expect(row?.sidebarState.isHidden).toBe(false); }); + it("keeps the pane layout of each tombstoned worktree so re-adding the project restores them", () => { + const collections = makeCollections(); + const layout = populatedPaneLayout("ws-placed"); + collections.v2WorkspaceLocalState.insert( + localStateRow("ws-placed", "proj-1", {}, layout), + ); + const workspaces: SidebarWorkspaceRow[] = [ + { + id: "ws-placed", + projectId: "proj-1", + hostId: "machine-1", + type: "worktree", + }, + ]; + collections.v2SidebarProjects.insert({ projectId: "proj-1" }); + + removeProjectFromSidebarState( + asRemoveArg(collections), + workspaces, + "proj-1", + "machine-1", + noopCleanup, + ); + + const row = collections.v2WorkspaceLocalState.get("ws-placed"); + expect(row?.sidebarState.isHidden).toBe(true); + expect(row?.paneLayout).toEqual(layout); + }); + it("leaves workspaces from other projects untouched", () => { const collections = makeCollections(); collections.v2WorkspaceLocalState.insert( @@ -312,4 +396,50 @@ describe("tombstoneSidebarWorkspaceRecord", () => { expect(row?.sidebarState.pinnedAt).toBeNull(); expect(cleaned).toEqual(["ws-1"]); }); + + it("keeps the row's pane layout — hiding is a visibility change, not a reset", () => { + // Hiding is advertised as reversible, and the re-add path only flips + // `isHidden` back to false: dropping paneLayout here loses every tab and + // split with no way to get them back. + const collections = makeCollections(); + const layout = populatedPaneLayout("ws-1"); + collections.v2WorkspaceLocalState.insert( + localStateRow("ws-1", "proj-1", {}, layout), + ); + + tombstoneSidebarWorkspaceRecord( + asTombstoneArg(collections), + "ws-1", + "proj-1", + noopCleanup, + ); + + expect(collections.v2WorkspaceLocalState.get("ws-1")?.paneLayout).toEqual( + layout, + ); + }); +}); + +describe("hide → re-add round trip", () => { + it("brings the workspace back with its tabs, splits and terminal ids intact", () => { + // The journey from the bug report: click "Remove from sidebar", then add + // the workspace back. It must return with its work, not empty. + const collections = makeCollections(); + const layout = populatedPaneLayout("ws-1"); + collections.v2WorkspaceLocalState.insert( + localStateRow("ws-1", "proj-1", {}, layout), + ); + + tombstoneSidebarWorkspaceRecord( + asTombstoneArg(collections), + "ws-1", + "proj-1", + noopCleanup, + ); + ensureSidebarWorkspaceRecord(asEnsureArg(collections), "ws-1", "proj-1"); + + const row = collections.v2WorkspaceLocalState.get("ws-1"); + expect(row?.sidebarState.isHidden).toBe(false); + expect(row?.paneLayout).toEqual(layout); + }); }); diff --git a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.ts b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.ts index 5f466c2f238..d2d38d209cd 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.ts @@ -30,6 +30,9 @@ type CleanupPaneRuntimes = (rows: PaneLifecycleRow[]) => void; * with no local-state row is re-surfaced by the gated auto-include path, so * hiding one requires a row (`isHidden: true`) to suppress it; a hard-delete * would let it reappear. + * + * Hiding is a visibility change only: the row keeps its `paneLayout`, so + * re-adding the workspace restores its tabs and splits. */ export function tombstoneSidebarWorkspaceRecord( collections: Pick, @@ -53,6 +56,12 @@ export function tombstoneSidebarWorkspaceRecord( return; } + // Runtimes are released — xterm views detached, WebSockets closed, webviews + // destroyed — because nothing renders a hidden workspace. `paneLayout` is + // deliberately kept: it is the only record of this workspace's tabs, splits + // and terminal ids, and releasing a terminal runtime does not kill its PTY. + // Keeping it is what makes hiding reversible — re-adding the workspace + // remounts the same panes and reattaches to the still-running sessions. cleanupPaneRuntimes([existing]); collections.v2WorkspaceLocalState.update(workspaceId, (draft) => { draft.sidebarState.projectId = projectId; @@ -61,7 +70,6 @@ export function tombstoneSidebarWorkspaceRecord( // A row must never be hidden and pinned at once — a resurrected // workspace would otherwise reappear pre-pinned. draft.sidebarState.pinnedAt = null; - draft.paneLayout = createEmptyPaneLayout(); }); } diff --git a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts index a51ab7c08c1..eb5ecf92356 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts @@ -128,7 +128,13 @@ function ensureSidebarProjectRecord( }); } -function ensureSidebarWorkspaceRecord( +/** + * Re-surfaces a workspace in the sidebar. A hidden ("tombstoned") row is only + * un-hidden and re-placed — `paneLayout` is left alone, which is what makes + * hiding reversible: the tabs and splits the row still carries come back with + * it. + */ +export function ensureSidebarWorkspaceRecord( collections: Pick< AppCollections, "v2SidebarSections" | "v2WorkspaceLocalState"