From ad8365d533cf02b62c350932feb2aefa96f956da Mon Sep 17 00:00:00 2001 From: unsupportedpastels Date: Fri, 14 Aug 2026 03:20:25 +0000 Subject: [PATCH] fix(desktop): preserve multi-pane plugins when closing panes Closing a pane contributed by a plugin used to disable the entire plugin, unloading every one of its contributions. For a plugin that owns several independent panes (e.g. Bot Mode's Cronjobs pane alongside its Bots roster and composer middleware), closing one pane silently killed the rest. Now: closing one pane of a multi-pane plugin dismisses only that pane; the plugin stays enabled and its other panes/commands/middleware keep working. Reset layout restores dismissed contributed panes. A single-pane plugin keeps the existing symmetric behavior (Close disables the plugin, with Settings -> Plugins as the recovery path). Adds regression coverage for both cases. --- .../pane-shell/tree/plugin-pane-close.test.ts | 71 +++++++++++++++++++ .../src/components/pane-shell/tree/store.ts | 39 +++++----- .../developer-guide/desktop-plugin-sdk.md | 6 ++ 3 files changed, 98 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/src/components/pane-shell/tree/plugin-pane-close.test.ts diff --git a/apps/desktop/src/components/pane-shell/tree/plugin-pane-close.test.ts b/apps/desktop/src/components/pane-shell/tree/plugin-pane-close.test.ts new file mode 100644 index 0000000000..3c0b0be682 --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/plugin-pane-close.test.ts @@ -0,0 +1,71 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { setPluginEnabled } from '@/contrib/plugins-store' +import { registry } from '@/contrib/registry' + +import { allPaneIds, group, split } from './model' +import { $dismissedPanes, $layoutTree, closeTreePane } from './store' + +vi.mock('@/contrib/plugins-store', () => ({ setPluginEnabled: vi.fn() })) +vi.mock('@/store/notifications', () => ({ notify: vi.fn() })) + +const disposers: (() => void)[] = [] + +function registerPluginPane(pluginId: string, paneId: string) { + disposers.push( + registry.register({ + area: 'panes', + data: { placement: 'main' }, + id: paneId, + render: () => null, + source: `plugin:${pluginId}`, + title: paneId + }) + ) +} + +beforeEach(() => { + window.localStorage.clear() + $dismissedPanes.set(new Set()) + vi.mocked(setPluginEnabled).mockReset() +}) + +afterEach(() => { + disposers.splice(0).forEach(dispose => dispose()) +}) + +describe('closing plugin panes', () => { + it('dismisses one pane without disabling a plugin that contributes multiple panes', () => { + registerPluginPane('bots', 'bots:pane') + registerPluginPane('bots', 'bots:routines') + $layoutTree.set( + split('row', [ + group(['workspace'], { active: 'workspace', id: 'g-main' }), + group(['bots:pane'], { active: 'bots:pane', id: 'g-bots' }), + group(['bots:routines'], { active: 'bots:routines', id: 'g-routines' }) + ]) + ) + + closeTreePane('bots:routines') + + expect(allPaneIds($layoutTree.get()!)).not.toContain('bots:routines') + expect(allPaneIds($layoutTree.get()!)).toContain('bots:pane') + expect($dismissedPanes.get()).toContain('bots:routines') + expect(setPluginEnabled).not.toHaveBeenCalled() + + // Any later registry mutation runs pane adoption. The dismissal must + // survive that cycle instead of immediately resurrecting Cronjobs. + registerPluginPane('other', 'other:pane') + expect(allPaneIds($layoutTree.get()!)).not.toContain('bots:routines') + }) + + it('keeps disabling a plugin whose only pane is closed', () => { + registerPluginPane('single', 'single:pane') + $layoutTree.set(group(['single:pane'], { active: 'single:pane', id: 'g-single' })) + + closeTreePane('single:pane') + + expect(setPluginEnabled).toHaveBeenCalledWith('single', false) + expect(allPaneIds($layoutTree.get()!)).toContain('single:pane') + }) +}) diff --git a/apps/desktop/src/components/pane-shell/tree/store.ts b/apps/desktop/src/components/pane-shell/tree/store.ts index dca7d28a2e..4ddbc65b4f 100644 --- a/apps/desktop/src/components/pane-shell/tree/store.ts +++ b/apps/desktop/src/components/pane-shell/tree/store.ts @@ -172,9 +172,11 @@ function frontPaneInGroup(paneId: string) { * - a registered closer (core panes whose visibility an app store owns: * review/terminal/preview/sessions) closes through that store, so the * titlebar/statusbar toggles stay truthful; - * - everything else (plugin panes, unbound core panes) is DISMISSED: removed - * from the tree and remembered so adoption doesn't re-add it. Reveal - * intent (a preview target, ⌘G) or a layout reset un-dismisses. + * - unbound core panes and panes from multi-pane plugins are DISMISSED: + * removed from the tree and remembered so adoption doesn't re-add them. + * Reveal intent (a preview target, ⌘G) or a layout reset un-dismisses; + * - closing the sole pane from a plugin disables that plugin, preserving the + * discoverable Settings → Plugins recovery path for single-pane plugins. */ const DISMISSED_KEY = 'hermes.desktop.dismissedPanes.v1' @@ -762,14 +764,24 @@ export function closeTreePane(paneId: string) { return } - // A plugin's pane: Close = DISABLE the plugin — the same switch as - // Settings → Plugins, so recovery is discoverable and symmetric. The - // contribution unregisters but the pane id STAYS in the tree, so - // re-enabling restores it exactly where it was. (Dismissal + removal - // would strand the pane with no way back short of a layout reset.) - const source = registry.getArea('panes').find(c => c.id === paneId)?.source + const panes = registry.getArea('panes') + const source = panes.find(c => c.id === paneId)?.source if (source?.startsWith('plugin:')) { + // A plugin may own several independent panes. Closing one of them must not + // unload every contribution from that plugin (for example, closing Bot + // Mode's Cronjobs pane must leave its Bots roster and composer middleware + // alive). Dismiss just that pane; Layout reset remains the explicit way to + // restore dismissed contributed panes. + if (panes.filter(c => c.source === source).length > 1) { + dismissTreePane(paneId) + + return + } + + // A single-pane plugin keeps the existing symmetric behavior: Close uses + // the same switch as Settings → Plugins. Its contribution unregisters but + // the pane id stays in the tree, so re-enabling restores its exact place. const pluginId = source.slice('plugin:'.length) void setPluginEnabled(pluginId, false) notify({ @@ -1111,15 +1123,6 @@ function adoptContributedPanes(): void { const mainId = panes.find(c => placementOf(c.id) === 'main')?.id const inTree = new Set(allPaneIds(tree)) - // Plugin panes are never dismissed anymore (Close disables the plugin - // instead) — drop stale entries so panes stranded by the old behavior - // re-adopt on their own. - for (const pane of panes) { - if (pane.source?.startsWith('plugin:') && $dismissedPanes.get().has(pane.id)) { - setDismissed(pane.id, false) - } - } - const dismissed = $dismissedPanes.get() // `placement: 'floating'` opts OUT of the tree entirely — those panes render diff --git a/website/docs/developer-guide/desktop-plugin-sdk.md b/website/docs/developer-guide/desktop-plugin-sdk.md index 6d4e88544b..73973aa24b 100644 --- a/website/docs/developer-guide/desktop-plugin-sdk.md +++ b/website/docs/developer-guide/desktop-plugin-sdk.md @@ -246,6 +246,12 @@ data: { `'top' | 'bottom' | 'left' | 'right' | 'center'`. Declare a `width`/`height` so the pane doesn't claim half the zone. +Closing the only pane contributed by a plugin disables that plugin, which can +be re-enabled from **Settings → Plugins**. When a plugin contributes multiple +panes, closing one dismisses only that pane and leaves the plugin's other panes, +commands, and middleware active. **Reset layout** restores dismissed contributed +panes. + ### Pages and sidebar nav A route mounts a full page in the workspace pane, like any built-in view. Pair it