fix(desktop): toggle every pane off the tree, not off its own boolean

The terminal fix was only one instance. An audit of the other pane toggles
found ⌘G and ⌘J diverging the same way, proven with a probe: with review
stacked behind files in the right column, or either pane inside a minimized
zone, the store reads open while nothing is on screen, so the press
re-asserts a value it already held and the key does nothing.

isPaneVisible / togglePaneVisible replace the tool-panel-only pair and now
back every toggle. Close still routes through closeTreePane, so each pane
keeps its own semantics: a tool panel collapses to its rail, files and
review close through their store, anything else is dismissed.

files and review were bound with a closer and no opener, so the boolean went
stale as soon as anything but the toggle revealed them. bindPaneVisibility
moves into the tree store beside bindToolPaneCollapse, documents the two as a
pair, and both panes now pass both halves. Keeping the binding in the store
also means the tests drive the real function — the earlier copy in the test
file passed with the fix reverted, which is how the missing opener survived
the first pass.

setTreePaneHidden keeps its quiet path: a reactive unhide (a cwd arriving)
must not front or un-minimize over what the user is looking at. Only user
intent goes through the reveal path.
This commit is contained in:
Brooklyn Nicholson
2026-08-01 01:17:45 -05:00
parent 16431b8ab2
commit 73b8847d7b
8 changed files with 254 additions and 70 deletions

View File

@@ -13,21 +13,20 @@ import { LayoutTreeRoot } from '@/components/pane-shell/tree/renderer'
import type { DoubleTapContext } from '@/components/pane-shell/tree/renderer/drag-session'
import {
$layoutTree,
bindPaneVisibility,
bindToolPaneCollapse,
bindTreeSideVisibility,
declareDefaultTree,
dismissTreePane,
dockPaneBeside,
isToolPaneVisible,
isPaneVisible,
mirrorLayoutTree,
paneRootSide,
registerLayoutResetHandler,
registerPaneCloser,
registerPaneOpener,
resetLayoutTree,
revealTreePane,
setTreePaneHidden,
toggleToolPane,
togglePaneVisible,
watchContributedPanes
} from '@/components/pane-shell/tree/store'
import { SidebarProvider } from '@/components/ui/sidebar'
@@ -55,7 +54,7 @@ import {
SIDEBAR_MAX_WIDTH
} from '@/store/layout'
import { $previewOpenRequest, $previewTabs, closeRightRail } from '@/store/preview'
import { $reviewOpen, closeReview, REVIEW_PANE_ID } from '@/store/review'
import { $reviewOpen, closeReview, openReview, REVIEW_PANE_ID } from '@/store/review'
import { $currentCwd, $selectedStoredSessionId, $sessions, $yoloActive, sessionMatchesStoredId } from '@/store/session'
import { watchSessionPins } from '@/store/session-pin-sync'
import { $statusbarVisible } from '@/store/statusbar-prefs'
@@ -472,27 +471,9 @@ registerLayoutResetHandler(stackSessionTilesIntoMain)
// toggle mirrors the root row.
// ---------------------------------------------------------------------------
function bindPaneVisibility(
paneId: string,
$open: { get(): boolean; listen(fn: (open: boolean) => void): void },
close?: () => void,
open?: () => void
) {
setTreePaneHidden(paneId, !$open.get())
$open.listen(isOpen => setTreePaneHidden(paneId, !isOpen))
// The tab menu's Close routes through the owning store (never dismissal),
// so the pane's toggle buttons stay truthful.
if (close) {
registerPaneCloser(paneId, close)
}
// The opener is the mirror: preset application (revealOnPreset) shows the
// pane through the same store, so the toggle stays truthful.
if (open) {
registerPaneOpener(paneId, open)
}
}
// HIDE-STYLE PANES (files, review, preview): the binding lives in the tree
// store — bindPaneVisibility — alongside bindToolPaneCollapse, so both are
// testable against the real function instead of a copy.
// TOOL PANELS (terminal, logs): the binding lives in the tree store —
// bindToolPaneCollapse — so the boot rule it encodes is testable against the
@@ -549,15 +530,25 @@ const $hasWorkspace = computed($currentCwd, cwd => Boolean(cwd.trim()))
// The tree pane's own presence tracks ⌘J directly, not just the column's
// collapse — otherwise revealing a preview (which opens that shared column)
// would drag the tree along with it. See revealPreview.
//
// Both get a CLOSER and an OPENER. The closer keeps ⌘J/⌘G truthful when the
// pane is closed from the tab menu; the opener is its mirror, so bringing the
// pane back through the tree (the toggle's reveal path, the rail, a preset)
// writes the store too. Without the opener the boolean went stale the moment
// anything but the toggle showed the pane — the divergence this whole change
// is about.
bindPaneVisibility(
'files',
computed([$hasWorkspace, $fileBrowserOpen], (workspace, open) => workspace && open)
computed([$hasWorkspace, $fileBrowserOpen], (workspace, open) => workspace && open),
() => setFileBrowserOpen(false),
() => setFileBrowserOpen(true)
)
// ⌘G — the review sidebar appears/disappears (and comes to the front).
bindPaneVisibility(
'review',
computed([$reviewOpen, $hasWorkspace], (open, workspace) => open && workspace),
closeReview
closeReview,
openReview
)
// ⌃` / statusbar toggle — the terminal COLLAPSES to a rail (tab stays), not
// hides; PTYs stay alive while collapsed (see PersistentTerminal).
@@ -593,8 +584,8 @@ registry.register(
keywords: ['logs', 'agent log', 'tail', 'debug'],
// 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')
get: () => isPaneVisible('logs'),
set: () => togglePaneVisible('logs')
})
)

View File

@@ -7,9 +7,9 @@ import { closeActiveTerminal, createTerminal, cycleTerminal } from '@/app/right-
import {
activateTreeTabSlot,
cycleTreeTabInFocusedZone,
isToolPaneVisible,
isPaneVisible,
layoutHasRootSide,
toggleToolPane
togglePaneVisible
} from '@/components/pane-shell/tree/store'
import { onReleaseTypingFocus } from '@/components/ui/keyboard-first'
import { findBarClaimsCombo } from '@/lib/find-in-page'
@@ -190,11 +190,11 @@ 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() : toggleToolPane('terminal'),
layoutHasRootSide('right') ? toggleFileBrowserOpen() : togglePaneVisible('terminal'),
'view.toggleReview': toggleReview,
'view.toggleStatusbar': toggleStatusbarVisible,
'view.showFiles': showFiles,
'view.showTerminal': () => toggleToolPane('terminal'),
'view.showTerminal': () => togglePaneVisible('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': () => {
@@ -204,9 +204,9 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void {
// 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.nextTerminal': () => isPaneVisible('terminal') && cycleTerminal(1),
'view.prevTerminal': () => isPaneVisible('terminal') && cycleTerminal(-1),
'view.closeTerminal': () => isPaneVisible('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 —

View File

@@ -5,7 +5,7 @@ import type { CommandCenterSection } from '@/app/command-center'
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 { $paneVisible, togglePaneVisible } from '@/components/pane-shell/tree/store'
import { Codicon } from '@/components/ui/codicon'
import { GlyphSpinner } from '@/components/ui/glyph-spinner'
import { useI18n } from '@/i18n'
@@ -95,7 +95,7 @@ export function useStatusbarItems({
// 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 terminalShowing = useStore($paneVisible('terminal'))
const primaryBusy = useStore($busy)
const currentCwd = useStore($currentCwd)
// Derive the workspace's project name from the already-cached project tree
@@ -513,7 +513,7 @@ export function useStatusbarItems({
hidden: !chatOpen,
icon: <Terminal className="size-3.5" />,
id: 'terminal',
onSelect: () => toggleToolPane('terminal'),
onSelect: () => togglePaneVisible('terminal'),
title: terminalShowing ? copy.hideTerminal : copy.showTerminal,
toggleLabel: copy.toggleTerminal,
variant: 'action'

View File

@@ -0,0 +1,126 @@
import { atom } from 'nanostores'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { registry } from '@/contrib/registry'
import { group, split } from './model'
import {
$dismissedPanes,
$hiddenTreePanes,
$layoutTree,
bindPaneVisibility,
isPaneVisible,
setTreeGroupMinimized,
togglePaneVisible
} from './store'
// The bug class, across EVERY pane kind — not just the terminal she reported.
//
// A toggle that flips its own boolean diverges from the tree the moment
// anything else moves the pane: stacked behind a sibling tab, folded into a
// minimized zone, closed with ⌘W. The store then says "open" while nothing is
// on screen, the press re-asserts a value it already held, and the key reads
// as dead. Same shape for the COLLAPSE-style tool panels (terminal, logs) and
// the HIDE-style panes (files, review) — proven here with one table.
const disposers: (() => void)[] = []
/** Bind through the REAL production function, with the closer/opener pair the
* controller gives it. */
function bindVisibility(paneId: string, $open: ReturnType<typeof atom<boolean>>) {
bindPaneVisibility(
paneId,
$open,
() => $open.set(false),
() => $open.set(true)
)
}
beforeEach(() => {
window.localStorage.clear()
$dismissedPanes.set(new Set())
$hiddenTreePanes.set(new Set())
for (const [id, data] of [
['workspace', { placement: 'main', uncloseable: true }],
['files', { placement: 'right' }],
['review', { placement: 'right' }]
] as const) {
disposers.push(registry.register({ area: 'panes', data, id, render: () => null, title: id }))
}
})
afterEach(() => {
disposers.splice(0).forEach(dispose => dispose())
})
describe('a hide-style pane stacked behind a sibling tab', () => {
it('comes forward on the first press instead of swallowing it', () => {
$layoutTree.set(
split('row', [
group(['workspace'], { active: 'workspace', id: 'g-main' }),
group(['files', 'review'], { active: 'files', id: 'g-right' })
])
)
const $review = atom(true)
bindVisibility('review', $review)
// The column is open but showing FILES, so the diff is not on screen —
// and $reviewOpen is true, which is what made ⌘G a no-op.
expect(isPaneVisible('review')).toBe(false)
togglePaneVisible('review')
expect(isPaneVisible('review')).toBe(true)
})
})
describe('a hide-style pane inside a minimized zone', () => {
it('un-minimizes on the first press', () => {
$layoutTree.set(
split('row', [
group(['workspace'], { active: 'workspace', id: 'g-main' }),
group(['files'], { active: 'files', id: 'g-files' })
])
)
const $files = atom(true)
bindVisibility('files', $files)
setTreeGroupMinimized('g-files', true)
expect(isPaneVisible('files')).toBe(false)
togglePaneVisible('files')
expect(isPaneVisible('files')).toBe(true)
})
})
describe('the toggle round-trip', () => {
it('closes a visible hide-style pane through its own store', () => {
$layoutTree.set(
split('row', [
group(['workspace'], { active: 'workspace', id: 'g-main' }),
group(['files'], { active: 'files', id: 'g-files' })
])
)
const $files = atom(true)
bindVisibility('files', $files)
expect(isPaneVisible('files')).toBe(true)
togglePaneVisible('files')
// Close routes through the registered closer, so the pane's own toggle
// store stays truthful rather than being bypassed by a tree edit.
expect($files.get()).toBe(false)
expect(isPaneVisible('files')).toBe(false)
togglePaneVisible('files')
expect($files.get()).toBe(true)
expect(isPaneVisible('files')).toBe(true)
})
})

View File

@@ -1340,9 +1340,10 @@ export function restoreTreePane(paneId: string) {
revealTreePane(paneId)
}
/** 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 {
/** Is a pane actually ON SCREEN? In the tree, not dismissed, not chrome
* hidden, its zone un-minimized, and holding its stack's active slot.
* True for every pane class — tool panels and hide-style panes alike. */
export function isPaneVisible(paneId: string): boolean {
if ($dismissedPanes.get().has(paneId) || $hiddenTreePanes.get().has(paneId)) {
return false
}
@@ -1352,22 +1353,54 @@ export function isToolPaneVisible(paneId: string): boolean {
return Boolean(group && !group.minimized && group.active === paneId)
}
const toolPaneVisibleCache = new Map<string, ReadableAtom<boolean>>()
const paneVisibleCache = new Map<string, ReadableAtom<boolean>>()
/** Reactive `isToolPaneVisible` for chrome that renders an on/off affordance
/** Reactive `isPaneVisible` 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)
export function $paneVisible(paneId: string): ReadableAtom<boolean> {
let cached = paneVisibleCache.get(paneId)
if (!cached) {
cached = computed([$layoutTree, $dismissedPanes, $hiddenTreePanes], () => isToolPaneVisible(paneId))
toolPaneVisibleCache.set(paneId, cached)
cached = computed([$layoutTree, $dismissedPanes, $hiddenTreePanes], () => isPaneVisible(paneId))
paneVisibleCache.set(paneId, cached)
}
return cached
}
/**
* HIDE-STYLE PANES (files, review, preview): bind a pane's visibility STORE to
* the tree so its toggle HIDES the pane — its zone collapses while the content
* stays mounted — as opposed to the tool panels, which collapse to a rail and
* keep their tab.
*
* `close` and `open` are a PAIR, and passing only one is the bug this exists to
* prevent. The closer keeps the toggle truthful when the pane is closed from
* the tab menu; the opener is its mirror, so anything that shows the pane
* through the tree — a reveal, a preset, the toggle's own un-hide path — writes
* the store too. With a closer and no opener the boolean goes stale the moment
* something other than the toggle reveals the pane, and the next press spends
* itself re-asserting a value it already held.
*/
export function bindPaneVisibility(
paneId: string,
$open: { get(): boolean; listen(fn: (open: boolean) => void): void },
close?: () => void,
open?: () => void
) {
setTreePaneHidden(paneId, !$open.get())
$open.listen(isOpen => setTreePaneHidden(paneId, !isOpen))
if (close) {
registerPaneCloser(paneId, close)
}
if (open) {
registerPaneOpener(paneId, open)
}
}
/**
* 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
@@ -1407,8 +1440,8 @@ export function bindToolPaneCollapse(
}
/**
* ⌃` / the statusbar button / ⌘J's terminal fallback: ONE resolver for "flip
* this tool panel", derived from what is on screen rather than from the
* EVERY pane toggle: ⌃`, ⌘G, the statusbar button, the ⌘K rows. ONE resolver
* for "flip this pane", 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
@@ -1416,10 +1449,21 @@ export function bindToolPaneCollapse(
* 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.
*
* This is not a tool-panel quirk. The hide-style panes (files, review) had it
* too: `setTreePaneHidden(id, false)` deliberately does NOT front or
* un-minimize, because reactive unhides (a cwd arriving) must not clobber what
* the user is looking at. Correct for a reactive change, useless for a
* keypress — so user intent routes here and reactive bindings keep the quiet
* path.
*
* Close goes through `closeTreePane` so each pane keeps its own semantics: a
* tool panel collapses to its rail, files/review close through their store,
* anything else is dismissed.
*/
export function toggleToolPane(paneId: string) {
if (isToolPaneVisible(paneId)) {
collapseTreePane(paneId)
export function togglePaneVisible(paneId: string) {
if (isPaneVisible(paneId)) {
closeTreePane(paneId)
} else {
restoreTreePane(paneId)
}

View File

@@ -10,9 +10,9 @@ import {
$layoutTree,
bindToolPaneCollapse,
closeToolPane,
isToolPaneVisible,
isPaneVisible,
setTreeGroupHeaderHidden,
toggleToolPane
togglePaneVisible
} from './store'
// Ground truth for "toggle terminal broke — ⌘J/⌘B work fine, but once I move
@@ -114,7 +114,7 @@ describe('binding a tool panel on boot', () => {
$terminal.set(false)
expect(toolZone()?.minimized).toBe(true)
expect(isToolPaneVisible('terminal')).toBe(false)
expect(isPaneVisible('terminal')).toBe(false)
})
it('still collapses a tool panel whose store says it is off', () => {
@@ -132,11 +132,11 @@ describe('toggling the terminal while it is stacked with logs', () => {
bindPaneCollapse('terminal', atom(true))
bindPaneCollapse('logs', atom(true))
toggleToolPane('terminal')
expect(isToolPaneVisible('terminal')).toBe(false)
togglePaneVisible('terminal')
expect(isPaneVisible('terminal')).toBe(false)
toggleToolPane('terminal')
expect(isToolPaneVisible('terminal')).toBe(true)
togglePaneVisible('terminal')
expect(isPaneVisible('terminal')).toBe(true)
})
it('brings the terminal forward when logs holds the active tab', () => {
@@ -146,11 +146,11 @@ describe('toggling the terminal while it is stacked with logs', () => {
// The zone is open but showing LOGS, so the terminal is not on screen —
// the first press must reveal it rather than collapse the whole zone.
expect(isToolPaneVisible('terminal')).toBe(false)
expect(isPaneVisible('terminal')).toBe(false)
toggleToolPane('terminal')
togglePaneVisible('terminal')
expect(isToolPaneVisible('terminal')).toBe(true)
expect(isPaneVisible('terminal')).toBe(true)
expect(toolZone()?.minimized).toBeFalsy()
})
@@ -163,10 +163,10 @@ describe('toggling the terminal while it is stacked with logs', () => {
closeToolPane('terminal')
expect(allPaneIds($layoutTree.get()!)).not.toContain('terminal')
toggleToolPane('terminal')
togglePaneVisible('terminal')
expect(allPaneIds($layoutTree.get()!)).toContain('terminal')
expect(isToolPaneVisible('terminal')).toBe(true)
expect(isPaneVisible('terminal')).toBe(true)
})
})

View File

@@ -2,6 +2,7 @@ import { atom, computed, type ReadableAtom, type WritableAtom } from 'nanostores
import { SIDEBAR_COLLAPSE_MEDIA_QUERY } from '@/app/layout-constants'
import { PANE_TOGGLE_REVEAL_EVENT } from '@/components/pane-shell'
import { isPaneVisible, revealTreePane } from '@/components/pane-shell/tree/store'
import { matchesQuery } from '@/hooks/use-media-query'
import { Codecs, persistentAtom } from '@/lib/persisted'
import { arraysEqual, insertUniqueId, readKey } from '@/lib/storage'
@@ -37,6 +38,9 @@ const RIGHT_RAIL_ACTIVE_TAB_STORAGE_KEY = 'hermes.desktop.rightRailActiveTab'
export const CHAT_SIDEBAR_PANE_ID = 'chat-sidebar'
export const FILE_BROWSER_PANE_ID = 'file-browser'
/** The file tree's id in the LAYOUT TREE — distinct from the pane-state id
* above, which keys its open/width record. Toggles need both. */
export const FILES_PANE_ID = 'files'
export const PREVIEW_PANE_ID = 'preview'
/** Every rail tab is a preview of something, namespaced by what backs it: a
@@ -287,9 +291,23 @@ export function toggleSidebarOpen() {
}
export function toggleFileBrowserOpen() {
if (!revealNarrowPane(FILE_BROWSER_PANE_ID, 'toggle')) {
togglePane(FILE_BROWSER_PANE_ID)
if (revealNarrowPane(FILE_BROWSER_PANE_ID, 'toggle')) {
return
}
// Ask the TREE, not the pane's boolean. `$fileBrowserOpen` stays true while
// the tree pane sits behind a sibling tab in the shared right column (the
// preview rail, the diff) or inside a minimized zone, so ⌘J spent its press
// re-asserting a value it already held and read as a dead key. Only fold the
// side when the tree is genuinely the thing on screen; otherwise bring it
// forward through the reveal path, which fronts and un-minimizes.
if (!isPaneVisible(FILES_PANE_ID) && $fileBrowserOpen.get()) {
revealTreePane(FILES_PANE_ID)
return
}
togglePane(FILE_BROWSER_PANE_ID)
}
export function setFileBrowserOpen(open: boolean) {

View File

@@ -2,7 +2,7 @@ import { atom, computed } from 'nanostores'
import { SIDEBAR_COLLAPSE_MEDIA_QUERY } from '@/app/layout-constants'
import { PANE_TOGGLE_REVEAL_EVENT } from '@/components/pane-shell'
import { revealTreePane } from '@/components/pane-shell/tree/store'
import { isPaneVisible, revealTreePane } from '@/components/pane-shell/tree/store'
import type { HermesReviewFile, HermesReviewShipInfo } from '@/global'
import { matchesQuery } from '@/hooks/use-media-query'
import { desktopGit } from '@/lib/desktop-git'
@@ -285,7 +285,12 @@ export function toggleReview(scopeCwd: null | string = null): void {
return
}
if ($reviewOpen.get()) {
// Ask the TREE, not `$reviewOpen`. The store stays true while the pane sits
// behind a sibling tab in the right column or inside a minimized zone, so a
// boolean flip spent the press re-asserting a value it already held and ⌘G
// read as a dead key. `revealReview` fronts and un-minimizes; only close when
// the diff is genuinely the thing on screen.
if (isPaneVisible(REVIEW_PANE_ID)) {
closeReview()
} else {
revealReview(scopeCwd)