fix(sidebar): keep scroll position stable when deleting a workspace (#2863)

* fix(sidebar): keep scroll position stable when deleting a workspace

Deleting a workspace made the sidebar scroll and shift instead of just
removing the row. Two causes: the scroll-anchor restore fell straight to
scrollToIndex(align:'start') — snapping the anchor row to the viewport
top — whenever the post-delete virtual window briefly lacked the row's
DOM node; and scroll preservation was only wired to the context menu at
click time, never to the async removal that actually drops the row.

Restore now pins from the measured slot before that snapping fallback,
and removeWorktree records the top-row anchor in the same tick it removes
the row, covering every delete entry point (modal, card, SSH, batch).

* fix(sidebar): preserve delete scroll anchor

Keep virtualized scroll listener registration stable across row changes so cleanup cannot overwrite the pre-delete anchor after the row list mutates.

Add a regression test for the listener dependency contract.

---------

Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>

src/renderer/src/hooks/requestVirtualizedScrollAnchorRecord.ts

@@ -0,0 +1,17 @@
+import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from './useVirtualizedScrollAnchor'
+
+/**
+ * Asks a mounted virtualized scroller (matched by selector) to snapshot its
+ * current top-row anchor right now. Lets code outside the sidebar — e.g. the
+ * store's async worktree removal — capture the live anchor in the same tick it
+ * mutates the row list, so the post-mutation restore pins the same visible row
+ * instead of recording a stale anchor from an earlier click.
+ */
+export function requestVirtualizedScrollAnchorRecord(scrollElementSelector: string): void {
+  if (typeof document === 'undefined') {
+    return
+  }
+  document
+    .querySelector(scrollElementSelector)
+    ?.dispatchEvent(new Event(VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT))
+}

src/renderer/src/hooks/useVirtualizedScrollAnchor.listener-deps.test.ts

@@ -0,0 +1,72 @@
+import { afterEach, describe, expect, it, vi } from 'vitest'
+
+function createReactHookHarness() {
+  const refs: { current: unknown }[] = []
+  const effects: { deps: readonly unknown[] | undefined }[] = []
+  let refIndex = 0
+
+  return {
+    beginRender: () => {
+      refIndex = 0
+      effects.length = 0
+    },
+    effects,
+    react: {
+      useCallback: <T extends (...args: never[]) => unknown>(callback: T): T => callback,
+      useLayoutEffect: (_effect: () => void | (() => void), deps?: readonly unknown[]) => {
+        effects.push({ deps })
+      },
+      useMemo: <T>(factory: () => T): T => factory(),
+      useRef: <T>(initialValue: T): { current: T } => {
+        const index = refIndex
+        refIndex += 1
+        refs[index] ??= { current: initialValue }
+        return refs[index] as { current: T }
+      }
+    }
+  }
+}
+
+describe('useVirtualizedScrollAnchor listener effect dependencies', () => {
+  afterEach(() => {
+    vi.doUnmock('react')
+    vi.resetModules()
+  })
+
+  it('does not tear down the scroll listener when row snapshots change', async () => {
+    const harness = createReactHookHarness()
+    vi.doMock('react', () => harness.react)
+    const { useVirtualizedScrollAnchor } = await import('./useVirtualizedScrollAnchor')
+
+    const anchorRef = { current: null }
+    const scrollElementRef = { current: null }
+    const scrollOffsetRef = { current: 0 }
+    const virtualizer = {
+      getVirtualItems: () => [],
+      isScrolling: false,
+      scrollToIndex: vi.fn()
+    }
+    const renderWithRows = (rows: readonly string[]) => {
+      harness.beginRender()
+      // oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
+      useVirtualizedScrollAnchor({
+        anchorRef,
+        getRowKey: (row) => row,
+        rows,
+        scrollElementRef,
+        scrollOffsetRef,
+        totalSize: rows.length,
+        virtualizer
+      } as never)
+      return harness.effects[0]?.deps
+    }
+
+    const initialDeps = renderWithRows(['before-delete', 'stable-top'])
+    const nextDeps = renderWithRows(['stable-top'])
+
+    // Why: cleanup records the current anchor. If rows are dependencies, a
+    // delete reruns cleanup after mutation and overwrites the pre-delete anchor.
+    expect(initialDeps).toEqual([scrollElementRef, scrollOffsetRef])
+    expect(nextDeps).toEqual(initialDeps)
+  })
+})

src/renderer/src/hooks/useVirtualizedScrollAnchor.ts

@@ -1,4 +1,11 @@
-import { useCallback, useLayoutEffect, useMemo, type MutableRefObject, type RefObject } from 'react'
+import {
+  useCallback,
+  useLayoutEffect,
+  useMemo,
+  useRef,
+  type MutableRefObject,
+  type RefObject
+} from 'react'
 import type { Virtualizer } from '@tanstack/react-virtual'
 
 export type VirtualizedScrollAnchor = {
@@ -145,6 +152,15 @@ export function useVirtualizedScrollAnchor<
     [anchorRef, findDomAnchor, recordVirtualScrollAnchor, scrollElementRef]
   )
 
+  // Why: row changes must not re-register the scroll listener; cleanup records
+  // an anchor and would overwrite the pre-delete anchor after the row is gone.
+  const recordScrollAnchorRef = useRef(recordScrollAnchor)
+  recordScrollAnchorRef.current = recordScrollAnchor
+  const recordVirtualScrollAnchorRef = useRef(recordVirtualScrollAnchor)
+  recordVirtualScrollAnchorRef.current = recordVirtualScrollAnchor
+  const hasDirectScrollInputRef = useRef(hasDirectScrollInput)
+  hasDirectScrollInputRef.current = hasDirectScrollInput
+
   useLayoutEffect(() => {
     const el = scrollElementRef.current
     if (!el) {
@@ -177,19 +193,19 @@ export function useVirtualizedScrollAnchor<
         // is idle, then do the read on the next frame instead of the input path.
         frameId = window.requestAnimationFrame(() => {
           frameId = null
-          recordScrollAnchor(el.scrollTop)
+          recordScrollAnchorRef.current(el.scrollTop)
         })
       }, RECORD_ANCHOR_SCROLL_IDLE_DELAY_MS)
     }
     const recordCurrentAnchor = (): void => {
       cancelScheduledRecord()
       scrollOffsetRef.current = el.scrollTop
-      recordScrollAnchor(el.scrollTop)
+      recordScrollAnchorRef.current(el.scrollTop)
     }
     const onScroll = (): void => {
       if (
         shouldCancelVirtualizedScrollOffsetRestore({
-          hasDirectScrollInput,
+          hasDirectScrollInput: hasDirectScrollInputRef.current,
           restoring
         })
       ) {
@@ -219,7 +235,7 @@ export function useVirtualizedScrollAnchor<
         return
       }
       scrollOffsetRef.current = el.scrollTop
-      recordVirtualScrollAnchor(el.scrollTop)
+      recordVirtualScrollAnchorRef.current(el.scrollTop)
       scheduleRecordAnchor()
     }
 
@@ -228,17 +244,11 @@ export function useVirtualizedScrollAnchor<
     return () => {
       cancelScheduledRecord()
       scrollOffsetRef.current = el.scrollTop
-      recordScrollAnchor(el.scrollTop)
+      recordScrollAnchorRef.current(el.scrollTop)
       el.removeEventListener(VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT, recordCurrentAnchor)
       el.removeEventListener('scroll', onScroll)
     }
-  }, [
-    recordScrollAnchor,
-    recordVirtualScrollAnchor,
-    scrollElementRef,
-    scrollOffsetRef,
-    hasDirectScrollInput
-  ])
+  }, [scrollElementRef, scrollOffsetRef])
 
   useLayoutEffect(() => {
     const anchor = anchorRef.current
@@ -309,13 +319,18 @@ export function useVirtualizedScrollAnchor<
       return
     }
 
-    if (!itemElementSelector && restoreFromMeasuredItem()) {
+    // Why: right after a delete the virtualizer can briefly render the wrong
+    // window, so the anchor row's DOM node isn't mounted yet even though the
+    // virtualizer still has its measured slot. Pin from that measured start
+    // (preserving the within-row offset) before falling back to scrollToIndex,
+    // whose align:'start' snaps the row to the viewport top and visibly jumps.
+    if (restoreFromMeasuredItem()) {
       return
     }
 
-    // Why: after add/delete the virtualizer can initially render the wrong
-    // window. Move to the anchored row, then apply the within-row offset once
-    // TanStack Virtual has mounted and measured that row.
+    // Why: the anchored row is outside the virtualizer's current window — no
+    // DOM node and no measured slot. Bring it in, then apply the within-row
+    // offset once TanStack Virtual has mounted and measured that row.
     virtualizer.scrollToIndex(index, { align: 'start' })
     const frameId = window.requestAnimationFrame(() => {
       if (!restoreFromDomElement()) {

src/renderer/src/store/slices/worktrees.test.ts

@@ -1233,6 +1233,35 @@ describe('removeWorktree state cleanup', () => {
     expect(store.getState().editorViewMode).toEqual({ 'file-2': 'changes' })
   })
 
+  it('records the sidebar scroll anchor in the same tick it removes the worktree', async () => {
+    const store = createTestStore()
+    const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' })
+    store.setState({ worktreesByRepo: { repo1: [wt] } } as Partial<AppState>)
+
+    const sidebar = new EventTarget()
+    let worktreePresentWhenRecorded: boolean | null = null
+    sidebar.addEventListener('orca-record-virtualized-scroll-anchor', () => {
+      worktreePresentWhenRecorded =
+        store.getState().worktreesByRepo.repo1?.some((w) => w.id === wt.id) ?? false
+    })
+    const globalWithDocument = globalThis as { document?: unknown }
+    const originalDocument = globalWithDocument.document
+    globalWithDocument.document = {
+      querySelector: (selector: string) => (selector === '[data-worktree-sidebar]' ? sidebar : null)
+    }
+
+    try {
+      await store.getState().removeWorktree(wt.id)
+    } finally {
+      globalWithDocument.document = originalDocument
+    }
+
+    // The anchor must be captured while the row still exists so the post-delete
+    // restore pins the pre-removal position instead of the already-shifted list.
+    expect(worktreePresentWhenRecorded).toBe(true)
+    expect(store.getState().worktreesByRepo.repo1).toEqual([])
+  })
+
   it('cleans up expandedDirs for the removed worktree', async () => {
     const store = createTestStore()
     const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' })

src/renderer/src/store/slices/worktrees.ts

@@ -26,6 +26,7 @@ import {
 import { getHostedReviewCacheKey } from './hosted-review'
 import { getGitHubPRCacheKey, getLegacyGitHubPRCacheKey } from './github-cache-key'
 import { moveFocusToRendererBeforeFocusedWebviewHidden } from './browser-webview-cleanup'
+import { requestVirtualizedScrollAnchorRecord } from '@/hooks/requestVirtualizedScrollAnchorRecord'
 export type { WorktreeSlice, WorktreeDeleteState } from './worktree-helpers'
 
 // Why: old runtime servers only have `worktree.list`; preserve the large-list
@@ -1021,6 +1022,13 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
       const tabs = get().tabsByWorktree[worktreeId] ?? []
       const tabIds = new Set(tabs.map((t) => t.id))
 
+      // Why: deletion is async (backend + terminal/browser teardown awaited
+      // above), so snapshot the sidebar's current top-row anchor in the same
+      // tick we remove the row. Recording at click time goes stale across the
+      // await, and this covers every delete entry point (modal, card, SSH,
+      // batch) rather than only the context menu.
+      requestVirtualizedScrollAnchorRecord('[data-worktree-sidebar]')
+
       set((s) => {
         const next = { ...s.worktreesByRepo }
         for (const repoId of Object.keys(next)) {