From 51618f1e924f5a298eb70fda26c07464be854fa6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 23 May 2026 14:10:15 -0700 Subject: [PATCH] Finish terminal perf rollback for OpenCode TUIs (#2716) * revert: terminal mounted worktree slicing * fix: restore non-terminal rollback scope --- src/main/ipc/shell.ts | 7 +- src/main/persistence.test.ts | 33 +++ src/main/persistence.ts | 36 ++- src/renderer/src/App.tsx | 4 +- src/renderer/src/components/Terminal.tsx | 268 +++++++++--------- .../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 | 16 ++ .../settings/TerminalPane.pwsh.test.ts | 43 ++- .../remote-runtime-pty-transport.test.ts | 28 +- .../active-worktree-open-files.test.ts | 41 --- .../terminal/active-worktree-open-files.ts | 35 --- .../terminal-browser-pane-worktrees.test.ts | 52 ---- .../terminal-browser-pane-worktrees.ts | 31 -- .../terminal-browser-tab-slices.test.ts | 75 ----- .../terminal/terminal-browser-tab-slices.ts | 68 ----- .../terminal-mounted-worktrees.test.ts | 113 -------- .../terminal/terminal-mounted-worktrees.ts | 67 ----- .../terminal/terminal-tab-slices.test.ts | 61 ---- .../terminal/terminal-tab-slices.ts | 67 ----- .../slices/repos-update-serialization.test.ts | 17 ++ src/renderer/src/store/slices/repos.ts | 26 +- src/shared/repo-icon.test.ts | 36 +++ src/shared/repo-icon.ts | 32 ++- 26 files changed, 386 insertions(+), 868 deletions(-) delete mode 100644 src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts delete mode 100644 src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts delete mode 100644 src/renderer/src/components/terminal/active-worktree-open-files.test.ts delete mode 100644 src/renderer/src/components/terminal/active-worktree-open-files.ts delete mode 100644 src/renderer/src/components/terminal/terminal-browser-pane-worktrees.test.ts delete mode 100644 src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts delete mode 100644 src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts delete mode 100644 src/renderer/src/components/terminal/terminal-browser-tab-slices.ts delete mode 100644 src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts delete mode 100644 src/renderer/src/components/terminal/terminal-mounted-worktrees.ts delete mode 100644 src/renderer/src/components/terminal/terminal-tab-slices.test.ts delete 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 4712828a5..37048c650 100644 --- a/src/main/ipc/shell.ts +++ b/src/main/ipc/shell.ts @@ -11,8 +11,7 @@ import { getSpawnArgsForWindows } from '../win32-utils' export const EXTERNAL_EDITOR_CLI_COMMAND = 'code' const REPO_ICON_IMAGE_MIME_TYPES: Record = { - '.png': 'image/png', - '.svg': 'image/svg+xml' + '.png': 'image/png' } async function pathExists(pathValue: string): Promise { @@ -248,7 +247,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', 'svg'] }] + filters: [{ name: 'Repo icon images', extensions: ['png'] }] }) if (result.canceled || result.filePaths.length === 0) { return null @@ -258,7 +257,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 or SVG files.') + throw new Error('Repo icons must be PNG files.') } const stats = await stat(filePath) diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index f5239344f..f3a1c2fd7 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -1376,6 +1376,39 @@ describe('Store', () => { expect(store.getRepo('r1')!.displayName).toBe('renamed') }) + it('updateRepo drops repo icons that fail shared sanitization', async () => { + const store = await createStore() + store.addRepo(makeRepo()) + + const updated = store.updateRepo('r1', { + repoIcon: { + type: 'image', + source: 'upload', + src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' + } as never + }) + + expect(updated).not.toBeNull() + expect(updated!.repoIcon).toBeUndefined() + expect(store.getRepo('r1')!.repoIcon).toBeUndefined() + }) + + it('getRepo does not expose invalid persisted repo icons', async () => { + const store = await createStore() + store.addRepo( + makeRepo({ + repoIcon: { + type: 'image', + source: 'upload', + src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' + } as never + }) + ) + + expect(store.getRepo('r1')!.repoIcon).toBeUndefined() + expect(store.getRepos()[0]!.repoIcon).toBeUndefined() + }) + it('updateRepo returns null for nonexistent id', async () => { const store = await createStore() expect(store.updateRepo('nope', { displayName: 'x' })).toBeNull() diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 65497208a..5e1be8718 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -93,6 +93,7 @@ import { normalizeWorkspaceStatuses } from '../shared/workspace-statuses' import { isLegacyRepoForExternalWorktreeVisibility } from '../shared/worktree-ownership' +import { sanitizeRepoIcon } from '../shared/repo-icon' function encrypt(plaintext: string): string { if (!plaintext || !safeStorage.isEncryptionAvailable()) { @@ -415,6 +416,21 @@ function readLegacySidekickFlag(parsed: PersistedState | undefined): boolean | u return (parsed?.settings as { experimentalSidekick?: boolean } | undefined)?.experimentalSidekick } +function sanitizeRepoUpdatesForPersistence>>( + updates: T +): T { + const sanitized = { ...updates } + if ('repoIcon' in sanitized) { + const repoIcon = sanitizeRepoIcon(sanitized.repoIcon) + if (repoIcon === undefined) { + delete sanitized.repoIcon + } else { + sanitized.repoIcon = repoIcon + } + } + return sanitized +} + function expandFloatingWorkspaceHomePath(input: string, home: string): string { if (input === '~') { return home @@ -2057,8 +2073,10 @@ export class Store { if (!repo) { return null } + const sanitizedUpdates = sanitizeRepoUpdatesForPersistence(updates) const externalWorktreeVisibilityLegacy = - 'externalWorktreeVisibility' in updates && repo.externalWorktreeVisibilityLegacy === undefined + 'externalWorktreeVisibility' in sanitizedUpdates && + repo.externalWorktreeVisibilityLegacy === undefined ? isLegacyRepoForExternalWorktreeVisibility(repo) : undefined // Why: `issueSourcePreference === undefined` in the patch means "reset to @@ -2066,15 +2084,18 @@ export class Store { // stale explicit value via Object.assign's skip-on-undefined behavior). // Without this delete branch, toggling explicit → auto would silently // leave the old preference in place on disk. - if ('issueSourcePreference' in updates && updates.issueSourcePreference === undefined) { + if ( + 'issueSourcePreference' in sanitizedUpdates && + sanitizedUpdates.issueSourcePreference === undefined + ) { delete repo.issueSourcePreference - const { issueSourcePreference: _drop, ...rest } = updates + const { issueSourcePreference: _drop, ...rest } = sanitizedUpdates Object.assign(repo, rest) } else { - Object.assign(repo, updates) + Object.assign(repo, sanitizedUpdates) } if ( - 'externalWorktreeVisibility' in updates && + 'externalWorktreeVisibility' in sanitizedUpdates && repo.externalWorktreeVisibilityLegacy === undefined ) { // Why: old persisted repos have no explicit marker. Stamp it the first @@ -2086,6 +2107,8 @@ export class Store { } private hydrateRepo(repo: Repo): Repo { + const { repoIcon: rawRepoIcon, ...repoWithoutIcon } = repo + const repoIcon = sanitizeRepoIcon(rawRepoIcon) const gitUsername = isFolderRepo(repo) ? '' : (this.gitUsernameCache.get(repo.path) ?? @@ -2096,7 +2119,8 @@ export class Store { })()) return { - ...repo, + ...repoWithoutIcon, + ...(repoIcon !== undefined ? { repoIcon } : {}), kind: isFolderRepo(repo) ? 'folder' : 'git', gitUsername, hookSettings: { diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 228e28486..19bdbd54e 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -108,7 +108,6 @@ 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' @@ -308,7 +307,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 tabs = useActiveTerminalTabs() + const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) const floatingUnifiedTabCount = useAppStore( (s) => s.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID]?.length ?? 0 ) @@ -964,6 +963,7 @@ 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 61578a1a9..88acbca21 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, useRef, useState, lazy, Suspense } from 'react' +import React, { useEffect, useCallback, useMemo, useRef, useState, lazy, Suspense } from 'react' import { createPortal } from 'react-dom' import { toast } from 'sonner' import { @@ -9,6 +9,7 @@ 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' @@ -49,13 +50,8 @@ 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 } 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 { @@ -105,8 +101,10 @@ 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) @@ -120,37 +118,7 @@ function Terminal(): React.JSX.Element | null { const consumeSuppressedPtyExit = useAppStore((s) => s.consumeSuppressedPtyExit) const expandedPaneByTabId = useAppStore((s) => s.expandedPaneByTabId) const workspaceSessionReady = useAppStore((s) => s.workspaceSessionReady) - // 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 openFiles = useAppStore((s) => s.openFiles) const activeFileId = useAppStore((s) => s.activeFileId) const activeBrowserTabId = useAppStore((s) => s.activeBrowserTabId) const activeTabType = useAppStore((s) => s.activeTabType) @@ -163,6 +131,7 @@ 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) @@ -186,7 +155,10 @@ function Terminal(): React.JSX.Element | null { activeView === 'activity' ) - const tabs = terminalTabSlices.activeTabs + const tabs = useMemo( + () => (activeWorktreeId ? (tabsByWorktree[activeWorktreeId] ?? []) : []), + [activeWorktreeId, tabsByWorktree] + ) // 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. @@ -206,7 +178,13 @@ function Terminal(): React.JSX.Element | null { ensureWorktreeRootGroup(activeWorktreeId) }, [activeWorktreeId, ensureWorktreeRootGroup]) - const worktreeBrowserTabs = terminalBrowserTabSlices.activeBrowserTabs + // 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 getEffectiveLayoutForWorktree = useCallback( (worktreeId: string) => getEffectiveLayout(worktreeId, layoutByWorktree, groupsByWorktree, activeGroupIdByWorktree), @@ -215,20 +193,13 @@ function Terminal(): React.JSX.Element | null { const effectiveActiveLayout = activeWorktreeId ? getEffectiveLayoutForWorktree(activeWorktreeId) : undefined - 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 - }) + const activeWorktreeBrowserTabIdsKey = activeWorktreeId + ? (browserTabsByWorktree[activeWorktreeId] ?? []).map((tab) => tab.id).join(',') + : '' // Save confirmation dialog state const [saveDialogFileId, setSaveDialogFileId] = useState(null) - const saveDialogFile = useAppStore((s) => - saveDialogFileId ? (s.openFiles.find((file) => file.id === saveDialogFileId) ?? null) : null - ) + const saveDialogFile = saveDialogFileId ? openFiles.find((f) => f.id === saveDialogFileId) : null const pendingEditorCloseQueueRef = useRef([]) // Why: while a save-and-close is awaiting the file to disappear from @@ -561,6 +532,13 @@ 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 => { @@ -603,15 +581,23 @@ 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(terminalWorktreeSnapshot.worktreeIds) + const allWorktreeIds = new Set(allWorktrees.map((wt) => wt.id)) for (const id of mountedWorktreeIdsRef.current) { if (!allWorktreeIds.has(id)) { mountedWorktreeIdsRef.current.delete(id) } } const anyMountedWorktreeHasLayout = computeAnyMountedWorktreeHasLayout( - terminalWorktreeSnapshot.worktreeIds, + allWorktrees.map((wt) => wt.id), mountedWorktreeIdsRef.current, layoutByWorktree, groupsByWorktree, @@ -1491,29 +1477,31 @@ 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. */} - {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 ( - - ) - })} + {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 ( + + ) + })} ) : null} @@ -1551,65 +1539,67 @@ function Terminal(): React.JSX.Element | null { : '' }`} > - {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 - const shouldMeasureHiddenWorktree = - !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) - return ( -
- - {(terminalTabSlices.mountedTabsByWorktree[worktree.id] ?? []).map((tab) => { - const activityTerminalPortal = findActivityTerminalPortal( - activityTerminalPortals, - { worktreeId: worktree.id, tabId: tab.id } - ) - const isActivityPortalTab = activityTerminalPortal !== null - const isActiveTerminalTab = - isVisible && tab.id === activeTabId && activeTabType === 'terminal' - const terminalPane = ( - no override. - isolatedPaneKey={activityTerminalPortal?.paneKey ?? null} - onPtyExit={(ptyId) => handlePtyExit(tab.id, ptyId)} - onCloseTab={() => handleCloseTab(tab.id)} - /> - ) - if (activityTerminalPortal) { - return createPortal( - terminalPane, - activityTerminalPortal.target, - `activity-terminal-${tab.id}` - ) + {allWorktrees + .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id)) + .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 + const shouldMeasureHiddenWorktree = + !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) + return ( +
- ) - })} + aria-hidden={!isVisible} + > + + {(tabsByWorktree[worktree.id] ?? []).map((tab) => { + const activityTerminalPortal = findActivityTerminalPortal( + activityTerminalPortals, + { worktreeId: worktree.id, tabId: tab.id } + ) + const isActivityPortalTab = activityTerminalPortal !== null + const isActiveTerminalTab = + isVisible && tab.id === activeTabId && activeTabType === 'terminal' + const terminalPane = ( + handlePtyExit(tab.id, ptyId)} + onCloseTab={() => handleCloseTab(tab.id)} + /> + ) + if (activityTerminalPortal) { + return createPortal( + terminalPane, + activityTerminalPortal.target, + `activity-terminal-${tab.id}` + ) + } + return terminalPane + })} +
+ ) + })}
{/* Browser panes container — all browser panes for the active worktree @@ -1620,20 +1610,18 @@ function Terminal(): React.JSX.Element | null { activeTabType !== 'browser' ? 'hidden' : '' }`} > - {browserPaneWorktreeIds.map((worktreeId) => { - const browserTabs = - worktreeId === activeWorktreeId - ? worktreeBrowserTabs - : (terminalBrowserTabSlices.mountedBrowserTabsByWorktree[worktreeId] ?? []) + {allWorktrees.map((worktree) => { + const browserTabs = browserTabsByWorktree[worktree.id] ?? [] // Why: use strict equality with 'terminal' instead of !== 'settings' // so browser panes also hide on the tasks page. - const isVisibleWorktree = activeView === 'terminal' && worktreeId === activeWorktreeId + const isVisibleWorktree = + activeView === 'terminal' && worktree.id === 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 ce19eefb1..760e933ee 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -64,7 +64,6 @@ 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[] = [] @@ -90,19 +89,11 @@ export function FloatingTerminalPanel({ open, onOpenChange }: FloatingTerminalPanelProps): React.JSX.Element | null { - 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 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 expandedPaneByTabId = useAppStore((s) => s.expandedPaneByTabId) const createTab = useAppStore((s) => s.createTab) const createBrowserTab = useAppStore((s) => s.createBrowserTab) @@ -141,6 +132,14 @@ 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) ?? @@ -243,7 +242,7 @@ export function FloatingTerminalPanel({ handleSaveDialogSave, handleSaveDialogDiscard, handleSaveDialogCancel - } = useTerminalSaveDialog({ openFiles: floatingFiles, closeFile, markFileDirty }) + } = useTerminalSaveDialog({ openFiles, 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 deleted file mode 100644 index 9f1eecf89..000000000 --- a/src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts +++ /dev/null @@ -1,38 +0,0 @@ -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 deleted file mode 100644 index 155cee881..000000000 --- a/src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts +++ /dev/null @@ -1,27 +0,0 @@ -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 5931d3afe..a0f90a20a 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/SVG + Upload PNG
-

PNG/SVG uploads must be 256KB or smaller.

+

PNG 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 16ef495cf..ba86793f6 100644 --- a/src/renderer/src/components/settings/TerminalPane.ghostty.test.ts +++ b/src/renderer/src/components/settings/TerminalPane.ghostty.test.ts @@ -80,11 +80,27 @@ 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 + }, + SettingsSubsectionHeader: function SettingsSubsectionHeader() { + return null + }, + SettingsSwitchRow: function SettingsSwitchRow() { + 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 3396421a5..804d2038b 100644 --- a/src/renderer/src/components/settings/TerminalPane.pwsh.test.ts +++ b/src/renderer/src/components/settings/TerminalPane.pwsh.test.ts @@ -80,11 +80,35 @@ 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 + }, + SettingsSubsectionHeader: function SettingsSubsectionHeader() { + return null + }, + SettingsSwitchRow: function SettingsSwitchRow() { + return null } })) @@ -140,6 +164,15 @@ 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)) + } + return nodes +} + function collectText(node: unknown): string { if (node == null) { return '' @@ -154,7 +187,7 @@ function collectText(node: unknown): string { return node.map(collectText).join('') } const el = node as ReactElementLike - return collectText(el.props?.children) + return getPropNodes(el).map(collectText).join('') } function findAnchorByText(node: unknown, text: string): ReactElementLike | null { @@ -178,7 +211,13 @@ function findAnchorByText(node: unknown, text: string): ReactElementLike | null if (typeName === 'a' && collectText(el.props.children).includes(text)) { return el } - return findAnchorByText(el.props?.children, text) + for (const child of getPropNodes(el)) { + const found = findAnchorByText(child, text) + if (found) { + return found + } + } + return null } describe('TerminalPane PowerShell version setting', () => { 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 1eabea0ea..cc10dd150 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,11 +515,13 @@ describe('createRemoteRuntimePtyTransport', () => { 'before\x1b]9999;{"state":"working","prompt":"ship it","agentType":"codex"}\x07after\x1b]0;. Claude working\x07\x07' ) - expect(onAgentStatus).toHaveBeenCalledWith({ - state: 'working', - prompt: 'ship it', - agentType: 'codex' - }) + 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) @@ -546,11 +548,13 @@ describe('createRemoteRuntimePtyTransport', () => { 'before\x1b]9999;{"state":"working","prompt":"ship it","agentType":"codex"}\x07after' ) - expect(onAgentStatus).toHaveBeenCalledWith({ - state: 'working', - prompt: 'ship it', - agentType: 'codex' - }) + await vi.waitFor(() => + expect(onAgentStatus).toHaveBeenCalledWith({ + state: 'working', + prompt: 'ship it', + agentType: 'codex' + }) + ) expect(onData).toHaveBeenCalledWith('beforeafter') }) @@ -852,7 +856,9 @@ describe('createRemoteRuntimePtyTransport', () => { ) expect(onReplayData).toHaveBeenCalledWith('beforeafter\x1b]0;Remote title\x07\x07') - expect(onTitleChange).toHaveBeenCalledWith('Remote title', 'Remote title') + 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 deleted file mode 100644 index 91600ef39..000000000 --- a/src/renderer/src/components/terminal/active-worktree-open-files.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -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 deleted file mode 100644 index 48f5342b5..000000000 --- a/src/renderer/src/components/terminal/active-worktree-open-files.ts +++ /dev/null @@ -1,35 +0,0 @@ -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 deleted file mode 100644 index 38729e667..000000000 --- a/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { getTerminalBrowserPaneWorktreeIds } 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([]) - }) -}) diff --git a/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts b/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts deleted file mode 100644 index 1c6789a4d..000000000 --- a/src/renderer/src/components/terminal/terminal-browser-pane-worktrees.ts +++ /dev/null @@ -1,31 +0,0 @@ -import type { WorkspaceVisibleTabType } from '../../../../shared/types' - -export type TerminalBrowserPaneWorktreeInput = { - mountedWorktreeIds: string[] - worktreeIds: string[] - activeWorktreeId: string | null - activeTabType: WorkspaceVisibleTabType - activeBrowserTabCount: number -} - -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 deleted file mode 100644 index 3f3379c06..000000000 --- a/src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -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 deleted file mode 100644 index df6f23c78..000000000 --- a/src/renderer/src/components/terminal/terminal-browser-tab-slices.ts +++ /dev/null @@ -1,68 +0,0 @@ -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 deleted file mode 100644 index 0413c1ad2..000000000 --- a/src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts +++ /dev/null @@ -1,113 +0,0 @@ -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']) - }) -}) diff --git a/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts b/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts deleted file mode 100644 index b93d7e1c8..000000000 --- a/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts +++ /dev/null @@ -1,67 +0,0 @@ -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 mountedWorktrees: Pick[] = [] - const worktreeIds: string[] = [] - for (const repoWorktrees of Object.values(worktreesByRepo)) { - for (const worktree of repoWorktrees) { - worktreeIds.push(worktree.id) - if (mountedWorktreeIds.has(worktree.id)) { - mountedWorktrees.push({ id: worktree.id, path: worktree.path }) - } - } - } - - 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 deleted file mode 100644 index d6372c34b..000000000 --- a/src/renderer/src/components/terminal/terminal-tab-slices.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -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 deleted file mode 100644 index a64b23499..000000000 --- a/src/renderer/src/components/terminal/terminal-tab-slices.ts +++ /dev/null @@ -1,67 +0,0 @@ -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 de5ecd3ff..811d4c81f 100644 --- a/src/renderer/src/store/slices/repos-update-serialization.test.ts +++ b/src/renderer/src/store/slices/repos-update-serialization.test.ts @@ -127,4 +127,21 @@ describe('repo update serialization', () => { errorSpy.mockRestore() } }) + + it('does not apply repo icons that fail shared sanitization', async () => { + reposUpdate.mockResolvedValueOnce(undefined) + const store = createTestStore() + store.setState({ repos: [localRepo] }) + + await store.getState().updateRepo(localRepo.id, { + repoIcon: { + type: 'image', + source: 'upload', + src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' + } as never + }) + + expect(reposUpdate).toHaveBeenCalledWith({ repoId: localRepo.id, updates: {} }) + expect(store.getState().repos[0]?.repoIcon).toBeUndefined() + }) }) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index bb5c1d295..288ec3ca4 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -7,6 +7,7 @@ import { toast } from 'sonner' import type { AppState } from '../types' import type { Repo } from '../../../../shared/types' import { isGitRepoKind } from '../../../../shared/repo-kind' +import { sanitizeRepoIcon } from '../../../../shared/repo-icon' import { getRepoIdFromWorktreeId } from './worktree-helpers' import { callRuntimeRpc, getActiveRuntimeTarget } from '../../runtime/runtime-rpc-client' import { buildDismissedOnboardingFolderAgentStartup } from '@/lib/onboarding-folder-agent-startup' @@ -29,6 +30,19 @@ type RepoUpdate = Partial< > > +function sanitizeRepoUpdate(updates: RepoUpdate): RepoUpdate { + const sanitized = { ...updates } + if ('repoIcon' in sanitized) { + const repoIcon = sanitizeRepoIcon(sanitized.repoIcon) + if (repoIcon === undefined) { + delete sanitized.repoIcon + } else { + sanitized.repoIcon = repoIcon + } + } + return sanitized +} + const updateRepoChainsByStore = new WeakMap<() => AppState, Map>>() function getRepoUpdateChains(get: () => AppState): Map> { @@ -345,12 +359,18 @@ export const createRepoSlice: StateCreator = (set, const updateRepoChains = getRepoUpdateChains(get) const applyRepoUpdate = async () => { try { + const sanitizedUpdates = sanitizeRepoUpdate(updates) const target = getActiveRuntimeTarget(get().settings) await (target.kind === 'local' - ? window.api.repos.update({ repoId, updates }) - : callRuntimeRpc(target, 'repo.update', { repo: repoId, updates }, { timeoutMs: 15_000 })) + ? window.api.repos.update({ repoId, updates: sanitizedUpdates }) + : callRuntimeRpc( + target, + 'repo.update', + { repo: repoId, updates: sanitizedUpdates }, + { timeoutMs: 15_000 } + )) set((s) => ({ - repos: s.repos.map((r) => (r.id === repoId ? { ...r, ...updates } : r)) + repos: s.repos.map((r) => (r.id === repoId ? { ...r, ...sanitizedUpdates } : r)) })) return true } catch (err) { diff --git a/src/shared/repo-icon.test.ts b/src/shared/repo-icon.test.ts index 072da3862..8f312104f 100644 --- a/src/shared/repo-icon.test.ts +++ b/src/shared/repo-icon.test.ts @@ -24,6 +24,28 @@ describe('sanitizeRepoIcon', () => { source: 'github', label: 'stablyai/orca' }) + expect( + sanitizeRepoIcon({ + type: 'image', + src: 'https://www.google.com/s2/favicons?domain=example.com&sz=64', + source: 'favicon' + }) + ).toEqual({ + type: 'image', + src: 'https://www.google.com/s2/favicons?domain=example.com&sz=64', + source: 'favicon' + }) + expect( + sanitizeRepoIcon({ + type: 'image', + src: 'data:image/png;base64,aGVsbG8=', + source: 'upload' + }) + ).toEqual({ + type: 'image', + src: 'data:image/png;base64,aGVsbG8=', + source: 'upload' + }) }) it('keeps null as an explicit reset', () => { @@ -45,5 +67,19 @@ describe('sanitizeRepoIcon', () => { source: 'upload' }) ).toBeUndefined() + expect( + sanitizeRepoIcon({ + type: 'image', + src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=', + source: 'upload' + }) + ).toBeUndefined() + expect( + sanitizeRepoIcon({ + type: 'image', + src: 'https://example.com/icon.png', + source: 'github' + }) + ).toBeUndefined() }) }) diff --git a/src/shared/repo-icon.ts b/src/shared/repo-icon.ts index b48300a13..84d865dc8 100644 --- a/src/shared/repo-icon.ts +++ b/src/shared/repo-icon.ts @@ -9,13 +9,29 @@ export const MAX_REPO_ICON_UPLOAD_BYTES = 256 * 1024 export const MAX_REPO_ICON_DATA_URL_LENGTH = 400 * 1024 const LUCIDE_ICON_NAME_PATTERN = /^[A-Za-z][A-Za-z0-9]*$/ -const IMAGE_SOURCE_IDS = new Set(['upload', 'favicon', 'github']) +const isRepoIconImageSource = (value: string): value is RepoIconImageSource => + value === 'upload' || value === 'favicon' || value === 'github' -function isSupportedImageSrc(src: string): boolean { - return ( - /^https:\/\/[^\s]+$/i.test(src) || - /^data:image\/(?:png|svg\+xml);base64,[A-Za-z0-9+/=\s]+$/i.test(src) - ) +function isSupportedImageSrc(src: string, source: RepoIconImageSource): boolean { + if (source === 'upload') { + return /^data:image\/png;base64,[A-Za-z0-9+/=\s]+$/i.test(src) + } + + let url: URL + try { + url = new URL(src) + } catch { + return false + } + if (url.protocol !== 'https:') { + return false + } + + if (source === 'github') { + return url.hostname === 'github.com' && /^\/[^/?#]+\.png$/i.test(url.pathname) + } + + return url.hostname === 'www.google.com' && url.pathname === '/s2/favicons' } export function sanitizeRepoIcon(value: unknown): RepoIcon | null | undefined { @@ -49,10 +65,10 @@ export function sanitizeRepoIcon(value: unknown): RepoIcon | null | undefined { if (candidate.type === 'image') { const src = typeof candidate.src === 'string' ? candidate.src.trim() : '' const source = typeof candidate.source === 'string' ? candidate.source : '' - if (!IMAGE_SOURCE_IDS.has(source) || src.length > MAX_REPO_ICON_DATA_URL_LENGTH) { + if (!isRepoIconImageSource(source) || src.length > MAX_REPO_ICON_DATA_URL_LENGTH) { return undefined } - if (!isSupportedImageSrc(src)) { + if (!isSupportedImageSrc(src, source)) { return undefined } const label = typeof candidate.label === 'string' ? candidate.label.trim().slice(0, 80) : ''