From 48c67ca6f408c67aeeba657d026552de65d2f183 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 13 Apr 2026 19:40:10 -0400 Subject: [PATCH] fix: resolve right sidebar freeze on Windows (#598) Co-authored-by: Claude Opus 4.6 (1M context) --- src/main/ipc/filesystem-auth.ts | 47 +++++--- src/main/ipc/filesystem-watcher.ts | 28 ++++- .../src/components/right-sidebar/index.tsx | 18 ++- .../terminal-pane/pty-connection.ts | 24 +++- .../use-terminal-pane-global-effects.ts | 103 ++++++++++++++---- 5 files changed, 175 insertions(+), 45 deletions(-) diff --git a/src/main/ipc/filesystem-auth.ts b/src/main/ipc/filesystem-auth.ts index 987c03e477e..dea8a5c023b 100644 --- a/src/main/ipc/filesystem-auth.ts +++ b/src/main/ipc/filesystem-auth.ts @@ -62,28 +62,39 @@ export function isPathAllowed(targetPath: string, store: Store): boolean { } export async function rebuildAuthorizedRootsCache(store: Store): Promise { - const nextRoots = new Set() + // Why: repos are processed in parallel so the cache rebuild completes in + // wall-clock time proportional to the slowest single repo, not the sum of + // all repos. The previous sequential loop was the main bottleneck on + // Windows where each `git worktree list` + realpath chain takes 500 ms+ + // due to slower process creation and antivirus I/O scanning. + const repos = store.getRepos() + const perRepoResults = await Promise.all( + repos.map(async (repo) => { + const roots: string[] = [] + try { + roots.push(await normalizeExistingPath(repo.path)) - for (const repo of store.getRepos()) { - try { - nextRoots.add(await normalizeExistingPath(repo.path)) - - const worktrees = await listRepoWorktrees(repo) - for (const worktree of worktrees) { - nextRoots.add(await normalizeExistingPath(worktree.path)) + const worktrees = await listRepoWorktrees(repo) + const worktreeRoots = await Promise.all( + worktrees.map((wt) => normalizeExistingPath(wt.path)) + ) + roots.push(...worktreeRoots) + } catch (error) { + // Why: a single inaccessible repo (EACCES, EIO, etc.) must not break + // the entire cache rebuild — that would disable File Explorer and + // Quick Open for all other repos. We skip the failing repo and let + // the rest proceed. + console.warn(`[filesystem-auth] skipping repo ${repo.path} during cache rebuild:`, error) } - } catch (error) { - // Why: a single inaccessible repo (EACCES, EIO, etc.) must not break - // the entire cache rebuild — that would disable File Explorer and - // Quick Open for all other repos. We skip the failing repo and let - // the rest proceed. - console.warn(`[filesystem-auth] skipping repo ${repo.path} during cache rebuild:`, error) - } - } + return roots + }) + ) registeredWorktreeRoots.clear() - for (const root of nextRoots) { - registeredWorktreeRoots.add(root) + for (const roots of perRepoResults) { + for (const root of roots) { + registeredWorktreeRoots.add(root) + } } registeredWorktreeRootsDirty = false } diff --git a/src/main/ipc/filesystem-watcher.ts b/src/main/ipc/filesystem-watcher.ts index 640f850cc1a..ef3d352bd12 100644 --- a/src/main/ipc/filesystem-watcher.ts +++ b/src/main/ipc/filesystem-watcher.ts @@ -214,6 +214,12 @@ async function createWatcher(rootKey: string, rootPath: string): Promise { @@ -238,9 +244,16 @@ async function createWatcher(rootKey: string, rootPath: string): Promise { - // Already errored — ignore cleanup failures - }) + // Why: the error callback can fire before `watcher.subscribe()` + // resolves and assigns root.subscription (e.g. the watched root + // is deleted or inaccessible at startup). Guard against null so + // the cleanup path doesn't crash the main process. + if (root.subscription) { + void root.subscription.unsubscribe().catch(() => { + // Already errored — ignore cleanup failures + }) + } + errorCleanedUp = true watchedRoots.delete(rootKey) return } @@ -252,6 +265,15 @@ async function createWatcher(rootKey: string, rootPath: string): Promise {}) + throw new Error(`Watcher for ${rootKey} errored during subscribe`) + } } catch (err) { // Why: if the watcher backend throws synchronously on a deleted root // or permission error, log rather than crashing the main process (§7.3). diff --git a/src/renderer/src/components/right-sidebar/index.tsx b/src/renderer/src/components/right-sidebar/index.tsx index 482f6961f2a..c465f9ccdbd 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, { useMemo } from 'react' +import React, { useEffect, useMemo, useState } from 'react' import { Files, Search, GitBranch, ListChecks } from 'lucide-react' import { useAppStore } from '@/store' import { cn } from '@/lib/utils' @@ -137,6 +137,20 @@ export default function RightSidebar(): 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 + // 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) + }, []) + const activityBarSideWidth = activityBarPosition === 'side' ? ACTIVITY_BAR_SIDE_WIDTH : 0 const { containerRef, isResizing, onResizeStart } = useSidebarResize({ isOpen: rightSidebarOpen, @@ -173,7 +187,7 @@ export default function RightSidebar(): React.JSX.Element { ref={containerRef} className={cn( 'relative flex-shrink-0 flex flex-row overflow-visible', - isResizing ? 'transition-none' : 'transition-[width] duration-200' + isResizing || isMounting ? 'transition-none' : 'transition-[width] duration-200' )} > {/* Panel content area */} diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 18e5a718186..15914f7f477 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -194,12 +194,34 @@ export function connectPanePty( deps.onPtyErrorRef?.current?.(pane.id, message) } + // Why: 512 KB cap keeps the pending buffer from growing without bound + // when an agent runs for minutes in a background worktree. When the + // cap is reached, the oldest output is trimmed so the most recent + // terminal state is preserved. This matches the MAX_BUFFER_BYTES + // constant used for serialized scrollback capture. + const MAX_PENDING_BYTES = 512 * 1024 + const dataCallback = (data: string): void => { if (deps.isActiveRef.current) { pane.terminal.write(data) } else { const pending = deps.pendingWritesRef.current - pending.set(pane.id, (pending.get(pane.id) ?? '') + data) + let buf = (pending.get(pane.id) ?? '') + data + if (buf.length > MAX_PENDING_BYTES) { + // Why: slicing at an arbitrary offset can bisect a multi-byte + // character or an ANSI escape sequence (e.g. \x1b[38;2;255;0m), + // producing garbled output when the buffer is later flushed. + // Snapping forward to the next newline ensures the cut lands on + // a line boundary where escape state is far less likely to be + // mid-sequence. + let cutAt = buf.length - MAX_PENDING_BYTES + const nl = buf.indexOf('\n', cutAt) + if (nl !== -1 && nl < cutAt + 256) { + cutAt = nl + 1 + } + buf = buf.slice(cutAt) + } + pending.set(pane.id, buf) } } 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 f6efa8d638b..e6053a35561 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 @@ -28,6 +28,10 @@ export function useTerminalPaneGlobalEffects({ }: UseTerminalPaneGlobalEffectsArgs): void { const wasActiveRef = useRef(false) + // Why: tracks any in-progress chunked pending-write flush so the cleanup + // function can cancel it if the pane deactivates mid-flush. + const pendingFlushRef = useRef | null>(null) + useEffect(() => { const manager = managerRef.current if (!manager) { @@ -35,17 +39,71 @@ export function useTerminalPaneGlobalEffects({ } if (isActive) { manager.resumeRendering() - for (const [paneId, pendingBuffer] of pendingWritesRef.current.entries()) { - if (pendingBuffer.length > 0) { - const pane = manager.getPanes().find((existingPane) => existingPane.id === paneId) - if (pane) { - pane.terminal.write(pendingBuffer) + + // Why: while a worktree is in the background, PTY output accumulates + // in pendingWritesRef with no size cap. A Claude agent running for + // minutes can produce hundreds of KB. Writing it all in one + // synchronous terminal.write() blocks the renderer for 2–5 s on + // Windows, freezing the UI on every worktree switch. + // + // Fix: drain each pane's pending buffer in 32 KB chunks with a + // setTimeout(0) yield between chunks. This lets the browser paint + // frames and process input events between chunks so the UI stays + // responsive while the scrollback catches up. The fit is deferred + // until after the final chunk so xterm only reflows once. + const CHUNK_SIZE = 32 * 1024 + const entries = Array.from(pendingWritesRef.current.entries()).filter( + ([, buf]) => buf.length > 0 + ) + // Clear all pending buffers immediately so new PTY output arriving + // during the flush goes into a fresh buffer instead of being lost. + for (const [paneId] of entries) { + pendingWritesRef.current.set(paneId, '') + } + + if (entries.length === 0) { + requestAnimationFrame(() => fitAndFocusPanes(manager)) + } else { + let entryIdx = 0 + let offset = 0 + + const drainNextChunk = (): void => { + if (entryIdx >= entries.length) { + pendingFlushRef.current = null + requestAnimationFrame(() => fitAndFocusPanes(manager)) + return } - pendingWritesRef.current.set(paneId, '') + + const [paneId, buffer] = entries[entryIdx] + const pane = manager.getPanes().find((p) => p.id === paneId) + if (!pane) { + entryIdx++ + offset = 0 + pendingFlushRef.current = setTimeout(drainNextChunk, 0) + return + } + + const chunk = buffer.slice(offset, offset + CHUNK_SIZE) + pane.terminal.write(chunk) + offset += CHUNK_SIZE + + if (offset >= buffer.length) { + entryIdx++ + offset = 0 + } + + // Yield to the browser between chunks so the UI stays responsive. + pendingFlushRef.current = setTimeout(drainNextChunk, 0) } + + drainNextChunk() } - requestAnimationFrame(() => fitAndFocusPanes(manager)) } else if (wasActiveRef.current) { + // Cancel any in-progress chunked flush before suspending. + if (pendingFlushRef.current !== null) { + clearTimeout(pendingFlushRef.current) + pendingFlushRef.current = null + } manager.suspendRendering() } wasActiveRef.current = isActive @@ -90,31 +148,34 @@ export function useTerminalPaneGlobalEffects({ // continuous window resizes or layout animations. Each fitPanes() call // triggers fitAddon.fit() → terminal.resize() which, when the column // count changes, reflows the entire scrollback buffer and recalculates - // the viewport scroll position. Rapid-fire reflows can leave the - // viewport at a stale scroll offset, causing the terminal to appear - // scrolled to the top or to show blank space where scrollback should be. - // Batching through requestAnimationFrame coalesces bursts into a single - // reflow per paint frame — the same pattern used by queueResizeAll in - // use-terminal-pane-lifecycle.ts. - let rafId: number | null = null + // the viewport scroll position. On Windows, a single reflow of 10 000 + // scrollback lines can block the renderer for 500 ms–2 s, freezing the + // UI while a sidebar opens or a window resizes. + // + // A trailing-edge debounce (150 ms) coalesces bursts into one reflow + // after the layout settles. This is longer than the previous RAF-only + // batch (≈16 ms) but still short enough that the user never notices the + // terminal running at a stale column count. + const RESIZE_DEBOUNCE_MS = 150 + let timerId: ReturnType | null = null const resizeObserver = new ResizeObserver(() => { - if (rafId !== null) { - return + if (timerId !== null) { + clearTimeout(timerId) } - rafId = requestAnimationFrame(() => { - rafId = null + timerId = setTimeout(() => { + timerId = null const manager = managerRef.current if (!manager) { return } fitPanes(manager) - }) + }, RESIZE_DEBOUNCE_MS) }) resizeObserver.observe(container) return () => { resizeObserver.disconnect() - if (rafId !== null) { - cancelAnimationFrame(rafId) + if (timerId !== null) { + clearTimeout(timerId) } } // eslint-disable-next-line react-hooks/exhaustive-deps