From f72de14dbb94c10b4291b7ad0ff3979dd781d31e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 23 May 2026 15:41:24 -0700 Subject: [PATCH] Revert terminal perf rollback for OpenCode TUIs (#2718) * Revert "Finish terminal perf rollback for OpenCode TUIs (#2716)" This reverts commit 51618f1e924f5a298eb70fda26c07464be854fa6. * fix: repair terminal perf restore regressions --- src/main/ipc/shell.ts | 7 +- src/main/persistence.test.ts | 8 +- src/renderer/src/App.tsx | 4 +- src/renderer/src/components/Terminal.tsx | 214 ++++++++++-------- .../FloatingTerminalPanel.tsx | 29 +-- .../floating-terminal-open-files.test.ts | 38 ++++ .../floating-terminal-open-files.ts | 27 +++ .../settings/RepositoryIconPicker.tsx | 4 +- .../settings/TerminalPane.ghostty.test.ts | 14 +- .../settings/TerminalPane.pwsh.test.ts | 49 ++-- .../remote-runtime-pty-transport.test.ts | 12 +- .../active-worktree-open-files.test.ts | 41 ++++ .../terminal/active-worktree-open-files.ts | 35 +++ .../terminal-browser-pane-worktrees.test.ts | 86 +++++++ .../terminal-browser-pane-worktrees.ts | 54 +++++ .../terminal-browser-tab-slices.test.ts | 75 ++++++ .../terminal/terminal-browser-tab-slices.ts | 68 ++++++ .../terminal-mounted-worktrees.test.ts | 126 +++++++++++ .../terminal/terminal-mounted-worktrees.ts | 69 ++++++ .../terminal/terminal-tab-slices.test.ts | 61 +++++ .../terminal/terminal-tab-slices.ts | 67 ++++++ .../slices/repos-update-serialization.test.ts | 4 +- src/shared/repo-icon.test.ts | 24 +- src/shared/repo-icon.ts | 2 +- 24 files changed, 957 insertions(+), 161 deletions(-) create mode 100644 src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts create mode 100644 src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts create mode 100644 src/renderer/src/components/terminal/active-worktree-open-files.test.ts create mode 100644 src/renderer/src/components/terminal/active-worktree-open-files.ts create mode 100644 src/renderer/src/components/terminal/terminal-browser-pane-worktrees.test.ts create mode 100644 src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts create mode 100644 src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts create mode 100644 src/renderer/src/components/terminal/terminal-browser-tab-slices.ts create mode 100644 src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts create mode 100644 src/renderer/src/components/terminal/terminal-mounted-worktrees.ts create mode 100644 src/renderer/src/components/terminal/terminal-tab-slices.test.ts create mode 100644 src/renderer/src/components/terminal/terminal-tab-slices.ts diff --git a/src/main/ipc/shell.ts b/src/main/ipc/shell.ts index 37048c650..4712828a5 100644 --- a/src/main/ipc/shell.ts +++ b/src/main/ipc/shell.ts @@ -11,7 +11,8 @@ import { getSpawnArgsForWindows } from '../win32-utils' export const EXTERNAL_EDITOR_CLI_COMMAND = 'code' const REPO_ICON_IMAGE_MIME_TYPES: Record = { - '.png': 'image/png' + '.png': 'image/png', + '.svg': 'image/svg+xml' } async function pathExists(pathValue: string): Promise { @@ -247,7 +248,7 @@ export function registerShellHandlers(): void { async (): Promise<{ dataUrl: string; fileName: string } | null> => { const result = await dialog.showOpenDialog({ properties: ['openFile'], - filters: [{ name: 'Repo icon images', extensions: ['png'] }] + filters: [{ name: 'Repo icon images', extensions: ['png', 'svg'] }] }) if (result.canceled || result.filePaths.length === 0) { return null @@ -257,7 +258,7 @@ export function registerShellHandlers(): void { const extension = extname(filePath).toLowerCase() const mimeType = REPO_ICON_IMAGE_MIME_TYPES[extension] if (!mimeType) { - throw new Error('Repo icons must be PNG files.') + throw new Error('Repo icons must be PNG or SVG files.') } const stats = await stat(filePath) diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index f3a1c2fd7..a2345bea7 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -1383,8 +1383,8 @@ describe('Store', () => { const updated = store.updateRepo('r1', { repoIcon: { type: 'image', - source: 'upload', - src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' + source: 'github', + src: 'https://example.com/icon.png' } as never }) @@ -1399,8 +1399,8 @@ describe('Store', () => { makeRepo({ repoIcon: { type: 'image', - source: 'upload', - src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' + source: 'github', + src: 'https://example.com/icon.png' } as never }) ) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 19bdbd54e..228e28486 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -108,6 +108,7 @@ import { canGoBackWorktreeHistory, canGoForwardWorktreeHistory } from '@/store/slices/worktree-nav-history' +import { useActiveTerminalTabs } from './store/selectors' import type { VirtualizedScrollAnchor } from './hooks/useVirtualizedScrollAnchor' import type { RemoteWorkspacePatchResult } from '../../shared/remote-workspace-types' import type { OnboardingState } from '../../shared/types' @@ -307,7 +308,7 @@ function App(): React.JSX.Element { // that remount so the left workspace list doesn't restart at scrollTop 0. const worktreeSidebarScrollOffsetRef = useRef(0) const worktreeSidebarScrollAnchorRef = useRef(null) - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) + const tabs = useActiveTerminalTabs() const floatingUnifiedTabCount = useAppStore( (s) => s.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID]?.length ?? 0 ) @@ -963,7 +964,6 @@ function App(): React.JSX.Element { return () => document.removeEventListener('visibilitychange', handler) }, [actions]) - const tabs = activeWorktreeId ? (tabsByWorktree[activeWorktreeId] ?? []) : [] const hasTabBar = tabs.length >= 2 const effectiveActiveTabId = activeTabId ?? tabs[0]?.id ?? null const activeTabCanExpand = effectiveActiveTabId diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 88acbca21..befc287d2 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ -import React, { useEffect, useCallback, useMemo, useRef, useState, lazy, Suspense } from 'react' +import React, { useEffect, useCallback, useRef, useState, lazy, Suspense } from 'react' import { createPortal } from 'react-dom' import { toast } from 'sonner' import { @@ -9,7 +9,6 @@ import { type BackgroundMountTerminalWorktreeDetail } from '@/constants/terminal' import { useAppStore } from '../store' -import { useAllWorktrees } from '../store/selectors' import { createUntitledMarkdownFile } from '../lib/create-untitled-markdown' import { getConnectionId } from '../lib/connection-context' import { extractIpcErrorMessage } from '../lib/ipc-error' @@ -50,8 +49,16 @@ import { handleSwitchTerminalTab } from '../hooks/ipc-tab-switch' import TabGroupSplitLayout from './tab-group/TabGroupSplitLayout' +import { getActiveWorktreeOpenFiles } from './terminal/active-worktree-open-files' import { shouldAutoCreateInitialTerminal } from './terminal/initial-terminal' import { shouldRepairActiveTerminalTab } from './terminal/active-terminal-repair' +import { + getTerminalBrowserPaneWorktreeIds, + shouldRenderPreReadyBrowserPaneFallback +} from './terminal/terminal-browser-pane-worktrees' +import { getTerminalBrowserTabSlices } from './terminal/terminal-browser-tab-slices' +import { getTerminalMountedWorktreeSnapshot } from './terminal/terminal-mounted-worktrees' +import { getTerminalTabSlices } from './terminal/terminal-tab-slices' import { addBackgroundMountedTerminalWorktree } from './terminal/background-terminal-worktree-mount' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { @@ -101,10 +108,8 @@ function getKeybindingContext(target: EventTarget | null): KeybindingContext { } function Terminal(): React.JSX.Element | null { - const allWorktrees = useAllWorktrees() const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const activeView = useAppStore((s) => s.activeView) - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) const activeTabId = useAppStore((s) => s.activeTabId) const createTab = useAppStore((s) => s.createTab) const closeTab = useAppStore((s) => s.closeTab) @@ -118,7 +123,37 @@ function Terminal(): React.JSX.Element | null { const consumeSuppressedPtyExit = useAppStore((s) => s.consumeSuppressedPtyExit) const expandedPaneByTabId = useAppStore((s) => s.expandedPaneByTabId) const workspaceSessionReady = useAppStore((s) => s.workspaceSessionReady) - const openFiles = useAppStore((s) => s.openFiles) + // Track which worktrees have been activated during this app session. + // Only mount TerminalPanes for visited worktrees to prevent mass PTY + // spawning when restoring a session with many saved worktree tabs. + const mountedWorktreeIdsRef = useRef(new Set()) + const measurableBackgroundWorktreeIdsRef = useRef(new Set()) + const measurableBackgroundWorktreeTimersRef = useRef(new Map()) + const [, setBackgroundMountRevision] = useState(0) + // Why: gated on workspaceSessionReady to prevent TerminalPane from mounting + // before reconnectPersistedTerminals() has finished eagerly spawning PTYs. + // Without this gate, Phase 1 (hydrateWorkspaceSession) sets activeWorktreeId + // with ptyId: null, and TerminalPane would call connectPanePty -> pty:spawn, + // creating a duplicate PTY for the same tab. + if (activeWorktreeId && workspaceSessionReady) { + mountedWorktreeIdsRef.current.add(activeWorktreeId) + } + const terminalWorktreeSnapshot = useAppStore((s) => + getTerminalMountedWorktreeSnapshot(s.worktreesByRepo, mountedWorktreeIdsRef.current) + ) + const terminalTabSlices = useAppStore((s) => + getTerminalTabSlices(s.tabsByWorktree, mountedWorktreeIdsRef.current, activeWorktreeId) + ) + const terminalBrowserTabSlices = useAppStore((s) => + getTerminalBrowserTabSlices( + s.browserTabsByWorktree, + mountedWorktreeIdsRef.current, + activeWorktreeId + ) + ) + const worktreeFiles = useAppStore((s) => + getActiveWorktreeOpenFiles(s.openFiles, activeWorktreeId) + ) const activeFileId = useAppStore((s) => s.activeFileId) const activeBrowserTabId = useAppStore((s) => s.activeBrowserTabId) const activeTabType = useAppStore((s) => s.activeTabType) @@ -131,7 +166,6 @@ function Terminal(): React.JSX.Element | null { const openFile = useAppStore((s) => s.openFile) const closeFile = useAppStore((s) => s.closeFile) const pinFile = useAppStore((s) => s.pinFile) - const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) const createBrowserTab = useAppStore((s) => s.createBrowserTab) const closeBrowserTab = useAppStore((s) => s.closeBrowserTab) const setActiveBrowserTab = useAppStore((s) => s.setActiveBrowserTab) @@ -155,10 +189,7 @@ function Terminal(): React.JSX.Element | null { activeView === 'activity' ) - const tabs = useMemo( - () => (activeWorktreeId ? (tabsByWorktree[activeWorktreeId] ?? []) : []), - [activeWorktreeId, tabsByWorktree] - ) + const tabs = terminalTabSlices.activeTabs // Why: the TabBar is rendered into the titlebar via a portal so tabs share // the same row as the "Orca" title. The target element is created by App.tsx. @@ -169,6 +200,9 @@ function Terminal(): React.JSX.Element | null { }, []) useEffect(() => { + if (!workspaceSessionReady) { + return + } if (!activeWorktreeId) { return } @@ -176,15 +210,9 @@ function Terminal(): React.JSX.Element | null { // worktree always has a root group so terminal-first fallback can attach // fresh tabs to a concrete owner even before any explicit split exists. ensureWorktreeRootGroup(activeWorktreeId) - }, [activeWorktreeId, ensureWorktreeRootGroup]) + }, [activeWorktreeId, ensureWorktreeRootGroup, workspaceSessionReady]) - // Filter editor files to only show those belonging to the active worktree - const worktreeFiles = activeWorktreeId - ? openFiles.filter((f) => f.worktreeId === activeWorktreeId) - : [] - const worktreeBrowserTabs = activeWorktreeId - ? (browserTabsByWorktree[activeWorktreeId] ?? []) - : [] + const worktreeBrowserTabs = terminalBrowserTabSlices.activeBrowserTabs const getEffectiveLayoutForWorktree = useCallback( (worktreeId: string) => getEffectiveLayout(worktreeId, layoutByWorktree, groupsByWorktree, activeGroupIdByWorktree), @@ -193,13 +221,20 @@ function Terminal(): React.JSX.Element | null { const effectiveActiveLayout = activeWorktreeId ? getEffectiveLayoutForWorktree(activeWorktreeId) : undefined - const activeWorktreeBrowserTabIdsKey = activeWorktreeId - ? (browserTabsByWorktree[activeWorktreeId] ?? []).map((tab) => tab.id).join(',') - : '' + const activeWorktreeBrowserTabIdsKey = worktreeBrowserTabs.map((tab) => tab.id).join(',') + const browserPaneWorktreeIds = getTerminalBrowserPaneWorktreeIds({ + mountedWorktreeIds: terminalWorktreeSnapshot.mountedWorktrees.map((worktree) => worktree.id), + worktreeIds: terminalWorktreeSnapshot.worktreeIds, + activeWorktreeId, + activeTabType, + activeBrowserTabCount: worktreeBrowserTabs.length + }) // Save confirmation dialog state const [saveDialogFileId, setSaveDialogFileId] = useState(null) - const saveDialogFile = saveDialogFileId ? openFiles.find((f) => f.id === saveDialogFileId) : null + const saveDialogFile = useAppStore((s) => + saveDialogFileId ? (s.openFiles.find((file) => file.id === saveDialogFileId) ?? null) : null + ) const pendingEditorCloseQueueRef = useRef([]) // Why: while a save-and-close is awaiting the file to disappear from @@ -532,13 +567,6 @@ function Terminal(): React.JSX.Element | null { // eslint-disable-next-line react-hooks/exhaustive-deps }, [activeTabId, activeTabType, setActiveTab, tabs]) - // Track which worktrees have been activated during this app session. - // Only mount TerminalPanes for visited worktrees to prevent mass PTY - // spawning when restoring a session with many saved worktree tabs. - const mountedWorktreeIdsRef = useRef(new Set()) - const measurableBackgroundWorktreeIdsRef = useRef(new Set()) - const measurableBackgroundWorktreeTimersRef = useRef(new Map()) - const [, setBackgroundMountRevision] = useState(0) useEffect(() => { const timers = measurableBackgroundWorktreeTimersRef.current const onBackgroundMountTerminalWorktree = (event: Event): void => { @@ -581,28 +609,35 @@ function Terminal(): React.JSX.Element | null { timers.clear() } }, []) - // Why: gated on workspaceSessionReady to prevent TerminalPane from mounting - // before reconnectPersistedTerminals() has finished eagerly spawning PTYs. - // Without this gate, Phase 1 (hydrateWorkspaceSession) sets activeWorktreeId - // with ptyId: null, and TerminalPane would call connectPanePty → pty:spawn, - // creating a duplicate PTY for the same tab. - if (activeWorktreeId && workspaceSessionReady) { - mountedWorktreeIdsRef.current.add(activeWorktreeId) - } // Prune IDs of worktrees that no longer exist (deleted/removed) - const allWorktreeIds = new Set(allWorktrees.map((wt) => wt.id)) + const allWorktreeIds = new Set(terminalWorktreeSnapshot.worktreeIds) for (const id of mountedWorktreeIdsRef.current) { if (!allWorktreeIds.has(id)) { mountedWorktreeIdsRef.current.delete(id) } } const anyMountedWorktreeHasLayout = computeAnyMountedWorktreeHasLayout( - allWorktrees.map((wt) => wt.id), + terminalWorktreeSnapshot.worktreeIds, mountedWorktreeIdsRef.current, layoutByWorktree, groupsByWorktree, activeGroupIdByWorktree ) + const activeWorktreeMounted = + activeWorktreeId !== null && mountedWorktreeIdsRef.current.has(activeWorktreeId) + const shouldRenderLegacyWorkspaceSurface = !effectiveActiveLayout && !anyMountedWorktreeHasLayout + const shouldRenderPreReadyBrowserSurface = shouldRenderPreReadyBrowserPaneFallback({ + worktreeIds: terminalWorktreeSnapshot.worktreeIds, + activeWorktreeId, + activeTabType, + activeBrowserTabCount: worktreeBrowserTabs.length, + activeWorktreeMounted + }) + const browserPaneWorktreeIdsForLegacySurface = shouldRenderLegacyWorkspaceSurface + ? browserPaneWorktreeIds + : shouldRenderPreReadyBrowserSurface && activeWorktreeId + ? [activeWorktreeId] + : [] // Auto-create first tab when worktree activates useEffect(() => { if (!workspaceSessionReady) { @@ -1477,35 +1512,33 @@ function Terminal(): React.JSX.Element | null { can preserve hidden trees without reflowing the active one. Keep a relative anchor here so those panes size to the workspace body rather than some outer ancestor when split groups are enabled. */} - {allWorktrees - .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id)) - .map((worktree) => { - const layout = getEffectiveLayoutForWorktree(worktree.id) - if (!layout) { - return null - } - // Why: use strict equality with 'terminal' instead of !== 'settings' - // so the terminal/browser surface hides on the tasks page too. - const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId - const shouldMeasureHiddenWorktree = - !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) - return ( - - ) - })} + {terminalWorktreeSnapshot.mountedWorktrees.map((worktree) => { + const layout = getEffectiveLayoutForWorktree(worktree.id) + if (!layout) { + return null + } + // Why: use strict equality with 'terminal' instead of !== 'settings' + // so the terminal/browser surface hides on the tasks page too. + const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId + const shouldMeasureHiddenWorktree = + !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) + return ( + + ) + })} ) : null} - {!effectiveActiveLayout && !anyMountedWorktreeHasLayout && ( + {(shouldRenderLegacyWorkspaceSurface || shouldRenderPreReadyBrowserSurface) && ( <> {/* Why: split-group layouts render their own terminal/browser/editor surfaces through TabGroupPanel plus stable overlay layers. @@ -1525,23 +1558,21 @@ function Terminal(): React.JSX.Element | null { startFreshSpawn → new PTY. That respawn is exactly what flips getWorktreeStatus back to 'active' and re-lights the sidebar dot green moments after the user clicked Shutdown. */} - {/* Terminal panes container - hidden when editor tab active */} -
0) || - (activeTabType === 'browser' && worktreeBrowserTabs.length > 0) - ? 'hidden' - : '' - }`} - > - {allWorktrees - .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id)) - .map((worktree) => { + {shouldRenderLegacyWorkspaceSurface ? ( +
0) || + (activeTabType === 'browser' && worktreeBrowserTabs.length > 0) + ? 'hidden' + : '' + }`} + > + {terminalWorktreeSnapshot.mountedWorktrees.map((worktree) => { // Why: use strict equality with 'terminal' instead of !== 'settings' // so the terminal/browser surface hides on the tasks page too. const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId @@ -1560,7 +1591,7 @@ function Terminal(): React.JSX.Element | null { aria-hidden={!isVisible} > - {(tabsByWorktree[worktree.id] ?? []).map((tab) => { + {(terminalTabSlices.mountedTabsByWorktree[worktree.id] ?? []).map((tab) => { const activityTerminalPortal = findActivityTerminalPortal( activityTerminalPortals, { worktreeId: worktree.id, tabId: tab.id } @@ -1582,7 +1613,7 @@ function Terminal(): React.JSX.Element | null { isVisible={isActiveTerminalTab || isActivityPortalTab} // Why: when portaled to Activity for a specific agent // pane, isolate that leaf so split siblings stay - // hidden. Workspace renders pass null → no override. + // hidden. Workspace renders pass null -> no override. isolatedPaneKey={activityTerminalPortal?.paneKey ?? null} onPtyExit={(ptyId) => handlePtyExit(tab.id, ptyId)} onCloseTab={() => handleCloseTab(tab.id)} @@ -1600,7 +1631,8 @@ function Terminal(): React.JSX.Element | null {
) })} -
+ + ) : null} {/* Browser panes container — all browser panes for the active worktree stay mounted so webview DOM state (scroll position, form inputs, etc.) @@ -1610,18 +1642,20 @@ function Terminal(): React.JSX.Element | null { activeTabType !== 'browser' ? 'hidden' : '' }`} > - {allWorktrees.map((worktree) => { - const browserTabs = browserTabsByWorktree[worktree.id] ?? [] + {browserPaneWorktreeIdsForLegacySurface.map((worktreeId) => { + const browserTabs = + worktreeId === activeWorktreeId + ? worktreeBrowserTabs + : (terminalBrowserTabSlices.mountedBrowserTabsByWorktree[worktreeId] ?? []) // Why: use strict equality with 'terminal' instead of !== 'settings' // so browser panes also hide on the tasks page. - const isVisibleWorktree = - activeView === 'terminal' && worktree.id === activeWorktreeId + const isVisibleWorktree = activeView === 'terminal' && worktreeId === activeWorktreeId if (browserTabs.length === 0) { return null } return (
diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index 760e933ee..ce19eefb1 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -64,6 +64,7 @@ import { getMaximizedFloatingTerminalBounds, type FloatingTerminalPanelBounds } from './floating-terminal-panel-bounds' +import { getFloatingTerminalOpenFiles } from './floating-terminal-open-files' const EMPTY_TERMINAL_TABS: TerminalTab[] = [] const EMPTY_BROWSER_TABS: BrowserTabState[] = [] const EMPTY_GROUPS: TabGroup[] = [] @@ -89,11 +90,19 @@ export function FloatingTerminalPanel({ open, onOpenChange }: FloatingTerminalPanelProps): React.JSX.Element | null { - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) - const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) - const groupsByWorktree = useAppStore((s) => s.groupsByWorktree) - const unifiedTabsByWorktree = useAppStore((s) => s.unifiedTabsByWorktree) - const openFiles = useAppStore((s) => s.openFiles) + const tabs = useAppStore( + (s) => s.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_TERMINAL_TABS + ) + const browserTabs = useAppStore( + (s) => s.browserTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_BROWSER_TABS + ) + const groups = useAppStore( + (s) => s.groupsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_GROUPS + ) + const unifiedTabs = useAppStore( + (s) => s.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_UNIFIED_TABS + ) + const floatingFiles = useAppStore((s) => getFloatingTerminalOpenFiles(s.openFiles)) const expandedPaneByTabId = useAppStore((s) => s.expandedPaneByTabId) const createTab = useAppStore((s) => s.createTab) const createBrowserTab = useAppStore((s) => s.createBrowserTab) @@ -132,14 +141,6 @@ export function FloatingTerminalPanel({ top: number } | null>(null) - const tabs = tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_TERMINAL_TABS - const browserTabs = browserTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_BROWSER_TABS - const groups = groupsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_GROUPS - const unifiedTabs = unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_UNIFIED_TABS - const floatingFiles = useMemo( - () => openFiles.filter((file) => file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID), - [openFiles] - ) const activeGroup = useMemo( () => groups.find((group) => group.activeTabId != null) ?? @@ -242,7 +243,7 @@ export function FloatingTerminalPanel({ handleSaveDialogSave, handleSaveDialogDiscard, handleSaveDialogCancel - } = useTerminalSaveDialog({ openFiles, closeFile, markFileDirty }) + } = useTerminalSaveDialog({ openFiles: floatingFiles, closeFile, markFileDirty }) const getNextQueuedEditorClose = useCallback((): string | null => { while (pendingEditorCloseQueueRef.current.length > 0) { diff --git a/src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts new file mode 100644 index 000000000..9f1eecf89 --- /dev/null +++ b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import { getFloatingTerminalOpenFiles } from './floating-terminal-open-files' + +const file = (id: string, worktreeId: string): OpenFile => + ({ + id, + filePath: `/tmp/${id}.md`, + relativePath: `${id}.md`, + worktreeId, + language: 'markdown', + content: '', + isDirty: false, + isPinned: false, + mode: 'edit', + mtime: 0, + runtimeEnvironmentId: null + }) as OpenFile + +describe('getFloatingTerminalOpenFiles', () => { + it('preserves the filtered array when unrelated worktree files change', () => { + const floating = file('floating', FLOATING_TERMINAL_WORKTREE_ID) + const first = getFloatingTerminalOpenFiles([floating, file('main-a', 'wt-1')]) + const second = getFloatingTerminalOpenFiles([floating, file('main-b', 'wt-2')]) + + expect(second).toBe(first) + expect(second).toEqual([floating]) + }) + + it('updates the filtered array when a floating file changes', () => { + const first = getFloatingTerminalOpenFiles([file('floating-a', FLOATING_TERMINAL_WORKTREE_ID)]) + const second = getFloatingTerminalOpenFiles([file('floating-b', FLOATING_TERMINAL_WORKTREE_ID)]) + + expect(second).not.toBe(first) + expect(second.map((item) => item.id)).toEqual(['floating-b']) + }) +}) diff --git a/src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts new file mode 100644 index 000000000..155cee881 --- /dev/null +++ b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts @@ -0,0 +1,27 @@ +import type { OpenFile } from '@/store/slices/editor' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' + +let cachedOpenFiles: OpenFile[] | null = null +let cachedFloatingFiles: OpenFile[] = [] + +export function getFloatingTerminalOpenFiles(openFiles: OpenFile[]): OpenFile[] { + if (openFiles === cachedOpenFiles) { + return cachedFloatingFiles + } + + const nextFloatingFiles = openFiles.filter( + (file) => file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID + ) + if ( + cachedOpenFiles !== null && + nextFloatingFiles.length === cachedFloatingFiles.length && + nextFloatingFiles.every((file, index) => file === cachedFloatingFiles[index]) + ) { + cachedOpenFiles = openFiles + return cachedFloatingFiles + } + + cachedOpenFiles = openFiles + cachedFloatingFiles = nextFloatingFiles + return cachedFloatingFiles +} diff --git a/src/renderer/src/components/settings/RepositoryIconPicker.tsx b/src/renderer/src/components/settings/RepositoryIconPicker.tsx index a0f90a20a..5931d3afe 100644 --- a/src/renderer/src/components/settings/RepositoryIconPicker.tsx +++ b/src/renderer/src/components/settings/RepositoryIconPicker.tsx @@ -230,7 +230,7 @@ export function RepositoryIconPicker({ onClick={handleUploadImage} > - Upload PNG + Upload PNG/SVG
-

PNG uploads must be 256KB or smaller.

+

PNG/SVG uploads must be 256KB or smaller.

diff --git a/src/renderer/src/components/settings/TerminalPane.ghostty.test.ts b/src/renderer/src/components/settings/TerminalPane.ghostty.test.ts index ba86793f6..3d62f77a4 100644 --- a/src/renderer/src/components/settings/TerminalPane.ghostty.test.ts +++ b/src/renderer/src/components/settings/TerminalPane.ghostty.test.ts @@ -80,21 +80,17 @@ vi.mock('../ui/toggle-group', () => ({ })) vi.mock('./SettingsFormControls', () => ({ - SettingsRow: function SettingsRow({ children }: { children?: unknown }) { - return children - }, NumberField: function NumberField() { return null }, FontAutocomplete: function FontAutocomplete() { return null }, - SettingsSegmentedControl: function SettingsSegmentedControl({ - options - }: { - options?: readonly { label: string }[] - }) { - return options?.map((option) => option.label) ?? null + SettingsRow: function SettingsRow() { + return null + }, + SettingsSegmentedControl: function SettingsSegmentedControl() { + return null }, SettingsSubsectionHeader: function SettingsSubsectionHeader() { return null diff --git a/src/renderer/src/components/settings/TerminalPane.pwsh.test.ts b/src/renderer/src/components/settings/TerminalPane.pwsh.test.ts index 804d2038b..53293e7a2 100644 --- a/src/renderer/src/components/settings/TerminalPane.pwsh.test.ts +++ b/src/renderer/src/components/settings/TerminalPane.pwsh.test.ts @@ -80,29 +80,17 @@ vi.mock('../ui/toggle-group', () => ({ })) vi.mock('./SettingsFormControls', () => ({ - SettingsRow: function SettingsRow({ - description, - control, - children - }: { - description?: unknown - control?: unknown - children?: unknown - }) { - return [description, control, children] - }, NumberField: function NumberField() { return null }, FontAutocomplete: function FontAutocomplete() { return null }, - SettingsSegmentedControl: function SettingsSegmentedControl({ - options - }: { - options?: readonly { label: string }[] - }) { - return options?.map((option) => option.label) ?? null + SettingsRow: function SettingsRow() { + return null + }, + SettingsSegmentedControl: function SettingsSegmentedControl() { + return null }, SettingsSubsectionHeader: function SettingsSubsectionHeader() { return null @@ -164,13 +152,24 @@ type ReactElementLike = { props: Record } -function getPropNodes(el: ReactElementLike): unknown[] { - const nodes = [el.props?.children, el.props?.description, el.props?.control] - const options = el.props?.options - if (Array.isArray(options)) { - nodes.push(options.map((option) => (option as { label?: unknown }).label)) +function getPropNodes(props: Record | undefined): unknown[] { + if (!props) { + return [] } - return nodes + const optionLabels = Array.isArray(props.options) + ? props.options.map((option) => + option && typeof option === 'object' ? (option as { label?: unknown }).label : undefined + ) + : [] + return [ + props.children, + props.title, + props.label, + props.description, + props.control, + props.action, + ...optionLabels + ] } function collectText(node: unknown): string { @@ -187,7 +186,7 @@ function collectText(node: unknown): string { return node.map(collectText).join('') } const el = node as ReactElementLike - return getPropNodes(el).map(collectText).join('') + return getPropNodes(el.props).map(collectText).join('') } function findAnchorByText(node: unknown, text: string): ReactElementLike | null { @@ -211,7 +210,7 @@ function findAnchorByText(node: unknown, text: string): ReactElementLike | null if (typeName === 'a' && collectText(el.props.children).includes(text)) { return el } - for (const child of getPropNodes(el)) { + for (const child of getPropNodes(el.props)) { const found = findAnchorByText(child, text) if (found) { return found diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index cc10dd150..2f6c14c69 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -515,13 +515,13 @@ describe('createRemoteRuntimePtyTransport', () => { 'before\x1b]9999;{"state":"working","prompt":"ship it","agentType":"codex"}\x07after\x1b]0;. Claude working\x07\x07' ) - await vi.waitFor(() => + await vi.waitFor(() => { expect(onAgentStatus).toHaveBeenCalledWith({ state: 'working', prompt: 'ship it', agentType: 'codex' }) - ) + }) expect(onData).toHaveBeenCalledWith('beforeafter\x1b]0;. Claude working\x07\x07') expect(onTitleChange).toHaveBeenCalledWith('. Claude working', '. Claude working') expect(onBell).toHaveBeenCalledTimes(1) @@ -548,13 +548,13 @@ describe('createRemoteRuntimePtyTransport', () => { 'before\x1b]9999;{"state":"working","prompt":"ship it","agentType":"codex"}\x07after' ) - await vi.waitFor(() => + await vi.waitFor(() => { expect(onAgentStatus).toHaveBeenCalledWith({ state: 'working', prompt: 'ship it', agentType: 'codex' }) - ) + }) expect(onData).toHaveBeenCalledWith('beforeafter') }) @@ -856,9 +856,9 @@ describe('createRemoteRuntimePtyTransport', () => { ) expect(onReplayData).toHaveBeenCalledWith('beforeafter\x1b]0;Remote title\x07\x07') - await vi.waitFor(() => + await vi.waitFor(() => { expect(onTitleChange).toHaveBeenCalledWith('Remote title', 'Remote title') - ) + }) expect(onAgentStatus).not.toHaveBeenCalled() expect(onBell).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/components/terminal/active-worktree-open-files.test.ts b/src/renderer/src/components/terminal/active-worktree-open-files.test.ts new file mode 100644 index 000000000..91600ef39 --- /dev/null +++ b/src/renderer/src/components/terminal/active-worktree-open-files.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { getActiveWorktreeOpenFiles } from './active-worktree-open-files' + +const file = (id: string, worktreeId: string): OpenFile => + ({ + id, + filePath: `/tmp/${id}.md`, + relativePath: `${id}.md`, + worktreeId, + language: 'markdown', + isDirty: false, + runtimeEnvironmentId: null + }) as OpenFile + +describe('getActiveWorktreeOpenFiles', () => { + it('preserves the active slice when unrelated worktree files change', () => { + const active = file('active', 'wt-active') + const first = getActiveWorktreeOpenFiles([active, file('other-a', 'wt-other')], 'wt-active') + const second = getActiveWorktreeOpenFiles([active, file('other-b', 'wt-other')], 'wt-active') + + expect(second).toBe(first) + expect(second).toEqual([active]) + }) + + it('updates the active slice when an active file changes', () => { + const first = getActiveWorktreeOpenFiles([file('active-a', 'wt-active')], 'wt-active') + const second = getActiveWorktreeOpenFiles([file('active-b', 'wt-active')], 'wt-active') + + expect(second).not.toBe(first) + expect(second.map((item) => item.id)).toEqual(['active-b']) + }) + + it('returns a stable empty slice without an active worktree', () => { + const first = getActiveWorktreeOpenFiles([file('active', 'wt-active')], null) + const second = getActiveWorktreeOpenFiles([file('other', 'wt-other')], null) + + expect(second).toBe(first) + expect(second).toEqual([]) + }) +}) diff --git a/src/renderer/src/components/terminal/active-worktree-open-files.ts b/src/renderer/src/components/terminal/active-worktree-open-files.ts new file mode 100644 index 000000000..48f5342b5 --- /dev/null +++ b/src/renderer/src/components/terminal/active-worktree-open-files.ts @@ -0,0 +1,35 @@ +import type { OpenFile } from '@/store/slices/editor' + +const EMPTY_OPEN_FILES: OpenFile[] = [] + +let cachedOpenFiles: OpenFile[] | null = null +let cachedWorktreeId: string | null = null +let cachedFiles: OpenFile[] = EMPTY_OPEN_FILES + +export function getActiveWorktreeOpenFiles( + openFiles: OpenFile[], + activeWorktreeId: string | null +): OpenFile[] { + if (!activeWorktreeId) { + return EMPTY_OPEN_FILES + } + if (openFiles === cachedOpenFiles && activeWorktreeId === cachedWorktreeId) { + return cachedFiles + } + + const nextFiles = openFiles.filter((file) => file.worktreeId === activeWorktreeId) + if ( + cachedOpenFiles !== null && + activeWorktreeId === cachedWorktreeId && + nextFiles.length === cachedFiles.length && + nextFiles.every((file, index) => file === cachedFiles[index]) + ) { + cachedOpenFiles = openFiles + return cachedFiles + } + + cachedOpenFiles = openFiles + cachedWorktreeId = activeWorktreeId + cachedFiles = nextFiles.length > 0 ? nextFiles : EMPTY_OPEN_FILES + return cachedFiles +} diff --git a/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.test.ts b/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.test.ts new file mode 100644 index 000000000..600eec9d7 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest' +import { + getTerminalBrowserPaneWorktreeIds, + shouldRenderPreReadyBrowserPaneFallback +} from './terminal-browser-pane-worktrees' + +describe('getTerminalBrowserPaneWorktreeIds', () => { + it('returns mounted worktrees unchanged for normal mounted browser panes', () => { + const mounted = ['wt-active'] + + const result = getTerminalBrowserPaneWorktreeIds({ + mountedWorktreeIds: mounted, + worktreeIds: ['wt-active'], + activeWorktreeId: 'wt-active', + activeTabType: 'browser', + activeBrowserTabCount: 1 + }) + + expect(result).toBe(mounted) + }) + + it('adds the active browser worktree before terminal panes are allowed to mount', () => { + const result = getTerminalBrowserPaneWorktreeIds({ + mountedWorktreeIds: [], + worktreeIds: ['wt-active'], + activeWorktreeId: 'wt-active', + activeTabType: 'browser', + activeBrowserTabCount: 1 + }) + + expect(result).toEqual(['wt-active']) + }) + + it('does not add missing or non-browser active worktrees', () => { + expect( + getTerminalBrowserPaneWorktreeIds({ + mountedWorktreeIds: [], + worktreeIds: ['wt-other'], + activeWorktreeId: 'wt-active', + activeTabType: 'browser', + activeBrowserTabCount: 1 + }) + ).toEqual([]) + + expect( + getTerminalBrowserPaneWorktreeIds({ + mountedWorktreeIds: [], + worktreeIds: ['wt-active'], + activeWorktreeId: 'wt-active', + activeTabType: 'terminal', + activeBrowserTabCount: 1 + }) + ).toEqual([]) + }) + + it('flags only active unmounted browser worktrees for pre-ready fallback rendering', () => { + expect( + shouldRenderPreReadyBrowserPaneFallback({ + worktreeIds: ['wt-active'], + activeWorktreeId: 'wt-active', + activeTabType: 'browser', + activeBrowserTabCount: 1, + activeWorktreeMounted: false + }) + ).toBe(true) + + expect( + shouldRenderPreReadyBrowserPaneFallback({ + worktreeIds: ['wt-active'], + activeWorktreeId: 'wt-active', + activeTabType: 'browser', + activeBrowserTabCount: 1, + activeWorktreeMounted: true + }) + ).toBe(false) + expect( + shouldRenderPreReadyBrowserPaneFallback({ + worktreeIds: ['wt-active'], + activeWorktreeId: 'wt-active', + activeTabType: 'terminal', + activeBrowserTabCount: 1, + activeWorktreeMounted: false + }) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts b/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts new file mode 100644 index 000000000..9c28c26cb --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts @@ -0,0 +1,54 @@ +import type { WorkspaceVisibleTabType } from '../../../../shared/types' + +export type TerminalBrowserPaneWorktreeInput = { + mountedWorktreeIds: string[] + worktreeIds: string[] + activeWorktreeId: string | null + activeTabType: WorkspaceVisibleTabType + activeBrowserTabCount: number +} + +export type TerminalBrowserPaneFallbackInput = Omit< + TerminalBrowserPaneWorktreeInput, + 'mountedWorktreeIds' +> & { + activeWorktreeMounted: boolean +} + +export function shouldRenderPreReadyBrowserPaneFallback({ + worktreeIds, + activeWorktreeId, + activeTabType, + activeBrowserTabCount, + activeWorktreeMounted +}: TerminalBrowserPaneFallbackInput): boolean { + return ( + activeWorktreeId !== null && + activeTabType === 'browser' && + activeBrowserTabCount > 0 && + !activeWorktreeMounted && + worktreeIds.includes(activeWorktreeId) + ) +} + +export function getTerminalBrowserPaneWorktreeIds({ + mountedWorktreeIds, + worktreeIds, + activeWorktreeId, + activeTabType, + activeBrowserTabCount +}: TerminalBrowserPaneWorktreeInput): string[] { + if ( + activeWorktreeId === null || + activeTabType !== 'browser' || + activeBrowserTabCount === 0 || + mountedWorktreeIds.includes(activeWorktreeId) || + !worktreeIds.includes(activeWorktreeId) + ) { + return mountedWorktreeIds + } + + // Why: BrowserPane does not spawn PTYs. Keep a restored active browser + // visible while TerminalPane mounts still wait for reconnect to finish. + return [...mountedWorktreeIds, activeWorktreeId] +} diff --git a/src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts b/src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts new file mode 100644 index 000000000..3f3379c06 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from 'vitest' +import type { BrowserTab } from '../../../../shared/types' +import { getTerminalBrowserTabSlices } from './terminal-browser-tab-slices' + +const browserTab = (id: string, worktreeId = 'wt-active'): BrowserTab => ({ + id, + worktreeId, + url: `https://example.com/${id}`, + title: id, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 0 +}) + +describe('getTerminalBrowserTabSlices', () => { + it('preserves slices when an unmounted worktree browser tab array changes', () => { + const activeBrowserTabs = [browserTab('active')] + const mountedIds = new Set(['wt-active']) + const first = getTerminalBrowserTabSlices( + { + 'wt-active': activeBrowserTabs, + 'wt-hidden': [browserTab('hidden-a', 'wt-hidden')] + }, + mountedIds, + 'wt-active' + ) + const second = getTerminalBrowserTabSlices( + { + 'wt-active': activeBrowserTabs, + 'wt-hidden': [browserTab('hidden-b', 'wt-hidden')] + }, + mountedIds, + 'wt-active' + ) + + expect(second).toBe(first) + expect(second.activeBrowserTabs).toBe(activeBrowserTabs) + }) + + it('updates slices when a mounted worktree browser tab array changes', () => { + const mountedIds = new Set(['wt-active', 'wt-mounted']) + const first = getTerminalBrowserTabSlices( + { + 'wt-active': [browserTab('active')], + 'wt-mounted': [browserTab('mounted-a', 'wt-mounted')] + }, + mountedIds, + 'wt-active' + ) + const mountedBrowserTabs = [browserTab('mounted-b', 'wt-mounted')] + const second = getTerminalBrowserTabSlices( + { 'wt-active': first.activeBrowserTabs, 'wt-mounted': mountedBrowserTabs }, + mountedIds, + 'wt-active' + ) + + expect(second).not.toBe(first) + expect(second.mountedBrowserTabsByWorktree['wt-mounted']).toBe(mountedBrowserTabs) + }) + + it('keeps active browser tabs available even before the active worktree is mounted', () => { + const activeBrowserTabs = [browserTab('active')] + const slices = getTerminalBrowserTabSlices( + { 'wt-active': activeBrowserTabs }, + new Set(), + 'wt-active' + ) + + expect(slices.activeBrowserTabs).toBe(activeBrowserTabs) + expect(slices.mountedBrowserTabsByWorktree).toEqual({}) + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-browser-tab-slices.ts b/src/renderer/src/components/terminal/terminal-browser-tab-slices.ts new file mode 100644 index 000000000..df6f23c78 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-browser-tab-slices.ts @@ -0,0 +1,68 @@ +import type { BrowserTab } from '../../../../shared/types' + +export type TerminalBrowserTabSlices = { + activeBrowserTabs: BrowserTab[] + mountedBrowserTabsByWorktree: Record +} + +const EMPTY_BROWSER_TABS: BrowserTab[] = [] +let cachedBrowserTabsByWorktree: Record | null = null +let cachedMountedIdsKey = '' +let cachedActiveWorktreeId: string | null = null +let cachedSlices: TerminalBrowserTabSlices = { + activeBrowserTabs: EMPTY_BROWSER_TABS, + mountedBrowserTabsByWorktree: {} +} + +function mountedIdsKey(mountedWorktreeIds: ReadonlySet): string { + return [...mountedWorktreeIds].sort().join('\0') +} + +function sameMountedBrowserTabs( + left: Record, + right: Record +): boolean { + const leftKeys = Object.keys(left) + const rightKeys = Object.keys(right) + return leftKeys.length === rightKeys.length && leftKeys.every((key) => left[key] === right[key]) +} + +export function getTerminalBrowserTabSlices( + browserTabsByWorktree: Record, + mountedWorktreeIds: ReadonlySet, + activeWorktreeId: string | null +): TerminalBrowserTabSlices { + const nextMountedIdsKey = mountedIdsKey(mountedWorktreeIds) + if ( + browserTabsByWorktree === cachedBrowserTabsByWorktree && + nextMountedIdsKey === cachedMountedIdsKey && + activeWorktreeId === cachedActiveWorktreeId + ) { + return cachedSlices + } + + const activeBrowserTabs = activeWorktreeId + ? (browserTabsByWorktree[activeWorktreeId] ?? EMPTY_BROWSER_TABS) + : EMPTY_BROWSER_TABS + const mountedBrowserTabsByWorktree: Record = {} + for (const worktreeId of mountedWorktreeIds) { + mountedBrowserTabsByWorktree[worktreeId] = + browserTabsByWorktree[worktreeId] ?? EMPTY_BROWSER_TABS + } + + cachedBrowserTabsByWorktree = browserTabsByWorktree + cachedMountedIdsKey = nextMountedIdsKey + cachedActiveWorktreeId = activeWorktreeId + if ( + activeBrowserTabs === cachedSlices.activeBrowserTabs && + sameMountedBrowserTabs(mountedBrowserTabsByWorktree, cachedSlices.mountedBrowserTabsByWorktree) + ) { + return cachedSlices + } + + // Why: hidden BrowserPanes are retained only for mounted worktrees. Avoid + // rendering or resubscribing the terminal surface when browser tabs in + // unvisited worktrees restore or refresh in the background. + cachedSlices = { activeBrowserTabs, mountedBrowserTabsByWorktree } + return cachedSlices +} diff --git a/src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts b/src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts new file mode 100644 index 000000000..7b5ae4b34 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from 'vitest' +import type { Worktree } from '../../../../shared/types' +import { getTerminalMountedWorktreeSnapshot } from './terminal-mounted-worktrees' + +const worktree = (input: Partial & Pick): Worktree => ({ + id: input.id, + path: input.path, + repoId: input.repoId ?? 'repo-1', + displayName: input.displayName ?? input.id, + comment: input.comment ?? '', + branch: input.branch ?? 'main', + head: input.head ?? 'abc123', + isBare: input.isBare ?? false, + isMainWorktree: input.isMainWorktree ?? false, + linkedIssue: input.linkedIssue ?? null, + linkedPR: input.linkedPR ?? null, + linkedLinearIssue: input.linkedLinearIssue ?? null, + isArchived: input.isArchived ?? false, + isUnread: input.isUnread ?? false, + isPinned: input.isPinned ?? false, + sortOrder: input.sortOrder ?? 0, + lastActivityAt: input.lastActivityAt ?? 0 +}) + +describe('getTerminalMountedWorktreeSnapshot', () => { + it('preserves the snapshot when unrelated worktree metadata changes', () => { + const mountedIds = new Set(['wt-active']) + const first = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active', linkedIssue: 1 }), + worktree({ id: 'wt-other', path: '/repo/other', displayName: 'Other' }) + ] + }, + mountedIds + ) + const second = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active', linkedIssue: 2 }), + worktree({ id: 'wt-other', path: '/repo/other', displayName: 'Renamed' }) + ] + }, + mountedIds + ) + + expect(second).toBe(first) + expect(second.mountedWorktrees).toEqual([{ id: 'wt-active', path: '/repo/active' }]) + }) + + it('returns a new snapshot when a mounted worktree path changes', () => { + const mountedIds = new Set(['wt-active']) + const first = getTerminalMountedWorktreeSnapshot( + { 'repo-1': [worktree({ id: 'wt-active', path: '/repo/active' })] }, + mountedIds + ) + const second = getTerminalMountedWorktreeSnapshot( + { 'repo-1': [worktree({ id: 'wt-active', path: '/repo/moved' })] }, + mountedIds + ) + + expect(second).not.toBe(first) + expect(second.mountedWorktrees).toEqual([{ id: 'wt-active', path: '/repo/moved' }]) + }) + + it('preserves the snapshot when an unmounted worktree path changes', () => { + const mountedIds = new Set(['wt-active']) + const first = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active' }), + worktree({ id: 'wt-hidden', path: '/repo/hidden-a' }) + ] + }, + mountedIds + ) + const second = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active' }), + worktree({ id: 'wt-hidden', path: '/repo/hidden-b' }) + ] + }, + mountedIds + ) + + expect(second).toBe(first) + }) + + it('updates mounted worktrees when the mounted id set changes', () => { + const first = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active' }), + worktree({ id: 'wt-mounted', path: '/repo/mounted' }) + ] + }, + new Set(['wt-active']) + ) + const second = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active' }), + worktree({ id: 'wt-mounted', path: '/repo/mounted' }) + ] + }, + new Set(['wt-active', 'wt-mounted']) + ) + + expect(second).not.toBe(first) + expect(second.mountedWorktrees.map((item) => item.id)).toEqual(['wt-active', 'wt-mounted']) + }) + + it('dedupes duplicate worktree ids before mounting pane trees', () => { + const snapshot = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [worktree({ id: 'wt-active', path: '/repo/active-a' })], + 'repo-2': [worktree({ id: 'wt-active', path: '/repo/active-b', repoId: 'repo-2' })] + }, + new Set(['wt-active']) + ) + + expect(snapshot.worktreeIds).toEqual(['wt-active']) + expect(snapshot.mountedWorktrees).toEqual([{ id: 'wt-active', path: '/repo/active-a' }]) + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts b/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts new file mode 100644 index 000000000..1d8fcae36 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts @@ -0,0 +1,69 @@ +import type { Worktree } from '../../../../shared/types' + +export type TerminalMountedWorktreeSnapshot = { + mountedWorktrees: Pick[] + worktreeIds: string[] +} + +let cachedWorktreesByRepo: Record | null = null +let cachedMountedIdsKey = '' +let cachedSnapshot: TerminalMountedWorktreeSnapshot = { + mountedWorktrees: [], + worktreeIds: [] +} + +function mountedIdsKey(mountedWorktreeIds: ReadonlySet): string { + return [...mountedWorktreeIds].sort().join('\0') +} + +function sameWorktreeProjection( + left: Pick[], + right: Pick[] +): boolean { + return ( + left.length === right.length && + left.every((worktree, index) => { + const other = right[index] + return worktree.id === other.id && worktree.path === other.path + }) + ) +} + +export function getTerminalMountedWorktreeSnapshot( + worktreesByRepo: Record, + mountedWorktreeIds: ReadonlySet +): TerminalMountedWorktreeSnapshot { + const nextMountedIdsKey = mountedIdsKey(mountedWorktreeIds) + if (worktreesByRepo === cachedWorktreesByRepo && nextMountedIdsKey === cachedMountedIdsKey) { + return cachedSnapshot + } + + const worktreeById = new Map>() + for (const repoWorktrees of Object.values(worktreesByRepo)) { + for (const worktree of repoWorktrees) { + if (!worktreeById.has(worktree.id)) { + worktreeById.set(worktree.id, { id: worktree.id, path: worktree.path }) + } + } + } + const worktreeIds = [...worktreeById.keys()] + const mountedWorktrees = [...worktreeById.values()].filter((worktree) => + mountedWorktreeIds.has(worktree.id) + ) + + cachedWorktreesByRepo = worktreesByRepo + cachedMountedIdsKey = nextMountedIdsKey + if ( + worktreeIds.length === cachedSnapshot.worktreeIds.length && + worktreeIds.every((id, index) => id === cachedSnapshot.worktreeIds[index]) && + sameWorktreeProjection(mountedWorktrees, cachedSnapshot.mountedWorktrees) + ) { + return cachedSnapshot + } + + // Why: Terminal only needs all IDs for pruning plus id/path for mounted pane + // trees. Preserve the snapshot when unrelated or unmounted worktree metadata + // changes so sidebar/status refreshes don't rerender xterm during typing. + cachedSnapshot = { mountedWorktrees, worktreeIds } + return cachedSnapshot +} diff --git a/src/renderer/src/components/terminal/terminal-tab-slices.test.ts b/src/renderer/src/components/terminal/terminal-tab-slices.test.ts new file mode 100644 index 000000000..d6372c34b --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-tab-slices.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import type { TerminalTab } from '../../../../shared/types' +import { getTerminalTabSlices } from './terminal-tab-slices' + +const tab = (id: string, worktreeId = 'wt-active'): TerminalTab => ({ + id, + title: id, + ptyId: null, + worktreeId, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0, + generation: 0 +}) + +describe('getTerminalTabSlices', () => { + it('preserves slices when an unmounted worktree tab array changes', () => { + const activeTabs = [tab('active')] + const mountedIds = new Set(['wt-active']) + const first = getTerminalTabSlices( + { 'wt-active': activeTabs, 'wt-hidden': [tab('hidden-a', 'wt-hidden')] }, + mountedIds, + 'wt-active' + ) + const second = getTerminalTabSlices( + { 'wt-active': activeTabs, 'wt-hidden': [tab('hidden-b', 'wt-hidden')] }, + mountedIds, + 'wt-active' + ) + + expect(second).toBe(first) + expect(second.activeTabs).toBe(activeTabs) + }) + + it('updates slices when a mounted worktree tab array changes', () => { + const mountedIds = new Set(['wt-active', 'wt-mounted']) + const first = getTerminalTabSlices( + { 'wt-active': [tab('active')], 'wt-mounted': [tab('mounted-a', 'wt-mounted')] }, + mountedIds, + 'wt-active' + ) + const mountedTabs = [tab('mounted-b', 'wt-mounted')] + const second = getTerminalTabSlices( + { 'wt-active': first.activeTabs, 'wt-mounted': mountedTabs }, + mountedIds, + 'wt-active' + ) + + expect(second).not.toBe(first) + expect(second.mountedTabsByWorktree['wt-mounted']).toBe(mountedTabs) + }) + + it('keeps active tabs available even before the active worktree is mounted', () => { + const activeTabs = [tab('active')] + const slices = getTerminalTabSlices({ 'wt-active': activeTabs }, new Set(), 'wt-active') + + expect(slices.activeTabs).toBe(activeTabs) + expect(slices.mountedTabsByWorktree).toEqual({}) + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-tab-slices.ts b/src/renderer/src/components/terminal/terminal-tab-slices.ts new file mode 100644 index 000000000..a64b23499 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-tab-slices.ts @@ -0,0 +1,67 @@ +import type { TerminalTab } from '../../../../shared/types' + +export type TerminalTabSlices = { + activeTabs: TerminalTab[] + mountedTabsByWorktree: Record +} + +const EMPTY_TERMINAL_TABS: TerminalTab[] = [] +let cachedTabsByWorktree: Record | null = null +let cachedMountedIdsKey = '' +let cachedActiveWorktreeId: string | null = null +let cachedSlices: TerminalTabSlices = { + activeTabs: EMPTY_TERMINAL_TABS, + mountedTabsByWorktree: {} +} + +function mountedIdsKey(mountedWorktreeIds: ReadonlySet): string { + return [...mountedWorktreeIds].sort().join('\0') +} + +function sameMountedTabs( + left: Record, + right: Record +): boolean { + const leftKeys = Object.keys(left) + const rightKeys = Object.keys(right) + return leftKeys.length === rightKeys.length && leftKeys.every((key) => left[key] === right[key]) +} + +export function getTerminalTabSlices( + tabsByWorktree: Record, + mountedWorktreeIds: ReadonlySet, + activeWorktreeId: string | null +): TerminalTabSlices { + const nextMountedIdsKey = mountedIdsKey(mountedWorktreeIds) + if ( + tabsByWorktree === cachedTabsByWorktree && + nextMountedIdsKey === cachedMountedIdsKey && + activeWorktreeId === cachedActiveWorktreeId + ) { + return cachedSlices + } + + const activeTabs = activeWorktreeId + ? (tabsByWorktree[activeWorktreeId] ?? EMPTY_TERMINAL_TABS) + : EMPTY_TERMINAL_TABS + const mountedTabsByWorktree: Record = {} + for (const worktreeId of mountedWorktreeIds) { + mountedTabsByWorktree[worktreeId] = tabsByWorktree[worktreeId] ?? EMPTY_TERMINAL_TABS + } + + cachedTabsByWorktree = tabsByWorktree + cachedMountedIdsKey = nextMountedIdsKey + cachedActiveWorktreeId = activeWorktreeId + if ( + activeTabs === cachedSlices.activeTabs && + sameMountedTabs(mountedTabsByWorktree, cachedSlices.mountedTabsByWorktree) + ) { + return cachedSlices + } + + // Why: Terminal renders only the active titlebar and mounted pane trees. + // Ignore tab-array churn for unmounted worktrees so background metadata + // updates do not rerender xterm while the user is typing. + cachedSlices = { activeTabs, mountedTabsByWorktree } + return cachedSlices +} diff --git a/src/renderer/src/store/slices/repos-update-serialization.test.ts b/src/renderer/src/store/slices/repos-update-serialization.test.ts index 811d4c81f..961eaa518 100644 --- a/src/renderer/src/store/slices/repos-update-serialization.test.ts +++ b/src/renderer/src/store/slices/repos-update-serialization.test.ts @@ -136,8 +136,8 @@ describe('repo update serialization', () => { await store.getState().updateRepo(localRepo.id, { repoIcon: { type: 'image', - source: 'upload', - src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' + source: 'github', + src: 'https://example.com/icon.png' } as never }) diff --git a/src/shared/repo-icon.test.ts b/src/shared/repo-icon.test.ts index 8f312104f..39f499d9e 100644 --- a/src/shared/repo-icon.test.ts +++ b/src/shared/repo-icon.test.ts @@ -46,6 +46,17 @@ describe('sanitizeRepoIcon', () => { src: 'data:image/png;base64,aGVsbG8=', source: 'upload' }) + expect( + sanitizeRepoIcon({ + type: 'image', + src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=', + source: 'upload' + }) + ).toEqual({ + type: 'image', + src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=', + source: 'upload' + }) }) it('keeps null as an explicit reset', () => { @@ -70,15 +81,22 @@ describe('sanitizeRepoIcon', () => { expect( sanitizeRepoIcon({ type: 'image', - src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=', - source: 'upload' + src: 'https://example.com/icon.png', + source: 'github' }) ).toBeUndefined() expect( sanitizeRepoIcon({ type: 'image', src: 'https://example.com/icon.png', - source: 'github' + source: 'favicon' + }) + ).toBeUndefined() + expect( + sanitizeRepoIcon({ + type: 'image', + src: 'https://example.com/icon.png', + source: 'upload' }) ).toBeUndefined() }) diff --git a/src/shared/repo-icon.ts b/src/shared/repo-icon.ts index 84d865dc8..1add7c82f 100644 --- a/src/shared/repo-icon.ts +++ b/src/shared/repo-icon.ts @@ -14,7 +14,7 @@ const isRepoIconImageSource = (value: string): value is RepoIconImageSource => function isSupportedImageSrc(src: string, source: RepoIconImageSource): boolean { if (source === 'upload') { - return /^data:image\/png;base64,[A-Za-z0-9+/=\s]+$/i.test(src) + return /^data:image\/(?:png|svg\+xml);base64,[A-Za-z0-9+/=\s]+$/i.test(src) } let url: URL