fix(desktop): make the terminal toggle work wherever the terminal sits

Dragging the terminal to the bottom stacks logs into its zone, and every tool-panel toggle broke in that stack. Boot revealed instead of leaving the tree alone: bindPaneCollapse ran setPaneCollapsed(id, !open), and `false` there fronts the pane. Logs binds last, so it stole the active tab from the persisted tree. Ctrl-` then asked to collapse a terminal that was no longer the active tab, the shared-zone branch declined by design, and the key did nothing until the stack was broken up. Boot now only ever collapses. The toggles also asked the wrong question. !$terminalTakeover.get() flips a boolean that has no idea what is on screen, so once anything else moved the pane the press spent itself re-asserting a value the store already held. toggleToolPane derives from the tree instead, and ctrl-`, Cmd+J's terminal fallback, the statusbar button and the logs palette row all route through it. The terminal cycle/close keys drop the same stale boolean. bindPaneCollapse moves into the tree store as bindToolPaneCollapse so the boot rule is testable against the real function rather than a copy.
2026-07-31 23:48:56 -05:00
parent 8555c6d67a
commit de46fb0365
4 changed files with 147 additions and 68 deletions
--- a/apps/desktop/src/app/contrib/controller.tsx
+++ b/apps/desktop/src/app/contrib/controller.tsx
@@ -13,11 +13,12 @@ import { LayoutTreeRoot } from '@/components/pane-shell/tree/renderer'
 import type { DoubleTapContext } from '@/components/pane-shell/tree/renderer/drag-session'
 import {
  $layoutTree,
+  bindToolPaneCollapse,
  bindTreeSideVisibility,
  declareDefaultTree,
  dismissTreePane,
  dockPaneBeside,
-  markCollapsePane,
+  isToolPaneVisible,
  mirrorLayoutTree,
  paneRootSide,
  registerLayoutResetHandler,
@@ -25,8 +26,8 @@ import {
  registerPaneOpener,
  resetLayoutTree,
  revealTreePane,
-  setPaneCollapsed,
  setTreePaneHidden,
+  toggleToolPane,
  watchContributedPanes
 } from '@/components/pane-shell/tree/store'
 import { SidebarProvider } from '@/components/ui/sidebar'
@@ -175,7 +176,11 @@ registry.registerMany([
    // staying collapsed behind the ⌃` toggle. height sizes the fixed track (a
    // single-pane zone declaring a height is a fixed track — the preset weight
    // is moot): a short deck, not a third of the window.
-    data: { placement: 'bottom', height: '20vh', minHeight: '7.5rem', maxHeight: '80vh', revealOnPreset: true },
+    //
+    // NO minHeight: a tool panel drags all the way down to its collapsed
+    // header (the sash floors it at COLLAPSED_ZONE_PX and folds the zone to
+    // its rail there). A real floor left a sliver of unusable terminal.
+    data: { placement: 'bottom', height: '20vh', maxHeight: '80vh', revealOnPreset: true },
    render: () => <WiredPane part="terminal" />
  },
  {
@@ -236,7 +241,8 @@ registry.registerMany([
    title: 'logs',
    // revealOnPreset: the Quad layout places logs, so applying it turns the
    // logs pane on (like a ⌘K "Toggle logs") instead of leaving it collapsed.
-    data: { placement: 'bottom', height: '20vh', minHeight: '7.5rem', maxHeight: '80vh', revealOnPreset: true },
+    // No minHeight — same tool-panel rule as the terminal above.
+    data: { placement: 'bottom', height: '20vh', maxHeight: '80vh', revealOnPreset: true },
    render: () => idle(<LogsPane />)
  }
 ])
@@ -488,27 +494,9 @@ function bindPaneVisibility(
  }
 }

-// TOOL PANELS (terminal, logs): like bindPaneVisibility but the toggle COLLAPSES
-// the zone to a persistent rail (tab stays) instead of hiding it — the
-// IntelliJ/VS-Code tool-window model. Restore routes back through `open` (rail
-// click / chevron) so ⌃`/the button stay truthful; Close removes the tab.
-//
-// OPEN goes through revealTreePane, not setPaneCollapsed: Close DISMISSES the
-// pane, and setPaneCollapsed can't act on a pane that has left the tree — the
-// toggle would flip its store with nothing coming back. revealTreePane
-// un-dismisses and re-adopts.
-function bindPaneCollapse(
-  paneId: string,
-  $open: { get(): boolean; listen(fn: (open: boolean) => void): void },
-  close: () => void,
-  open: () => void
-) {
-  markCollapsePane(paneId)
-  setPaneCollapsed(paneId, !$open.get())
-  $open.listen(isOpen => (isOpen ? revealTreePane(paneId) : setPaneCollapsed(paneId, true)))
-  registerPaneCloser(paneId, close)
-  registerPaneOpener(paneId, open)
-}
+// TOOL PANELS (terminal, logs): the binding lives in the tree store —
+// bindToolPaneCollapse — so the boot rule it encodes is testable against the
+// real function instead of a copy. See its docblock for the semantics.

 // SIDES have one source of truth: the TREE. The legacy $panesFlipped flag is
 // DERIVED from where the sessions zone actually sits (TitlebarControls maps
@@ -573,7 +561,7 @@ bindPaneVisibility(
 )
 // ⌃` / statusbar toggle — the terminal COLLAPSES to a rail (tab stays), not
 // hides; PTYs stay alive while collapsed (see PersistentTerminal).
-bindPaneCollapse(
+bindToolPaneCollapse(
  'terminal',
  $terminalTakeover,
  () => setTerminalTakeover(false),
@@ -591,7 +579,7 @@ bindPaneVisibility('preview', $previewVisible, closeRightRail)
 // Logs are optional chrome: off by default, toggled from ⌘K, persisted.
 const $logsOpen = persistentAtom('hermes.desktop.logsOpen', false, Codecs.bool)

-bindPaneCollapse(
+bindToolPaneCollapse(
  'logs',
  $logsOpen,
  () => $logsOpen.set(false),
@@ -603,8 +591,10 @@ registry.register(
    label: 'Toggle logs',
    icon: FileText,
    keywords: ['logs', 'agent log', 'tail', 'debug'],
-    get: () => $logsOpen.get(),
-    set: enabled => $logsOpen.set(enabled)
+    // On-screen, not the store's boolean: logs stacks with the terminal, and
+    // behind its sibling's tab `$logsOpen` stays true while nothing is visible.
+    get: () => isToolPaneVisible('logs'),
+    set: () => toggleToolPane('logs')
  })
 )

--- a/apps/desktop/src/app/hooks/use-keybinds.ts
+++ b/apps/desktop/src/app/hooks/use-keybinds.ts
@@ -2,9 +2,15 @@ import { useEffect, useRef } from 'react'
 import { useNavigate } from 'react-router'

 import { closeActiveTab } from '@/app/chat/close-tab'
-import { $terminalTakeover, setTerminalTakeover } from '@/app/right-sidebar/store'
+import { setTerminalTakeover } from '@/app/right-sidebar/store'
 import { closeActiveTerminal, createTerminal, cycleTerminal } from '@/app/right-sidebar/terminal/terminals'
-import { activateTreeTabSlot, cycleTreeTabInFocusedZone, layoutHasRootSide } from '@/components/pane-shell/tree/store'
+import {
+  activateTreeTabSlot,
+  cycleTreeTabInFocusedZone,
+  isToolPaneVisible,
+  layoutHasRootSide,
+  toggleToolPane
+} from '@/components/pane-shell/tree/store'
 import { onReleaseTypingFocus } from '@/components/ui/keyboard-first'
 import { findBarClaimsCombo } from '@/lib/find-in-page'
 import { contributedKeybindHandler, PROFILE_SLOT_COUNT, SESSION_SLOT_COUNT } from '@/lib/keybinds/actions'
@@ -184,22 +190,23 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void {
    // terminal-on-bottom) would leave it a dead key, so it falls back to the
    // terminal there. The single "secondary panel" toggle.
    'view.toggleRightSidebar': () =>
-      layoutHasRootSide('right') ? toggleFileBrowserOpen() : setTerminalTakeover(!$terminalTakeover.get()),
+      layoutHasRootSide('right') ? toggleFileBrowserOpen() : toggleToolPane('terminal'),
    'view.toggleReview': toggleReview,
    'view.toggleStatusbar': toggleStatusbarVisible,
    'view.showFiles': showFiles,
-    'view.showTerminal': () => setTerminalTakeover(!$terminalTakeover.get()),
+    'view.showTerminal': () => toggleToolPane('terminal'),
    // Create first so the pane's open-effect ensure sees a non-empty set and
    // doesn't also spawn one — net effect is exactly one fresh terminal.
    'view.newTerminal': () => {
      createTerminal()
      setTerminalTakeover(true)
    },
-    // Switch / close only act while the pane is open (no focus-scoping here, so
-    // this stands in for "terminal is showing").
-    'view.nextTerminal': () => $terminalTakeover.get() && cycleTerminal(1),
-    'view.prevTerminal': () => $terminalTakeover.get() && cycleTerminal(-1),
-    'view.closeTerminal': () => $terminalTakeover.get() && closeActiveTerminal(),
+    // Switch / close only act while the terminal is actually ON SCREEN — ask
+    // the tree, not the toggle store (which stays true behind a stacked
+    // sibling tab or a minimized zone).
+    'view.nextTerminal': () => isToolPaneVisible('terminal') && cycleTerminal(1),
+    'view.prevTerminal': () => isToolPaneVisible('terminal') && cycleTerminal(-1),
+    'view.closeTerminal': () => isToolPaneVisible('terminal') && closeActiveTerminal(),
    'view.flipPanes': togglePanesFlipped,
    // ⌘W: close the focused tab (terminal / preview target / zone tree tab).
    // On the main tab with session tabs stacked, it shifts the next one in —
--- a/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx
+++ b/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx
@@ -2,10 +2,10 @@ import { useStore } from '@nanostores/react'
 import { useCallback, useMemo } from 'react'

 import type { CommandCenterSection } from '@/app/command-center'
-import { $terminalTakeover, setTerminalTakeover } from '@/app/right-sidebar/store'
 import { useApprovalModeStatusbarItem } from '@/app/shell/approval-mode-menu'
 import { ContextUsagePanel } from '@/app/shell/context-usage-panel'
 import { GatewayMenuPanel } from '@/app/shell/gateway-menu-panel'
+import { $toolPaneVisible, toggleToolPane } from '@/components/pane-shell/tree/store'
 import { Codicon } from '@/components/ui/codicon'
 import { GlyphSpinner } from '@/components/ui/glyph-spinner'
 import { useI18n } from '@/i18n'
@@ -92,7 +92,10 @@ export function useStatusbarItems({
  const fileMenu = t.fileMenu
  const primaryActiveSessionId = useStore($activeSessionId)
  const activeGatewayProfile = useStore($activeGatewayProfile)
-  const terminalTakeover = useStore($terminalTakeover)
+  // What the button paints and flips is whether the terminal is ON SCREEN —
+  // the takeover store alone stays true behind a stacked sibling tab or a
+  // minimized zone, which lit the button for a pane the user couldn't see.
+  const terminalShowing = useStore($toolPaneVisible('terminal'))
  const primaryBusy = useStore($busy)
  const currentCwd = useStore($currentCwd)
  // Derive the workspace's project name from the already-cached project tree
@@ -506,12 +509,12 @@ export function useStatusbarItems({
      },
      {
        actionId: 'view.showTerminal',
-        className: `w-7 justify-center px-0${terminalTakeover ? ' bg-accent/55 text-foreground' : ''}`,
+        className: `w-7 justify-center px-0${terminalShowing ? ' bg-accent/55 text-foreground' : ''}`,
        hidden: !chatOpen,
        icon: <Terminal className="size-3.5" />,
        id: 'terminal',
-        onSelect: () => setTerminalTakeover(!$terminalTakeover.get()),
-        title: terminalTakeover ? copy.hideTerminal : copy.showTerminal,
+        onSelect: () => toggleToolPane('terminal'),
+        title: terminalShowing ? copy.hideTerminal : copy.showTerminal,
        toggleLabel: copy.toggleTerminal,
        variant: 'action'
      },
@@ -533,7 +536,7 @@ export function useStatusbarItems({
      requestGateway,
      sessionStartedAt,
      gatewayState,
-      terminalTakeover,
+      terminalShowing,
      turnStartedAt
    ]
  )
--- a/apps/desktop/src/components/pane-shell/tree/store.ts
+++ b/apps/desktop/src/components/pane-shell/tree/store.ts
@@ -4,7 +4,7 @@
 * the persisted tree is the user's customization; reset returns to default.
 */

-import { atom, computed } from 'nanostores'
+import { atom, computed, type ReadableAtom } from 'nanostores'

 import { SIDEBAR_COLLAPSE_MEDIA_QUERY } from '@/app/layout-constants'
 import { setPluginEnabled } from '@/contrib/plugins-store'
@@ -1050,16 +1050,27 @@ function adoptContributedPanes(): void {
    const target = findGroupOfPane(next, anchor ?? '')?.id

    if (target) {
+      // Whether the DESTINATION zone's header was explicitly hidden, read
+      // BEFORE the insert — `insertAtGroup` pins `headerHidden: false` on a
+      // center drop (a stack you can't see is a trap), which is right for a
+      // drag but wrong for adoption into a zone whose bar the user hid.
+      const hostHeaderHidden = findGroup(next, target)?.headerHidden === true
+
      // Silent adoption: don't front over the zone's active tab — a reveal does.
      next = insertAtGroup(next, target, pane.id, dock?.pos ?? 'center', dock?.before, false) ?? next

      // An adopted pane ARRIVES with its chip showing — a surprise zone with
      // zero chrome has no obvious handle to drag or close. (Explicit reveal;
      // the next structural op returns lone panes to the auto-hide default.)
+      //
+      // EXCEPT into a zone whose header the user explicitly hid: that's a
+      // standing preference about the zone, not a stale default. Without this
+      // the bar came back every time a tool panel was closed and toggled on
+      // again — Close dismisses the pane, the toggle re-adopts it through here.
      const landed = findGroupOfPane(next, pane.id)

      if (landed) {
-        next = setGroupHeaderHiddenOp(next, landed.id, false)
+        next = setGroupHeaderHiddenOp(next, landed.id, hostHeaderHidden)
      }
    }
  }
@@ -1315,34 +1326,102 @@ export function setPaneCollapsed(paneId: string, collapsed: boolean) {
 }

 /** Restore a minimized tool pane the truthful way — through its store opener
- *  when bound (keeps ⌃`/titlebar toggles in sync), else just un-minimize +
- *  front. Used by the rail (tab / whole-rail click) and the header chevron. */
+ *  when bound (keeps ⌃`/titlebar toggles in sync), then reveal regardless.
+ *  Used by the rail (tab / whole-rail click), the header chevron, and ⌃`.
+ *
+ *  The opener is fire-and-forget because it may be a NO-OP: the store can
+ *  already be `true` while the pane is off screen (the zone was minimized from
+ *  the zone menu, the tab was closed with ⌘W, or a stacked sibling holds the
+ *  active slot). nanostores don't fire listeners on a same-value `.set()`, so
+ *  the bindPaneCollapse listener never runs. `revealTreePane` is idempotent and
+ *  does the real work — un-dismiss, un-collapse the side, un-minimize, front. */
 export function restoreTreePane(paneId: string) {
-  const open = paneOpeners[paneId]
+  paneOpeners[paneId]?.()
+  revealTreePane(paneId)
+}

-  if (open) {
-    open()
-
-    // The opener may be a no-op — the store was already true (zone minimized
-    // via the zone menu, not the toggle). nanostores don't fire listeners on
-    // a same-value .set(), so the bindPaneCollapse listener never runs and
-    // the zone stays minimized. Un-minimize directly when that happens.
-    const group = paneGroup(paneId)
-
-    if (group?.minimized) {
-      setTreeGroupMinimized(group.id, false)
-    }
-
-    revealTreePane(paneId)
-
-    return
+/** Is a tool pane actually ON SCREEN? In the tree, not dismissed or chrome
+ *  hidden, its zone un-minimized, and holding its stack's active slot. */
+export function isToolPaneVisible(paneId: string): boolean {
+  if ($dismissedPanes.get().has(paneId) || $hiddenTreePanes.get().has(paneId)) {
+    return false
  }

  const group = paneGroup(paneId)

-  if (group) {
-    setTreeGroupMinimized(group.id, false)
-    activateTreePane(group.id, paneId)
+  return Boolean(group && !group.minimized && group.active === paneId)
+}
+
+const toolPaneVisibleCache = new Map<string, ReadableAtom<boolean>>()
+
+/** Reactive `isToolPaneVisible` for chrome that renders an on/off affordance
+ *  (the statusbar's terminal button). Memoized per pane id so `useStore`
+ *  subscriptions stay referentially stable across renders. */
+export function $toolPaneVisible(paneId: string): ReadableAtom<boolean> {
+  let cached = toolPaneVisibleCache.get(paneId)
+
+  if (!cached) {
+    cached = computed([$layoutTree, $dismissedPanes, $hiddenTreePanes], () => isToolPaneVisible(paneId))
+    toolPaneVisibleCache.set(paneId, cached)
+  }
+
+  return cached
+}
+
+/**
+ * TOOL PANELS (terminal, logs): bind a pane's visibility STORE to the tree so
+ * its toggle COLLAPSES the zone to a persistent rail (the tab stays) instead of
+ * hiding it — the IntelliJ/VS-Code tool-window model. Restore routes back
+ * through `open` (rail click / chevron) so ⌃` and the statusbar button stay
+ * truthful; Close removes the tab.
+ *
+ * OPEN goes through `revealTreePane`, not `setPaneCollapsed`: Close DISMISSES
+ * the pane, and `setPaneCollapsed` is a no-op on a pane that has left the tree,
+ * so the toggle would flip its store with nothing coming back. `revealTreePane`
+ * un-dismisses and re-adopts.
+ *
+ * BOOT ONLY COLLAPSES — it must never reveal. `setPaneCollapsed(id, false)`
+ * fronts the pane in its stack, so binding two tool panels that are both "open"
+ * let the second one steal the active tab from the persisted tree. With
+ * terminal+logs stacked (what you get by dragging the terminal to the bottom),
+ * logs bound last and won the slot; ⌃` then asked to collapse a terminal that
+ * wasn't the active tab, the shared-zone branch declined, and the key read as
+ * dead until the stack was broken up. The persisted tree already records which
+ * tab was active — leave it alone.
+ */
+export function bindToolPaneCollapse(
+  paneId: string,
+  $open: { get(): boolean; listen(fn: (open: boolean) => void): void },
+  close: () => void,
+  open: () => void
+) {
+  markCollapsePane(paneId)
+
+  if (!$open.get()) {
+    setPaneCollapsed(paneId, true)
+  }
+
+  $open.listen(isOpen => (isOpen ? revealTreePane(paneId) : setPaneCollapsed(paneId, true)))
+  registerPaneCloser(paneId, close)
+  registerPaneOpener(paneId, open)
+}
+
+/**
+ * ⌃` / the statusbar button / ⌘J's terminal fallback: ONE resolver for "flip
+ * this tool panel", derived from what is on screen rather than from the
+ * toggle's own boolean.
+ *
+ * A free-floating `!$open.get()` diverges from the tree the moment anything
+ * else moves the pane — stacked behind a sibling tab, minimized from the zone
+ * menu, closed with ⌘W — and then the toggle spends its press re-asserting a
+ * value the store already held, which reads as a dead key. Asking the tree
+ * instead means the first press always does the visible thing.
+ */
+export function toggleToolPane(paneId: string) {
+  if (isToolPaneVisible(paneId)) {
+    collapseTreePane(paneId)
+  } else {
+    restoreTreePane(paneId)
  }
 }