fix(desktop): solve #5989 — "Remove from sidebar" wipes the workspace pane layout and orphans its agents

GI
1mo ago
+151-6

Root cause

tombstoneSidebarWorkspaceRecord (apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.ts) hid the workspace and overwrote its layout:

draft.sidebarState.isHidden = true;
draft.paneLayout = createEmptyPaneLayout();   // every tab and split, gone

The re-add path, ensureSidebarWorkspaceRecord, only flips isHidden back to false — it has no branch that rebuilds a layout — so the wipe was unrecoverable. And because terminalRuntimeRegistry.release() deliberately does not kill the PTY, terminals whose panes had just been discarded kept running with nothing in the app referencing them.

Both symptoms came from that one line. The same function backs project removal, so "Remove project from sidebar" wiped every worktree's layout too.

The fix

Drop the paneLayout wipe. Hiding is now what its own doc comment claims: a sidebar-placement change.

Pane runtimes are still released on hide — nothing renders a hidden workspace, so keeping xterm views attached and WebSockets open would be pure waste — but release() persists the terminal's buffer and leaves the host-service session alive, and the WebSocket route only ever attaches to an existing session. So re-adding the workspace remounts the same panes from the preserved layout and reattaches to the still-running agents.

Nothing else depended on the layout being emptied on hide:

  • useVisibleSidebarWorkspaceIds excludes hidden rows, so notifications (V2NotificationController) and the ports list stay filtered.
  • ResourceConsumption filters hidden rows via getVisibleSidebarWorkspaces.
  • useGlobalBrowserLifecycle is a GC for panes absent from any persisted layout; a preserved layout simply means it no longer treats those webviews as removed, and the explicit browserRuntimeRegistry.destroy() on hide still runs.
  • removeWorkspaceFromSidebar (the post-delete cleanup path, used when the workspace really is deleted) is untouched.

Tests

sidebarMutations.test.ts, all three failing before the change and passing after:

  • tombstoneSidebarWorkspaceRecord > keeps the row's pane layout — hides a row holding two tabs, one split across two terminal panes, and asserts the layout survives. Before: {version: 1, tabs: [], activeTabId: null}.
  • removeProjectFromSidebarState > keeps the pane layout of each tombstoned worktree — same guarantee via the project-removal path.
  • hide → re-add round trip — the reported journey end to end: hide, then ensureSidebarWorkspaceRecord, and assert the workspace comes back visible with its tabs, splits and terminal ids intact.

ensureSidebarWorkspaceRecord is now exported so the round trip can be tested through the real re-add path rather than a stand-in.

Ran: bun test src/renderer/routes/_authenticated (10/10 in this file; the 2 remaining React.forwardRef is not a function module-load errors elsewhere in that directory are pre-existing — 4 of them on main), tsc --noEmit clean, bun run lint clean.

Not included

The reporter's suggestions (2)–(4) — a confirm dialog / undo toast, at-stake counts, moving the affordance into the context menu — are deliberately left out. With the layout preserved the action is no longer destructive, so gating it is a UX call rather than a data-loss fix. Worth deciding separately.

Closes #5989

Open in Stage

Summary by cubic

Fixes the “Remove from sidebar” action wiping workspace/project pane layouts and orphaning agents by making hide a pure visibility change that preserves layout and restores it on re-add. Fixes #5989.

  • Bug Fixes
    • Preserve paneLayout when hiding a workspace or project; only set isHidden and clear pin so tabs, splits, and terminal ids return on re-add.
    • Release pane runtimes on hide to free UI resources without killing sessions; re-adding reattaches to the same running agents.
    • Added tests for layout preservation and the hide → re-add flow; exported ensureSidebarWorkspaceRecord to cover the real path.