From 170610d1b2ce2496c1ee004a87fba1fc8db63032 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Tue, 14 Apr 2026 03:45:04 -0400 Subject: [PATCH] fix: resolve Windows freeze when switching worktrees with right sidebar open (#628) --- src/main/ipc/filesystem-watcher.ts | 50 ++++++++++-- src/main/window/createMainWindow.ts | 10 +++ src/preload/api-types.d.ts | 2 + src/preload/index.ts | 10 +++ .../components/right-sidebar/ChecksPanel.tsx | 13 ++++ .../right-sidebar/SourceControl.tsx | 13 ++++ .../src/components/right-sidebar/index.tsx | 58 +++++++++----- .../components/terminal-pane/pane-helpers.ts | 37 ++++++++- .../use-terminal-pane-global-effects.ts | 76 ++++++++++++++++++- src/renderer/src/hooks/useIpcEvents.test.ts | 2 + src/renderer/src/hooks/useIpcEvents.ts | 12 +++ src/shared/window-shortcut-policy.ts | 14 ++++ 12 files changed, 265 insertions(+), 32 deletions(-) diff --git a/src/main/ipc/filesystem-watcher.ts b/src/main/ipc/filesystem-watcher.ts index 4fcf8648e..b29c368cd 100644 --- a/src/main/ipc/filesystem-watcher.ts +++ b/src/main/ipc/filesystem-watcher.ts @@ -56,6 +56,14 @@ const unwatchableRoots = new Set() // listener so we register exactly once per sender. const senderCleanupRegistered = new Set() +// Why: on Windows, tearing down and recreating @parcel/watcher subscriptions +// is expensive (ReadDirectoryChangesW setup + antivirus scanning can take +// 500 ms+). A 30 s grace period lets rapid worktree switches reuse the +// existing watcher instead of paying the creation cost on every switch. +// Key: rootKey, Value: pending teardown timer. +const WATCHER_TEARDOWN_GRACE_MS = 30_000 +const pendingTeardowns = new Map>() + // ── Path normalization ─────────────────────────────────────────────── function normalizeRootPath(rootPath: string): string { @@ -306,6 +314,14 @@ async function subscribe(worktreePath: string, sender: WebContents): Promise { - console.error(`[filesystem-watcher] unsubscribe error for ${rootKey}:`, err) - }) - watchedRoots.delete(rootKey) + + const teardownTimer = setTimeout(() => { + pendingTeardowns.delete(rootKey) + // Re-check: a new listener may have arrived during the grace period. + const currentRoot = watchedRoots.get(rootKey) + if (!currentRoot || currentRoot.listeners.size > 0) { + return + } + void currentRoot.subscription.unsubscribe().catch((err: unknown) => { + console.error(`[filesystem-watcher] unsubscribe error for ${rootKey}:`, err) + }) + watchedRoots.delete(rootKey) + }, WATCHER_TEARDOWN_GRACE_MS) + + pendingTeardowns.set(rootKey, teardownTimer) } } @@ -454,6 +488,12 @@ export function registerFilesystemWatcherHandlers(): void { /** Tear down all watchers on app shutdown. */ export async function closeAllWatchers(): Promise { + // Cancel any pending grace-period teardowns — we're tearing down everything. + for (const timer of pendingTeardowns.values()) { + clearTimeout(timer) + } + pendingTeardowns.clear() + for (const [rootKey, root] of watchedRoots) { if (root.batch.timer) { clearTimeout(root.batch.timer) diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index 71270df6e..5a9922175 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -235,6 +235,16 @@ export function createMainWindow( return } + if (action.type === 'toggleLeftSidebar') { + mainWindow.webContents.send('ui:toggleLeftSidebar') + return + } + + if (action.type === 'toggleRightSidebar') { + mainWindow.webContents.send('ui:toggleRightSidebar') + return + } + if (action.type === 'toggleWorktreePalette') { // Why: embedded browser guests can keep keyboard focus inside Chromium's // guest webContents, which bypasses the renderer's window-level keydown diff --git a/src/preload/api-types.d.ts b/src/preload/api-types.d.ts index 54cb433cc..1307a1d46 100644 --- a/src/preload/api-types.d.ts +++ b/src/preload/api-types.d.ts @@ -482,6 +482,8 @@ export type PreloadApi = { get: () => Promise set: (args: Partial) => Promise onOpenSettings: (callback: () => void) => () => void + onToggleLeftSidebar: (callback: () => void) => () => void + onToggleRightSidebar: (callback: () => void) => () => void onToggleWorktreePalette: (callback: () => void) => () => void onOpenQuickOpen: (callback: () => void) => () => void onJumpToWorktreeIndex: (callback: (index: number) => void) => () => void diff --git a/src/preload/index.ts b/src/preload/index.ts index 6844a49aa..276b5cb53 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -922,6 +922,16 @@ const api = { ipcRenderer.on('ui:openSettings', listener) return () => ipcRenderer.removeListener('ui:openSettings', listener) }, + onToggleLeftSidebar: (callback: () => void): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent) => callback() + ipcRenderer.on('ui:toggleLeftSidebar', listener) + return () => ipcRenderer.removeListener('ui:toggleLeftSidebar', listener) + }, + onToggleRightSidebar: (callback: () => void): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent) => callback() + ipcRenderer.on('ui:toggleRightSidebar', listener) + return () => ipcRenderer.removeListener('ui:toggleRightSidebar', listener) + }, onToggleWorktreePalette: (callback: () => void): (() => void) => { const listener = (_event: Electron.IpcRendererEvent) => callback() ipcRenderer.on('ui:toggleWorktreePalette', listener) diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 69638e720..11fe3ee49 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -43,6 +43,19 @@ export default function ChecksPanel(): React.JSX.Element { const prevChecksRef = useRef('') const conflictSummaryRefreshKeyRef = useRef(null) + // Why: the sidebar no longer uses key={activeWorktreeId} to force a full + // remount on worktree switch (that caused an IPC storm on Windows). + // Reset worktree-specific local state so stale UI from the previous + // worktree doesn't leak (e.g. mid-edit title, stale loading indicators). + useEffect(() => { + setEditingTitle(false) + setTitleDraft('') + setTitleSaving(false) + setIsRefreshing(false) + setEmptyRefreshing(false) + conflictSummaryRefreshKeyRef.current = null + }, [activeWorktreeId]) + // Find active worktree and repo const { worktree, repo } = useMemo(() => { if (!activeWorktreeId) { diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 672a753af..6765f10f7 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -254,6 +254,19 @@ function SourceControlInner(): React.JSX.Element { const [isExecutingBulk, setIsExecutingBulk] = useState(false) + // Why: the sidebar no longer uses key={activeWorktreeId} to force a full + // remount on worktree switch (that caused an IPC storm on Windows). + // Instead, reset worktree-specific local state here so the previous + // worktree's UI state doesn't leak into the new one. + useEffect(() => { + setScope('all') + setCollapsedSections(new Set()) + setBaseRefDialogOpen(false) + setDefaultBaseRef('origin/main') + setFilterQuery('') + setIsExecutingBulk(false) + }, [activeWorktreeId]) + const handleOpenDiff = useCallback( (entry: GitStatusEntry) => { if (!activeWorktreeId || !worktreePath) { diff --git a/src/renderer/src/components/right-sidebar/index.tsx b/src/renderer/src/components/right-sidebar/index.tsx index a56957d8d..0dca319ec 100644 --- a/src/renderer/src/components/right-sidebar/index.tsx +++ b/src/renderer/src/components/right-sidebar/index.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo, useState } from 'react' +import React, { useLayoutEffect, useMemo, useRef } from 'react' import { Files, Search, GitBranch, ListChecks } from 'lucide-react' import { useAppStore } from '@/store' import { cn } from '@/lib/utils' @@ -114,7 +114,6 @@ function RightSidebarInner(): React.JSX.Element { const setRightSidebarWidth = useAppStore((s) => s.setRightSidebarWidth) const rightSidebarTab = useAppStore((s) => s.rightSidebarTab) const setRightSidebarTab = useAppStore((s) => s.setRightSidebarTab) - const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const checksStatus = useAppStore(getActiveChecksStatus) const activityBarPosition = useAppStore((s) => s.activityBarPosition) const setActivityBarPosition = useAppStore((s) => s.setActivityBarPosition) @@ -137,19 +136,18 @@ function RightSidebarInner(): React.JSX.Element { ? rightSidebarTab : visibleItems[0].id - // Why: suppress CSS width transitions on first mount so the sidebar - // appears at full width instantly instead of animating from auto→320 px. - // Without this, Chromium may fire the transition, causing the terminal - // container to resize through intermediate widths. Each intermediate + // Why: suppress CSS width transitions when the sidebar opens so the + // width snaps to 320 px instantly instead of animating from 0→320 px. + // Without this, Chromium fires the transition causing the terminal + // container to resize through intermediate widths. Each intermediate // width triggers a synchronous xterm scrollback reflow that blocks the - // renderer, freezing the entire app for seconds on Windows. - const [isMounting, setIsMounting] = useState(true) - useEffect(() => { - // Clear the flag after the first paint so drag-resize transitions - // work normally after the sidebar is visible. - const id = requestAnimationFrame(() => setIsMounting(false)) - return () => cancelAnimationFrame(id) - }, []) + // renderer for seconds on Windows. + // + // useLayoutEffect + direct DOM style manipulation runs synchronously + // BEFORE the browser paints, so the transition is killed before Chromium + // can start it. A useState-based approach has a timing race: the state + // update triggers a re-render that arrives one frame too late. + const prevOpenRef = useRef(rightSidebarOpen) const activityBarSideWidth = activityBarPosition === 'side' ? ACTIVITY_BAR_SIDE_WIDTH : 0 const { containerRef, isResizing, onResizeStart } = useSidebarResize({ @@ -162,12 +160,34 @@ function RightSidebarInner(): React.JSX.Element { setWidth: setRightSidebarWidth }) + useLayoutEffect(() => { + if (rightSidebarOpen && !prevOpenRef.current) { + const el = containerRef.current + if (el) { + // Kill the transition before the browser paints the width change. + el.style.transition = 'none' + // Restore CSS-class-driven transitions in the next frame so close + // animations and drag-resize still animate smoothly. + requestAnimationFrame(() => { + el.style.transition = '' + }) + } + } + prevOpenRef.current = rightSidebarOpen + }, [rightSidebarOpen, containerRef]) + const panelContent = (
- {effectiveTab === 'explorer' && } - {effectiveTab === 'search' && } - {effectiveTab === 'source-control' && } - {effectiveTab === 'checks' && } + {/* Why: sidebar panels no longer use key={activeWorktreeId} because + the full unmount/remount cycle on every worktree switch triggered + an IPC storm (watchWorktree + readDir + git:branchCompare + …) + that froze the app for seconds on Windows. Each panel now reacts + to activeWorktreeId changes via store subscriptions and reset + effects, keeping the component instance alive across switches. */} + {effectiveTab === 'explorer' && } + {effectiveTab === 'search' && } + {effectiveTab === 'source-control' && } + {effectiveTab === 'checks' && }
) @@ -187,7 +207,7 @@ function RightSidebarInner(): React.JSX.Element { ref={containerRef} className={cn( 'relative flex-shrink-0 flex flex-row overflow-visible', - isResizing || isMounting ? 'transition-none' : 'transition-[width] duration-200' + isResizing ? 'transition-none' : 'transition-[width] duration-200' )} > {/* Panel content area */} diff --git a/src/renderer/src/components/terminal-pane/pane-helpers.ts b/src/renderer/src/components/terminal-pane/pane-helpers.ts index 6776e0f60..9ab72347e 100644 --- a/src/renderer/src/components/terminal-pane/pane-helpers.ts +++ b/src/renderer/src/components/terminal-pane/pane-helpers.ts @@ -3,10 +3,16 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' export function fitPanes(manager: PaneManager): void { for (const pane of manager.getPanes()) { try { - // Why: fitAddon.fit() triggers a terminal reflow that can leave the viewport - // at a stale scroll offset, making the terminal appear scrolled up after a - // resize. Capture whether the terminal was at the bottom before fitting and - // restore that position afterwards so the user's prompt stays visible. + // Why: fitAddon.fit() calls _renderService.clear() + terminal.refresh() + // even when dimensions haven't changed (the patched FitAddon only skips + // terminal.resize()). On Windows the clear+refresh overhead is non-trivial + // with 10 000 scrollback lines. Skip entirely when the proposed dimensions + // match the current ones — this is the common case when a terminal simply + // transitions from hidden → visible at the same container size. + const dims = pane.fitAddon.proposeDimensions() + if (dims && dims.cols === pane.terminal.cols && dims.rows === pane.terminal.rows) { + continue + } const buf = pane.terminal.buffer.active const wasAtBottom = buf.viewportY >= buf.baseY pane.fitAddon.fit() @@ -19,6 +25,29 @@ export function fitPanes(manager: PaneManager): void { } } +/** + * Returns true if any pane's proposed dimensions differ from its current + * terminal cols/rows, meaning a fit() call would actually change layout. + * Used by the epoch-based deduplication in use-terminal-pane-global-effects + * to allow legitimate resize fits while suppressing redundant ones. + */ +export function hasDimensionsChanged(manager: PaneManager): boolean { + for (const pane of manager.getPanes()) { + try { + const dims = pane.fitAddon.proposeDimensions() + if (!dims) { + return true // can't determine — assume changed + } + if (dims.cols !== pane.terminal.cols || dims.rows !== pane.terminal.rows) { + return true + } + } catch { + return true + } + } + return false +} + export function focusActivePane(manager: PaneManager): void { const panes = manager.getPanes() const activePane = manager.getActivePane() ?? panes[0] diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index c8fdab9bd..25c1068e2 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -6,7 +6,7 @@ import { } from '@/constants/terminal' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { shellEscapePath } from './pane-helpers' -import { fitAndFocusPanes, fitPanes } from './pane-helpers' +import { fitAndFocusPanes, fitPanes, hasDimensionsChanged } from './pane-helpers' import type { PtyTransport } from './pty-transport' type UseTerminalPaneGlobalEffectsArgs = { @@ -36,13 +36,37 @@ export function useTerminalPaneGlobalEffects({ // function can cancel it if the pane deactivates mid-flush. const pendingFlushRef = useRef | null>(null) + // Why: the deferred rAF (guardedResumeAndFit) must be cancellable when + // the pane deactivates before the rAF fires — otherwise it would call + // resumeRendering() on an already-suspended manager. + const pendingRafRef = useRef(null) + + // Why: two independent code paths schedule fitPanes() after a worktree + // switch — the isActive effect (after pending-write drain) and the + // ResizeObserver (after its 150 ms debounce). On Windows, each redundant + // fit() call adds non-trivial overhead (clear + refresh of 10 000 + // scrollback lines). An epoch counter lets whichever path fires first + // serve the activation, while the second path skips. The staleness + // check also rejects callbacks from a prior activation during rapid + // worktree switches (A→B→C). + const fitEpochRef = useRef(0) + const fitRanForEpochRef = useRef(-1) + useEffect(() => { const manager = managerRef.current if (!manager) { return } if (isActive) { - manager.resumeRendering() + // Why: resumeRendering() creates WebGL contexts for each pane, which + // blocks the renderer for 100–500 ms per pane on Windows (ANGLE → + // D3D11). Deferring it into the rAF that runs after the pending-write + // drain lets the browser paint one frame with the DOM renderer so the + // terminal content appears immediately. WebGL takes over seamlessly + // in the next frame without a visible flash. + + fitEpochRef.current++ + const epoch = fitEpochRef.current // Why: while a worktree is in the background, PTY output accumulates // in pendingWritesRef with no size cap. A Claude agent running for @@ -65,8 +89,35 @@ export function useTerminalPaneGlobalEffects({ pendingWritesRef.current.set(paneId, '') } + const guardedResumeAndFit = (): void => { + pendingRafRef.current = null + // Why: read managerRef.current at rAF time instead of capturing + // it at effect entry — the PaneManager instance can change if the + // component unmounts and remounts during rapid tab switches. + const mgr = managerRef.current + if (!mgr) { + return + } + mgr.resumeRendering() + // Why: three-layer guard prevents redundant and stale fits. + // 1. Staleness — reject callbacks from a superseded activation + // (e.g. rapid A→B→C worktree switch). + if (epoch !== fitEpochRef.current) { + return + } + // 2. Dimension check — if a window resize changed the container + // size, the fit must run even if one already ran for this epoch. + const dimensionsChanged = hasDimensionsChanged(mgr) + // 3. Dedup — if dims are the same and a fit already ran, skip. + if (!dimensionsChanged && fitRanForEpochRef.current >= epoch) { + return + } + fitRanForEpochRef.current = epoch + fitAndFocusPanes(mgr) + } + if (entries.length === 0) { - requestAnimationFrame(() => fitAndFocusPanes(manager)) + pendingRafRef.current = requestAnimationFrame(guardedResumeAndFit) } else { let entryIdx = 0 let offset = 0 @@ -74,7 +125,7 @@ export function useTerminalPaneGlobalEffects({ const drainNextChunk = (): void => { if (entryIdx >= entries.length) { pendingFlushRef.current = null - requestAnimationFrame(() => fitAndFocusPanes(manager)) + pendingRafRef.current = requestAnimationFrame(guardedResumeAndFit) return } @@ -108,6 +159,12 @@ export function useTerminalPaneGlobalEffects({ clearTimeout(pendingFlushRef.current) pendingFlushRef.current = null } + // Cancel any pending rAF so guardedResumeAndFit doesn't call + // resumeRendering() on an already-suspended manager. + if (pendingRafRef.current !== null) { + cancelAnimationFrame(pendingRafRef.current) + pendingRafRef.current = null + } manager.suspendRendering() } wasActiveRef.current = isActive @@ -192,6 +249,17 @@ export function useTerminalPaneGlobalEffects({ if (!manager) { return } + // Why: apply the same epoch-based deduplication as the isActive + // effect's rAF path. Read the current epoch at fire time (not a + // captured value) because the ResizeObserver persists across the + // activation. Dimension changes (e.g. window resize) bypass the + // dedup so legitimate refits are never suppressed. + const currentEpoch = fitEpochRef.current + const dimensionsChanged = hasDimensionsChanged(manager) + if (!dimensionsChanged && fitRanForEpochRef.current >= currentEpoch) { + return + } + fitRanForEpochRef.current = currentEpoch fitPanes(manager) }, RESIZE_DEBOUNCE_MS) }) diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index ac340fc23..999461860 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -136,6 +136,8 @@ describe('useIpcEvents updater integration', () => { worktrees: { onChanged: () => () => {} }, ui: { onOpenSettings: () => () => {}, + onToggleLeftSidebar: () => () => {}, + onToggleRightSidebar: () => () => {}, onToggleWorktreePalette: () => () => {}, onOpenQuickOpen: () => () => {}, onJumpToWorktreeIndex: () => () => {}, diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index de3c84426..e8153a984 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -42,6 +42,18 @@ export function useIpcEvents(): void { }) ) + unsubs.push( + window.api.ui.onToggleLeftSidebar(() => { + useAppStore.getState().toggleSidebar() + }) + ) + + unsubs.push( + window.api.ui.onToggleRightSidebar(() => { + useAppStore.getState().toggleRightSidebar() + }) + ) + unsubs.push( window.api.ui.onToggleWorktreePalette(() => { const store = useAppStore.getState() diff --git a/src/shared/window-shortcut-policy.ts b/src/shared/window-shortcut-policy.ts index d6e4d9e20..d27079f1c 100644 --- a/src/shared/window-shortcut-policy.ts +++ b/src/shared/window-shortcut-policy.ts @@ -10,6 +10,8 @@ export type WindowShortcutInput = { export type WindowShortcutAction = | { type: 'zoom'; direction: 'in' | 'out' | 'reset' } | { type: 'toggleWorktreePalette' } + | { type: 'toggleLeftSidebar' } + | { type: 'toggleRightSidebar' } | { type: 'openQuickOpen' } | { type: 'jumpToWorktreeIndex'; index: number } @@ -69,6 +71,18 @@ export function resolveWindowShortcutAction( return { type: 'toggleWorktreePalette' } } + // Why: Ctrl+B and Ctrl+L are terminal control characters (STX / form-feed). + // Without main-process interception, xterm.js processes the keydown before + // the renderer's window-capture handler can preventDefault, causing ^B / ^L + // to appear in the terminal alongside the sidebar toggle. + if (input.code === 'KeyB' && !input.shift) { + return { type: 'toggleLeftSidebar' } + } + + if (input.code === 'KeyL' && !input.shift) { + return { type: 'toggleRightSidebar' } + } + if (input.code === 'KeyP' && !input.shift) { return { type: 'openQuickOpen' } }