Repository navigation
fix(desktop): solve #5989 — "Remove from sidebar" wipes the workspace pane layout and orphans its agents - #5991
Open
github-actions[bot] wants to merge 1 commit into
Open
github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
…idebar "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
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you:
Chapters generated by Stage for commit 5a2c504 on Jul 27, 2026 2:54pm UTC. |
Contributor
Author
|
This PR has had no activity for 53 days and will close in 7 days unless there is new activity. If this contribution is still important or relevant, please leave a comment to keep it open. If it closes, you are welcome to reopen it or ask a maintainer to reopen it. Thank you for contributing! |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
tombstoneSidebarWorkspaceRecord(apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/sidebarMutations.ts) hid the workspace and overwrote its layout:The re-add path,
ensureSidebarWorkspaceRecord, only flipsisHiddenback tofalse— it has no branch that rebuilds a layout — so the wipe was unrecoverable. And becauseterminalRuntimeRegistry.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
paneLayoutwipe. 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:
useVisibleSidebarWorkspaceIdsexcludes hidden rows, so notifications (V2NotificationController) and the ports list stay filtered.ResourceConsumptionfilters hidden rows viagetVisibleSidebarWorkspaces.useGlobalBrowserLifecycleis a GC for panes absent from any persisted layout; a preserved layout simply means it no longer treats those webviews as removed, and the explicitbrowserRuntimeRegistry.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.ensureSidebarWorkspaceRecord, and assert the workspace comes back visible with its tabs, splits and terminal ids intact.ensureSidebarWorkspaceRecordis 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 remainingReact.forwardRef is not a functionmodule-load errors elsewhere in that directory are pre-existing — 4 of them onmain),tsc --noEmitclean,bun run lintclean.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
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.
paneLayoutwhen hiding a workspace or project; only setisHiddenand clear pin so tabs, splits, and terminal ids return on re-add.ensureSidebarWorkspaceRecordto cover the real path.Written for commit 5a2c504. Summary will update on new commits.