diff --git a/src/main/index.ts b/src/main/index.ts index 2ea4ad9f..554383a0 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -32,11 +32,12 @@ import { createWebClientServer } from './web-client-server' import { getOrCreateWsToken, rotateWsToken } from './ws-token' import { networkInterfaces } from 'os' import type { Server as HttpServer } from 'http' -import type { ServerTransport } from '../shared/transport/transport' +import type { ServerTransport, ConnectionContext } from '../shared/transport/transport' import { detectRuntime } from './paths' import { fixPathFromLoginShell } from './path-fix' import { ChatStartGate } from './chat-start-gate' -import { createSingleFlight } from './single-flight' +import { createSingleFlight } from '../shared/single-flight' +import { isWithinRoots, ownedPathsOnly } from './own-paths' import { knownClaudeConfigDirs, onShellEnvCaptured, @@ -1974,6 +1975,32 @@ function registerIpcHandlers(): void { } ) + // Worktree reads only run for paths this backend hosts. A misrouted + // request carries another machine's path, and git spawned there blocks + // main — about a second per call under macOS's /home automount. See + // own-paths.ts. + const refusedForeignPaths = new Set() + const isOwnPath = (path: string): boolean => { + const { worktrees, terminals } = store.getSnapshot().state + const owned = isWithinRoots(path, worktrees.list.map((w) => w.path)) || + isWithinRoots(path, worktrees.repoRoots) || + isWithinRoots(path, Object.keys(terminals.panes)) + if (!owned && !refusedForeignPaths.has(path)) { + refusedForeignPaths.add(path) + log('ipc', `refusing worktree read for a path this backend does not host: ${path}`) + } + return owned + } + // Every handler that takes a worktree path first and reaches git or the + // disk registers through these two, so a new one can't skip the check. + // Store-only handlers (panes, aliases, pins) don't need it: a foreign + // path there costs nothing. + type WorktreeHandler = (ctx: ConnectionContext, worktreePath: string, ...rest: any[]) => unknown // eslint-disable-line @typescript-eslint/no-explicit-any + const onWorktreeRequest = (name: string, handler: WorktreeHandler): void => + transport.onRequest(name, ownedPathsOnly(isOwnPath, handler)) + const onWorktreeSignal = (name: string, handler: WorktreeHandler): void => + transport.onSignal(name, ownedPathsOnly(isOwnPath, handler, 'drop')) + // Identical concurrent asks share one `git status`. A client that re-sends // the same sweep on every render (the 2.18 Cleanup modal did, once per // streamed token) otherwise queues thousands of reads, and each spawn @@ -1982,7 +2009,7 @@ function registerIpcHandlers(): void { // Keyed on priority too, so a bulk sweep never hands its queued answer // to an interactive caller. const dirtyFlight = createSingleFlight() - transport.onRequest('worktree:isDirty', async (_ctx, path: string, opts?: { bulk?: boolean }) => { + onWorktreeRequest('worktree:isDirty', async (_ctx, path: string, opts?: { bulk?: boolean }) => { const priority = opts?.bulk ? 'bulk' : 'interactive' const git = await dirtyFlight(`${priority}\0${path}`, () => isWorktreeDirty(path, priority)) const scratchpad = hasScratchpadNote(store.getSnapshot().state.scratchpad, path) @@ -2117,11 +2144,11 @@ function registerIpcHandlers(): void { }) // Changed files - transport.onRequest('worktree:changedFiles', async (_ctx, worktreePath: string, mode?: 'working' | 'branch') => { + onWorktreeRequest('worktree:changedFiles', async (_ctx, worktreePath: string, mode?: 'working' | 'branch') => { return getChangedFiles(worktreePath, mode ?? 'working') }) - transport.onSignal('worktree:watchChangedFiles', (ctx, worktreePath: string) => { + onWorktreeSignal('worktree:watchChangedFiles', (ctx, worktreePath: string) => { if (!worktreePath) return let subs = worktreeWatchSubs.get(ctx.clientId) if (!subs) { @@ -2148,7 +2175,7 @@ function registerIpcHandlers(): void { // (worktreePath, relativePath) tuple when the absolute file changes on // disk. Used by FileView and working-tree ReviewDiffPane to refresh // when an agent (or any external tool) edits the same file. - transport.onSignal( + onWorktreeSignal( 'file:watchSubscribe', (ctx, worktreePath: string, relativePath: string) => { if (!worktreePath || !relativePath) return @@ -2181,7 +2208,7 @@ function registerIpcHandlers(): void { } ) - transport.onRequest( + onWorktreeRequest( 'worktree:fileDiff', async (_ctx, worktreePath: string, @@ -2193,33 +2220,33 @@ function registerIpcHandlers(): void { } ) - transport.onRequest('worktree:listFiles', async (_ctx, worktreePath: string) => { + onWorktreeRequest('worktree:listFiles', async (_ctx, worktreePath: string) => { return listAllFiles(worktreePath) }) - transport.onRequest('worktree:recentCommitShas', async (_ctx, worktreePath: string) => { + onWorktreeRequest('worktree:recentCommitShas', async (_ctx, worktreePath: string) => { return listRecentCommitShas(worktreePath) }) - transport.onRequest('worktree:readFile', async (_ctx, worktreePath: string, filePath: string) => { + onWorktreeRequest('worktree:readFile', async (_ctx, worktreePath: string, filePath: string) => { return readWorktreeFile(worktreePath, filePath) }) - transport.onRequest( + onWorktreeRequest( 'worktree:readFileBinary', async (_ctx, worktreePath: string, filePath: string) => { return readWorktreeFileBinary(worktreePath, filePath) } ) - transport.onRequest( + onWorktreeRequest( 'worktree:writeFile', async (_ctx, worktreePath: string, filePath: string, contents: string) => { return writeWorktreeFile(worktreePath, filePath, contents) } ) - transport.onRequest( + onWorktreeRequest( 'worktree:fileDiffSides', async (_ctx, worktreePath: string, @@ -2231,15 +2258,15 @@ function registerIpcHandlers(): void { } ) - transport.onRequest('worktree:branchCommits', async (_ctx, worktreePath: string) => { + onWorktreeRequest('worktree:branchCommits', async (_ctx, worktreePath: string) => { return getBranchCommits(worktreePath) }) - transport.onRequest('tools:list', async (_ctx, worktreePath: string) => { + onWorktreeRequest('tools:list', async (_ctx, worktreePath: string) => { return discoverTools(worktreePath) }) - transport.onRequest('tools:run', async (_ctx, worktreePath: string, toolId: string) => { + onWorktreeRequest('tools:run', async (_ctx, worktreePath: string, toolId: string) => { const wt = store.getSnapshot().state.worktrees.list.find((w) => w.path === worktreePath) return runTool(worktreePath, toolId, { branch: wt?.branch ?? '', @@ -2247,33 +2274,33 @@ function registerIpcHandlers(): void { }) }) - transport.onRequest('worktree:commitDiff', async (_ctx, worktreePath: string, hash: string) => { + onWorktreeRequest('worktree:commitDiff', async (_ctx, worktreePath: string, hash: string) => { return getCommitDiff(worktreePath, hash) }) - transport.onRequest('worktree:commitMeta', async (_ctx, worktreePath: string, hash: string) => { + onWorktreeRequest('worktree:commitMeta', async (_ctx, worktreePath: string, hash: string) => { return getCommitMeta(worktreePath, hash) }) - transport.onRequest('worktree:commitChangedFiles', async (_ctx, worktreePath: string, hash: string) => { + onWorktreeRequest('worktree:commitChangedFiles', async (_ctx, worktreePath: string, hash: string) => { return getCommitChangedFiles(worktreePath, hash) }) - transport.onRequest( + onWorktreeRequest( 'worktree:commitFileDiffSides', async (_ctx, worktreePath: string, hash: string, filePath: string) => { return getCommitFileDiffSides(worktreePath, hash, filePath) } ) - transport.onRequest( + onWorktreeRequest( 'worktree:commitRangeChangedFiles', async (_ctx, worktreePath: string, fromHash: string, toHash: string) => { return getCommitRangeChangedFiles(worktreePath, fromHash, toHash) } ) - transport.onRequest( + onWorktreeRequest( 'worktree:commitRangeFileDiffSides', async (_ctx, worktreePath: string, fromHash: string, toHash: string, filePath: string) => { return getCommitRangeFileDiffSides(worktreePath, fromHash, toHash, filePath) @@ -3288,7 +3315,7 @@ function registerIpcHandlers(): void { return AVAILABLE_EDITORS.map(({ id, name }) => ({ id, name })) }) - transport.onRequest('editor:open', (_ctx, worktreePath: string, filePath?: string) => { + onWorktreeRequest('editor:open', (_ctx, worktreePath: string, filePath?: string) => { return openInEditor(resolveEditorForWorktree(worktreePath), worktreePath, filePath) }) @@ -3636,13 +3663,13 @@ function registerIpcHandlers(): void { return true }) - transport.onRequest('agent:sessionFileExists', async (_ctx, cwd: string, sessionId: string, agentKind?: string): Promise => { + onWorktreeRequest('agent:sessionFileExists', async (_ctx, cwd: string, sessionId: string, agentKind?: string): Promise => { const kind = toAgentKind(agentKind) if (kind === 'claude') await resolveShellEnv(cwd) return getAgent(kind).sessionFileExists(cwd, sessionId) }) - transport.onRequest('agent:latestSessionId', async (_ctx, cwd: string, agentKind?: string): Promise => { + onWorktreeRequest('agent:latestSessionId', async (_ctx, cwd: string, agentKind?: string): Promise => { const kind = toAgentKind(agentKind) if (kind === 'claude') await resolveShellEnv(cwd) return getAgent(kind).latestSessionId(cwd) @@ -3656,6 +3683,7 @@ function registerIpcHandlers(): void { sessionName?: string; modelOverride?: string }): Promise => { + if (!isOwnPath(opts.cwd)) throw new Error(`Not a worktree on this host: ${opts.cwd}`) const kind = toAgentKind(agentKind) const agent = getAgent(kind) // --resume vs --session-id is decided by looking for the transcript diff --git a/src/main/own-paths.test.ts b/src/main/own-paths.test.ts new file mode 100644 index 00000000..916e6eb1 --- /dev/null +++ b/src/main/own-paths.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect, vi } from 'vitest' +import { isWithinRoots, ownedPathsOnly } from './own-paths' + +describe('isWithinRoots', () => { + const roots = ['/Users/me/repo', '/Users/me/repo-worktrees/feat'] + + it('accepts a root itself and paths inside one', () => { + expect(isWithinRoots('/Users/me/repo', roots)).toBe(true) + expect(isWithinRoots('/Users/me/repo/src/a.ts', roots)).toBe(true) + expect(isWithinRoots('/Users/me/repo-worktrees/feat', roots)).toBe(true) + }) + + it("rejects another host's path", () => { + expect(isWithinRoots('/home/me/personal-assistant', roots)).toBe(false) + }) + + it('does not treat a shared name prefix as containment', () => { + expect(isWithinRoots('/Users/me/repo-other', roots)).toBe(false) + }) + + it('handles roots with a trailing slash, and empty input', () => { + expect(isWithinRoots('/a/b/c', ['/a/b/'])).toBe(true) + expect(isWithinRoots('', roots)).toBe(false) + expect(isWithinRoots('/a', [''])).toBe(false) + }) +}) + +describe('ownedPathsOnly', () => { + const isOwn = (p: string) => isWithinRoots(p, ['/Users/me/repo']) + + it('runs the handler for an owned path, passing every argument through', () => { + const handler = vi.fn((_ctx: string, path: string, a: number, b: string) => `${path}:${a}:${b}`) + const guarded = ownedPathsOnly(isOwn, handler) + expect(guarded('ctx', '/Users/me/repo/src', 1, 'x')).toBe('/Users/me/repo/src:1:x') + expect(handler).toHaveBeenCalledWith('ctx', '/Users/me/repo/src', 1, 'x') + }) + + it('rejects a foreign path without running the handler', () => { + const handler = vi.fn() + expect(() => ownedPathsOnly(isOwn, handler)('ctx', '/home/me/repo')).toThrow(/Not a worktree on this host/) + expect(handler).not.toHaveBeenCalled() + }) + + it('drops a foreign path silently in signal mode', () => { + const handler = vi.fn() + expect(ownedPathsOnly(isOwn, handler, 'drop')('ctx', '/home/me/repo')).toBeUndefined() + expect(handler).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/own-paths.ts b/src/main/own-paths.ts new file mode 100644 index 00000000..1c3e0e5b --- /dev/null +++ b/src/main/own-paths.ts @@ -0,0 +1,42 @@ +// Whether a path belongs to this backend: equal to, or inside, one of its +// worktrees or repo roots. +// +// A client that routes a worktree request to the wrong backend sends a +// path that doesn't exist on this machine. Running git there isn't just a +// wasted call: spawn blocks the main thread until the child has entered +// its cwd, and on macOS `/home` is an autofs mount, so a Linux remote's +// `/home//…` path costs about a second of frozen main per spawn +// while the automounter gives up. A panel polling a remote worktree +// against the local backend beach-balled the app that way. This check is +// pure string work — it must never touch the filesystem, or it would hit +// the same automounter it exists to avoid. +export function isWithinRoots(path: string, roots: Iterable): boolean { + if (!path) return false + for (const root of roots) { + if (!root) continue + if (path === root) return true + const prefix = root.endsWith('/') ? root : `${root}/` + if (path.startsWith(prefix)) return true + } + return false +} + +/** Wraps a handler whose first argument after the context is a worktree + * path so it only runs for paths `isOwn` accepts. A request handler + * rejects a foreign path; a signal handler (`mode: 'drop'`) ignores it, + * since a signal has no caller to report to. Every worktree handler that + * reaches git or the disk registers through this, so a new one can't + * forget the check. */ +export function ownedPathsOnly( + isOwn: (path: string) => boolean, + handler: (ctx: C, path: string, ...rest: any[]) => R, // eslint-disable-line @typescript-eslint/no-explicit-any + mode: 'reject' | 'drop' = 'reject' +): (ctx: C, path: string, ...rest: any[]) => R | undefined { // eslint-disable-line @typescript-eslint/no-explicit-any + return (ctx, path, ...rest) => { + if (!isOwn(path)) { + if (mode === 'drop') return undefined + throw new Error(`Not a worktree on this host: ${path}`) + } + return handler(ctx, path, ...rest) + } +} diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index c98c897d..416285a1 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -1,6 +1,6 @@ import { useState, useEffect, useCallback, useRef, useMemo, useOptimistic, startTransition } from 'react' import { NessMark } from './components/NessMark' -import { LOCAL_BACKEND_ID, useAppState, useLocalAppState, useActiveBackend, useAllBackendStates, useSettings, usePrs, useOnboarding, useHooks, useWorktrees, useTerminals, usePanes, useLastActive, useUpdater, useRepoConfigs, useAnnouncements, useAssignedPRs, useConfigLoadError, useAliasForWorktree } from './store' +import { LOCAL_BACKEND_ID, useAppState, useLocalAppState, useActiveBackend, useAllBackendStatesSelector, useSettings, usePrs, useOnboarding, useHooks, useWorktrees, useTerminals, usePanes, useLastActive, useUpdater, useRepoConfigs, useAnnouncements, useAssignedPRs, useConfigLoadError, useAliasForWorktree } from './store' import { getBackendFor } from './backend' import { useGlobalBackend, useBackendForWorktree } from './backend-routing' import { useTailLineBuffer } from './hooks/useTailLineBuffer' @@ -31,7 +31,8 @@ import { QuakeTerminal } from './components/QuakeTerminal' import { RightColumn } from './components/RightColumn' import { BackendScope } from './backend-scope' import { repoKeyFor } from './worktree-list-model' -import { useMergedWorktrees } from './hooks/useMergedWorktrees' +import { useMergedWorktrees, useFleetWorktrees } from './hooks/useMergedWorktrees' +import { getFleetWorktreeLists } from './merge-backend-states' import { CollapsedSidebar } from './components/CollapsedSidebar' import { CollapsedRightPanel } from './components/CollapsedRightPanel' import { Settings } from './components/Settings' @@ -124,11 +125,11 @@ function DesktopApp(): JSX.Element { // useWorktreeListModel), so rows and mounted workspaces can't disagree // about which backends exist. const mergedData = useMergedWorktrees() - const mergedWorktrees = mergedData.worktrees + const fleetData = useFleetWorktrees() // Routes a per-worktree call to the backend hosting that worktree. // App has no ambient handle that can reach these methods — see // backend-routing.ts for why. - const backendFor = useBackendForWorktree(mergedData.pathBackendId) + const backendFor = useBackendForWorktree(fleetData.pathBackendId) // Backend label per worktree path, for rows that aren't on this machine. // Local ones are deliberately absent — badging the common case is noise, // and the badge's job is to make "this acts on another machine" obvious @@ -180,26 +181,16 @@ function DesktopApp(): JSX.Element { // remote worktrees were open. Without this, a worktree recreated at the // same path — or a backend re-added — would mount eagerly on reappearance // without the user opening it: the fan-out this set exists to prevent. - const fleetBackends = useAllBackendStates() - // `fleetBackends` is a new array on any backend event, status ticks - // included. The pruning only cares about worktree LISTS, so compare - // those by reference and bail before touching state when none moved. - const fleetListsRef = useRef>(new Map()) + // Subscribes to the worktree LISTS alone, not every backend's state: + // holding the full states re-rendered App on every streamed chat token + // from any host. This only changes when some backend's list does. + const fleetWorktreeLists = useAllBackendStatesSelector(getFleetWorktreeLists) useEffect(() => { - const prev = fleetListsRef.current - const next = new Map() - let changed = prev.size !== fleetBackends.length - for (const b of fleetBackends) { - next.set(b.backendId, b.state.worktrees.list) - if (prev.get(b.backendId) !== b.state.worktrees.list) changed = true - } - fleetListsRef.current = next - if (!changed) return setOpenedWorktrees((prev) => { if (prev.size === 0) return prev const live = new Set() - for (const b of fleetBackends) { - for (const w of b.state.worktrees.list) live.add(`${b.backendId}\u0000${w.path}`) + for (const b of fleetWorktreeLists) { + for (const w of b.value) live.add(`${b.backendId}\u0000${w.path}`) } let next: Set | null = null for (const key of prev) { @@ -210,7 +201,7 @@ function DesktopApp(): JSX.Element { } return next ?? prev }) - }, [fleetBackends]) + }, [fleetWorktreeLists]) // Worktree list, repoRoots, and pending-creation FSM all live in the // main-process store. activeWorktreeId stays local — it's per-client view // focus that eventually becomes per-window. @@ -261,7 +252,7 @@ function DesktopApp(): JSX.Element { // by ANY route now implies its backend. Ambiguous only when two backends // share an absolute path — the known composite-identity gap. const selectedBackendId = - (activeWorktreeId ? mergedData.pathBackendId[activeWorktreeId] : undefined) ?? + (activeWorktreeId ? fleetData.pathBackendId[activeWorktreeId] : undefined) ?? activeBackendId const switchWorktree = useCallback( (path: string, alsoInTransition?: () => void) => { @@ -869,9 +860,9 @@ const setQuestStep = useCallback((next: QuestStep) => { handleConfirmRepoResolve, handleDismissRepoPrompt } = useWorktreeHandlers({ - pathBackendId: mergedData.pathBackendId, - repoBackendId: mergedData.repoBackendId, - pendingBackendId: mergedData.pendingBackendId, + pathBackendId: fleetData.pathBackendId, + repoBackendId: fleetData.repoBackendId, + pendingBackendId: fleetData.pendingBackendId, worktrees, pendingWorktrees, repoRoots, @@ -913,7 +904,7 @@ const setQuestStep = useCallback((next: QuestStep) => { setActivePaneId, activeWorktreeId, setActiveWorktreeId, - pathBackendId: mergedData.pathBackendId + pathBackendId: fleetData.pathBackendId }) // Hoisted out of the WorkspaceView JSX below. Every worktree stays mounted, @@ -1053,7 +1044,7 @@ const setQuestStep = useCallback((next: QuestStep) => { // Sidebar-aware hotkey handlers + the resolved binding map for tooltips. // The hook also subscribes to keystrokes via useHotkeys internally. const { hotkeyActions, resolvedHotkeys } = useHotkeyHandlers({ - pathBackendId: mergedData.pathBackendId, + pathBackendId: fleetData.pathBackendId, worktrees, repoRoots, unifiedRepos, @@ -1176,7 +1167,7 @@ const setQuestStep = useCallback((next: QuestStep) => {
{ onOpenAssignedPR={handleOpenAssignedPR} agentCount={agentWorktreeCount} onSelectWorktree={handleSidebarSelectWorktree} - pathBackendId={mergedData.pathBackendId} + pathBackendId={fleetData.pathBackendId} originByPath={originByPath} onSelectTab={handleSidebarSelectTab} onDismissPendingWorktree={handleDismissPendingWorktree} @@ -1840,9 +1831,10 @@ const setQuestStep = useCallback((next: QuestStep) => { {/* Render ALL worktrees' terminals to keep PTYs alive across switches. Now spans every backend, not just the active one — each subtree is pinned to its owning backend below, so a remote worktree's panes - keep streaming while a local one is on screen. */} - {mergedWorktrees.map(({ backendId, worktree: wt }) => { - const paneTree = panes[wt.path] + keep streaming while a local one is on screen. Driven by the + whole fleet, not the view-filtered list: see useFleetWorktrees. */} + {fleetData.worktrees.map(({ backendId, worktree: wt }) => { + const paneTree = fleetData.panes[wt.path] if (!paneTree) return null const leaves = getLeaves(paneTree) if (leaves.length === 0 || !leaves.some((l) => l.tabs.length > 0)) return null @@ -1854,7 +1846,7 @@ const setQuestStep = useCallback((next: QuestStep) => { const everOpened = openedWorktrees.has(mountKey) const isSelected = wt.path === activeWorktreeId && backendId === selectedBackendId if (!eagerMount && !everOpened && !isSelected) return null - const isVisible = !showNewWorktree && !showActivity && !showCleanup && !showCommandCenter && reportIssueState === null && wt.path === activeWorktreeId && !pendingDeletionByPath[wt.path] + const isVisible = !showNewWorktree && !showActivity && !showCleanup && !showCommandCenter && reportIssueState === null && isSelected && !pendingDeletionByPath[wt.path] return (
() +// +// Kept per backend: a path is only unique within one host, and with the +// whole fleet mounted two hosts with the same layout would otherwise show +// each other's files. `useBackend()` hands out one object per backend, so +// it keys the WeakMap. +// +// Listings also single-flight. Chat tabs in one worktree mount together +// (app start, a remount), all before the first listing lands, so without +// it each one shelled out — and each reply carries the whole file list +// across IPC. On a large repo that burst alone stalled the main process +// for seconds. const FILE_CACHE_TTL_MS = 10_000 +interface BackendFileCache { + files: Map + flight: (key: string, fn: () => Promise) => Promise +} +const FILE_CACHES = new WeakMap() + +function fileCacheFor(backend: ElectronAPI): BackendFileCache { + let cache = FILE_CACHES.get(backend) + if (!cache) { + cache = { files: new Map(), flight: createSingleFlight() } + FILE_CACHES.set(backend, cache) + } + return cache +} + +function listWorktreeFilesOnce(backend: ElectronAPI, worktreePath: string): Promise { + const cache = fileCacheFor(backend) + return cache.flight(worktreePath, () => + backend.listAllFiles(worktreePath).then((files) => { + cache.files.set(worktreePath, { files, ts: Date.now() }) + return files + }) + ) +} const MAX_MENTION_RESULTS = 50 // `@worktree:/` is what a worktree mention inserts. The prefix @@ -2188,22 +2224,22 @@ export function JsonModeChat({ sessionId, worktreePath, mode = 'awake' }: JsonMo // module scope so reopening the popover (or clicking through several // json-claude tabs in the same worktree) doesn't re-shell every time. useEffect(() => { - const cached = FILE_CACHE.get(worktreePath) + const cached = fileCacheFor(backend).files.get(worktreePath) const now = Date.now() if (cached && now - cached.ts < FILE_CACHE_TTL_MS) { setFiles(cached.files) return } let cancelled = false - void backend.listAllFiles(worktreePath).then((result) => { - if (cancelled) return - FILE_CACHE.set(worktreePath, { files: result, ts: Date.now() }) - setFiles(result) - }) + void listWorktreeFilesOnce(backend, worktreePath) + .then((result) => { + if (!cancelled) setFiles(result) + }) + .catch(() => {}) return () => { cancelled = true } - }, [worktreePath]) + }, [worktreePath, backend]) // Find the most recent trigger char (`/` or `@`) before the cursor. // The token between trigger+1 and cursor is the query. Returns null if diff --git a/src/renderer/hooks/useMergedWorktrees.ts b/src/renderer/hooks/useMergedWorktrees.ts index 83e1cc19..96b238f2 100644 --- a/src/renderer/hooks/useMergedWorktrees.ts +++ b/src/renderer/hooks/useMergedWorktrees.ts @@ -1,19 +1,44 @@ // React face of `merge-backend-states.ts`. Thin on purpose: the merge is // pure and cached at module level over there, so this hook only gathers -// the three reactive inputs and hands back the shared result. Every -// caller — App and useWorktreeListModel — therefore receives the same -// object for the same inputs, and the merge runs once per change between -// them. +// the reactive inputs and hands back the shared result. Every caller — +// App and useWorktreeListModel — therefore receives the same object for +// the same inputs, and the merge runs once per change between them. +// +// It subscribes through the merge rather than to the raw backend states. +// Those change on every event from every backend, a streamed chat token +// included, and holding them re-rendered App per token. The merge hands +// back its previous result when an event changed nothing it reads, so +// useSyncExternalStore bails and the render never happens. -import { useActiveBackend, useAllBackendStates } from '../store' +import { useCallback } from 'react' +import { useActiveBackend, useAllBackendStatesSelector, type BackendStateEntry } from '../store' import { useFleetViewMode } from '../fleet-view' import { getMergedWorktrees, type MergedWorktreeData } from '../merge-backend-states' export type { MergedWorktree, MergedWorktreeData } from '../merge-backend-states' export function useMergedWorktrees(): MergedWorktreeData { - const allBackends = useAllBackendStates() const viewMode = useFleetViewMode() const activeId = useActiveBackend().id - return getMergedWorktrees(allBackends, viewMode, activeId) + const select = useCallback( + (entries: readonly BackendStateEntry[]) => getMergedWorktrees(entries, viewMode, activeId), + [viewMode, activeId] + ) + return useAllBackendStatesSelector(select) +} + +/** Every backend's worktrees whatever the view mode — what stays MOUNTED, + * as opposed to what the sidebar lists. Single mode filters the list to + * the active host; rendering workspaces from that filtered list unmounted + * every other host's worktrees on a chip switch and mounted them all back + * on the return trip. Switching back to Local remounted 38 worktrees and + * 64 chat tabs at once, and the burst of file listings and scrollback + * replays it set off froze the main process for seconds. */ +export function useFleetWorktrees(): MergedWorktreeData { + const activeId = useActiveBackend().id + const select = useCallback( + (entries: readonly BackendStateEntry[]) => getMergedWorktrees(entries, 'merged', activeId), + [activeId] + ) + return useAllBackendStatesSelector(select) } diff --git a/src/renderer/merge-backend-states.test.ts b/src/renderer/merge-backend-states.test.ts index 9b062a18..ace65802 100644 --- a/src/renderer/merge-backend-states.test.ts +++ b/src/renderer/merge-backend-states.test.ts @@ -10,6 +10,7 @@ import type { Worktree, PendingWorktree } from '../shared/state/worktrees' import { mergeBackendStates, getMergedWorktrees, + getFleetWorktreeLists, resetMergedWorktreesCache, tabsFromPanes, type BackendStateLike @@ -209,6 +210,45 @@ describe('getMergedWorktrees (shared cache)', () => { } }) + it('a streamed chat token on a remote leaves the merge untouched', () => { + // A token is a jsonClaude event: a new AppState for that backend, with + // nothing the merge reads changed. App subscribes through the merge, + // so this is what keeps it from re-rendering per token. + const list = [wt('/r/a', '/r')] + const before = backend('beta', { worktrees: list }) + const after: BackendStateLike = { + ...before, + state: { ...before.state, jsonClaude: { ...before.state.jsonClaude } } + } + const local = backend('local', { kind: 'local' }) + for (const mode of ['merged', 'single'] as const) { + const first = getMergedWorktrees([local, before], mode, 'local') + expect(getMergedWorktrees([local, after], mode, 'local')).toBe(first) + } + }) + + it('keeps one cache per view mode, so reading both never churns either', () => { + // App reads the visible view and the full fleet on every render. With + // one shared slot, alternating reads evicted each other and handed back + // a fresh object per read. + const entries = [backend('local', { kind: 'local', worktrees: [wt('/l/a', '/l')] }), backend('beta')] + const single = getMergedWorktrees(entries, 'single', 'local') + const merged = getMergedWorktrees(entries, 'merged', 'local') + expect(getMergedWorktrees(entries, 'single', 'local')).toBe(single) + expect(getMergedWorktrees(entries, 'merged', 'local')).toBe(merged) + }) + + it('the fleet view keeps a hidden backend mounted in single mode', () => { + const entries = [ + backend('local', { kind: 'local', worktrees: [wt('/l/a', '/l')] }), + backend('beta', { worktrees: [wt('/b/a', '/b')] }) + ] + const visible = getMergedWorktrees(entries, 'single', 'beta') + const fleet = getMergedWorktrees(entries, 'merged', 'beta') + expect(visible.flatWorktrees.map((w) => w.path)).toEqual(['/b/a']) + expect(fleet.flatWorktrees.map((w) => w.path)).toEqual(['/l/a', '/b/a']) + }) + it('returns the previous result when an event changed nothing it reads', () => { const list = [wt('/w', '/r')] const first = getMergedWorktrees( @@ -236,3 +276,20 @@ describe('getMergedWorktrees (shared cache)', () => { expect(second.flatWorktrees.map((w) => w.path)).toEqual(['/a', '/b']) }) }) + +describe('getFleetWorktreeLists', () => { + it('stays the same array until some backend list changes', () => { + const list = [wt('/b/a', '/b')] + const before = backend('beta', { worktrees: list }) + const first = getFleetWorktreeLists([before]) + const streamed: BackendStateLike = { + ...before, + state: { ...before.state, jsonClaude: { ...before.state.jsonClaude } } + } + expect(getFleetWorktreeLists([streamed])).toBe(first) + + const grown = getFleetWorktreeLists([backend('beta', { worktrees: [...list, wt('/b/b', '/b')] })]) + expect(grown).not.toBe(first) + expect(grown[0].value.map((w) => w.path)).toEqual(['/b/a', '/b/b']) + }) +}) diff --git a/src/renderer/merge-backend-states.ts b/src/renderer/merge-backend-states.ts index 79b47a99..c33a56db 100644 --- a/src/renderer/merge-backend-states.ts +++ b/src/renderer/merge-backend-states.ts @@ -203,12 +203,17 @@ const tabsSelector = new SegmentedSelector tabsFromPanes(state.terminals.panes) ) -let cache: { +interface MergeCache { entries: readonly BackendStateLike[] - viewMode: FleetViewMode activeId: string result: MergedWorktreeData -} | null = null +} + +/** One entry per view mode. App reads the visible view for the sidebar and + * the full fleet for what stays mounted, both on every render; a single + * shared slot would evict one with the other each time and hand back a + * new object per read, which useSyncExternalStore treats as a change. */ +const caches: Record = { single: null, merged: null } /** The shared, reference-stable merge. Same three inputs → same object, * across every caller — so App and useWorktreeListModel pay for one @@ -218,12 +223,8 @@ export function getMergedWorktrees( viewMode: FleetViewMode, activeId: string ): MergedWorktreeData { - if ( - cache && - cache.entries === entries && - cache.viewMode === viewMode && - cache.activeId === activeId - ) { + const cache = caches[viewMode] + if (cache && cache.entries === entries && cache.activeId === activeId) { return cache.result } // All entries, not just the visible ones, so flipping view mode never @@ -236,7 +237,7 @@ export function getMergedWorktrees( tabsFor: (backendId) => tabsById.get(backendId) ?? {} }) const result = cache ? reuseUnchanged(cache.result, fresh) : fresh - cache = { entries, viewMode, activeId, result } + caches[viewMode] = { entries, activeId, result } return result } @@ -297,5 +298,20 @@ function isRecord(v: unknown): v is Record { /** Test-only: forget the shared cache between cases. */ export function resetMergedWorktreesCache(): void { - cache = null + caches.single = null + caches.merged = null +} + +const worktreeListsSelector = new SegmentedSelector( + (state) => [state.worktrees.list], + (state) => state.worktrees.list +) + +/** Each backend's worktree list, visible or not. Same array back until + * some backend's list actually changes, so App can watch the fleet for + * deletions without re-rendering on every other event. */ +export function getFleetWorktreeLists( + entries: readonly BackendStateLike[] +): readonly { backendId: string; value: Worktree[] }[] { + return worktreeListsSelector.select(entries.map((e) => ({ id: e.backendId, state: e.state }))) } diff --git a/src/renderer/store.ts b/src/renderer/store.ts index 803e3650..14704254 100644 --- a/src/renderer/store.ts +++ b/src/renderer/store.ts @@ -554,6 +554,26 @@ export function useAllBackendStates(): readonly BackendStateEntry[] { ) } +/** Every backend's state, narrowed through `select` — re-renders only when + * the selected value changes by reference. + * + * Prefer this to `useAllBackendStates` anywhere near the root. That hook's + * array changes on every event from every backend, so a component holding + * it re-renders per streamed chat token on any host; App did, which made + * the whole UI lag whenever a remote chat was streaming. `select` must be + * a cached projection that returns the same reference for unchanged + * inputs (getMergedWorktrees, a SegmentedSelector) — a fresh object per + * call makes useSyncExternalStore re-render forever. */ +export function useAllBackendStatesSelector( + select: (entries: readonly BackendStateEntry[]) => T +): T { + return useSyncExternalStore( + subscribeAllBackends, + () => select(getAllBackendStatesSnapshot()), + () => select(EMPTY_BACKEND_STATES) + ) +} + const EMPTY_BACKEND_STATES: readonly BackendStateEntry[] = [] const getAllBackendStatesServerSnapshot = (): readonly BackendStateEntry[] => EMPTY_BACKEND_STATES diff --git a/src/main/single-flight.test.ts b/src/shared/single-flight.test.ts similarity index 100% rename from src/main/single-flight.test.ts rename to src/shared/single-flight.test.ts diff --git a/src/main/single-flight.ts b/src/shared/single-flight.ts similarity index 100% rename from src/main/single-flight.ts rename to src/shared/single-flight.ts