fix(desktop): don't auto-expand user-collapsed side on reactive unhide (#65375)

* fix(desktop): don't auto-expand user-collapsed side on reactive unhide

When `setTreePaneHidden(paneId, false)` was called for a workspace-gated
pane like `files` (bound to $hasWorkspace), it auto-called
`revealTreePane(paneId)` — which expanded the parent column even when
the user had explicitly collapsed it via Cmd+J.

That meant every session create / resume — anything that flipped
$currentCwd from empty to non-empty — silently re-opened the right
sidebar and persisted that open state to localStorage, so the original
session also showed the sidebar as open on return.

`setTreePaneHidden` is a state primitive; user-intent semantics (open
the side, front the tab) belong to `revealTreePane`. Drop the auto-call
and replace it with a narrower `frontPaneInGroup` helper that only
makes the pane the active tab in its group — visible the next time the
column is opened, without forcing it open now.

* fix(desktop): preserve explicit review reveal

---------

Co-authored-by: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com>

apps/desktop/src/components/pane-shell/tree/reactive-unhide.test.ts

@@ -0,0 +1,147 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+// Ground-truth repro for "reactive unhides silently re-open a user-collapsed
+// side": `setTreePaneHidden` is the primitive that backs `bindPaneVisibility`
+// (e.g. `bindPaneVisibility('files', $hasWorkspace)`). The original
+// implementation auto-called `revealTreePane` on every unhide, which expanded
+// the parent column even when the user had explicitly collapsed it — so a
+// workspace transition (`$currentCwd` flipping on session create / resume)
+// forced the right sidebar open and persisted that open state to localStorage.
+//
+// We mirror controller.tsx exactly: register the `files` pane with
+// `placement: 'right'`, declare the default tree, bind the right side to
+// the file-browser open store, then collapse the right sidebar and trigger
+// the reactive unhide via the same primitive `bindPaneVisibility` invokes.
+
+describe('reactive pane unhide', () => {
+  beforeEach(() => {
+    window.localStorage.clear()
+    vi.resetModules()
+  })
+
+  afterEach(() => {
+    vi.resetModules()
+  })
+
+  async function setupWithFiles() {
+    const tree = await import('@/components/pane-shell/tree/store')
+    const layout = await import('@/store/layout')
+    const model = await import('@/components/pane-shell/tree/model')
+    const { registry } = await import('@/contrib/registry')
+
+    // Register `files` like controller.tsx does — placement 'right' is what
+    // makes `treeSideOfPane('files')` return 'right' (and therefore what
+    // makes the buggy `revealTreePane` auto-expand the right column).
+    registry.register({
+      id: 'files',
+      area: 'panes',
+      title: 'files',
+      data: { placement: 'right' },
+      render: () => null
+    })
+
+    // Declare a minimal default tree mirroring the production DEFAULT_TREE's
+    // row shape (sessions | workspace | right-column-with-files).
+    tree.declareDefaultTree(
+      model.split('row', [
+        model.group(['sessions'], { id: 'grp-sessions' }),
+        model.group(['workspace'], { id: 'grp-main' }),
+        model.split('column', [
+          model.split('row', [
+            model.group(['files'], { id: 'grp-files' })
+          ], [1], 'spl-rail'),
+          model.group(['terminal'], { id: 'grp-terminal' })
+        ], [1.6, 1], 'spl-right')
+      ], [0.85, 3, 1.6])
+    )
+
+    // Mirror controller.tsx:512.
+    tree.bindTreeSideVisibility('right', layout.$fileBrowserOpen, layout.setFileBrowserOpen)
+
+    return { tree, layout }
+  }
+
+  it('reactive unhide does NOT expand a user-collapsed side', async () => {
+    const { tree, layout } = await setupWithFiles()
+
+    // Simulate the production scenario: a detached chat has no cwd, so
+    // `bindPaneVisibility('files', $hasWorkspace)` calls `setTreePaneHidden(
+    // 'files', true)` (hidden). Then the user collapses the right sidebar.
+    // Then a new session acquires a cwd and the workspace wiring unhides
+    // `files`. That last step is what the bug corrupts.
+    tree.setTreePaneHidden('files', true)
+    expect(tree.$hiddenTreePanes.get().has('files')).toBe(true)
+
+    layout.setFileBrowserOpen(false)
+    expect(layout.$fileBrowserOpen.get()).toBe(false)
+    expect(tree.$collapsedTreeSides.get().has('right')).toBe(true)
+
+    // Workspace flips: `bindPaneVisibility('files', $hasWorkspace)` calls
+    // `setTreePaneHidden('files', false)` — model it directly.
+    tree.setTreePaneHidden('files', false)
+
+    // The pane unhides…
+    expect(tree.$hiddenTreePanes.get().has('files')).toBe(false)
+
+    // …but the user-collapsed side MUST stay collapsed. Before the fix, the
+    // unhide called `revealTreePane`, which saw the side was collapsed and
+    // called `setFileBrowserOpen(true)` — silently re-opening the right
+    // sidebar on every session create / resume.
+    expect(layout.$fileBrowserOpen.get()).toBe(false)
+    expect(tree.$collapsedTreeSides.get().has('right')).toBe(true)
+  })
+
+  it('reactive HIDE (hidden=true) leaves the side flag alone', async () => {
+    const { tree, layout } = await setupWithFiles()
+
+    layout.setFileBrowserOpen(false)
+    expect(tree.$collapsedTreeSides.get().has('right')).toBe(true)
+
+    // A reactive HIDE (workspace goes away) must not flip the side either —
+    // collapsing the side is the user's domain; hiding individual panes
+    // inside it is the workspace domain.
+    tree.setTreePaneHidden('files', true)
+
+    expect(tree.$hiddenTreePanes.get().has('files')).toBe(true)
+    expect(layout.$fileBrowserOpen.get()).toBe(false)
+    expect(tree.$collapsedTreeSides.get().has('right')).toBe(true)
+  })
+
+  it('explicit setFileBrowserOpen still expands the side (user toggle)', async () => {
+    const { tree, layout } = await setupWithFiles()
+
+    layout.setFileBrowserOpen(false)
+    expect(tree.$collapsedTreeSides.get().has('right')).toBe(true)
+
+    // The user toggles the sidebar back open — this MUST still work; the
+    // binding mirrors `$fileBrowserOpen` to `$collapsedTreeSides`.
+    layout.setFileBrowserOpen(true)
+    expect(layout.$fileBrowserOpen.get()).toBe(true)
+    expect(tree.$collapsedTreeSides.get().has('right')).toBe(false)
+  })
+
+  it('reactive unhide does not invoke the right side opener directly', async () => {
+    const { tree, layout } = await setupWithFiles()
+
+    // Spy on the opener that `revealTreePane` would call when expanding a
+    // collapsed side — the bug is exactly this call firing on reactive unhide.
+    const openerSpy = vi.fn()
+    tree.bindTreeSideVisibility('right', layout.$fileBrowserOpen, openerSpy)
+
+    layout.setFileBrowserOpen(false)
+    expect(tree.$collapsedTreeSides.get().has('right')).toBe(true)
+
+    tree.setTreePaneHidden('files', true)
+    expect(tree.$hiddenTreePanes.get().has('files')).toBe(true)
+
+    openerSpy.mockClear()
+
+    // Reactive unhide fires via the same primitive the workspace wiring uses.
+    tree.setTreePaneHidden('files', false)
+
+    // The opener MUST NOT be called — before the fix, the auto-reveal would
+    // call `setFileBrowserOpen(true)` via this opener.
+    expect(tree.$hiddenTreePanes.get().has('files')).toBe(false)
+    expect(openerSpy).not.toHaveBeenCalled()
+  })
+})

apps/desktop/src/components/pane-shell/tree/store.ts

@@ -126,9 +126,33 @@ export function setTreePaneHidden(paneId: string, hidden: boolean) {
 
   $hiddenTreePanes.set(next)
 
-  // Unhiding is an intent to SEE the pane — front it in its group.
+  // Reactive unhides (e.g. `bindPaneVisibility('files', $hasWorkspace)`) are
+  // state-driven, not user intent — opening the side or fronting the tab in
+  // response to an environmental flag change would clobber an explicit user
+  // collapse (Cmd+J) and silently re-open the rail after every session create.
+  // Callers that want user-intent semantics (open the side, front the tab)
+  // must call `revealTreePane` explicitly. We still front the pane in its
+  // group so it's visible the next time the column is shown.
   if (!hidden) {
-    revealTreePane(paneId)
+    frontPaneInGroup(paneId)
+  }
+}
+
+/** Make `paneId` the active tab in its group without touching side collapse
+ *  or zone-minimized state — the safe "make it visible next time the column
+ *  is shown" primitive that reactive unhides need. */
+function frontPaneInGroup(paneId: string) {
+  const tree = $layoutTree.get()
+  const group = tree ? findGroupOfPane(tree, paneId) : null
+
+  if (!tree || !group || group.active === paneId) {
+    return
+  }
+
+  const next = setActivePaneOp(tree, group.id, paneId)
+
+  if (next !== tree) {
+    commit(next)
   }
 }
 

apps/desktop/src/store/review.ts

@@ -2,6 +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 type { HermesReviewFile, HermesReviewShipInfo } from '@/global'
 import { matchesQuery } from '@/hooks/use-media-query'
 import { desktopGit } from '@/lib/desktop-git'
@@ -274,6 +275,7 @@ export function toggleReview(): void {
     closeReview()
   } else {
     openReview()
+    revealTreePane(REVIEW_PANE_ID)
   }
 }