From a5e410e7cffe8bd4890a960fc5904b41f7d750d7 Mon Sep 17 00:00:00 2001 From: Mike Lyons Date: Tue, 3 Nov 2026 11:07:23 -0500 Subject: [PATCH 1/4] Stop remote streaming and chip switches from freezing the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two 2.18 fleet-view regressions behind the beach ball users see while a remote chat streams: - App subscribed to every backend's whole AppState (directly and through useMergedWorktrees), so each streamed token on any host re-rendered the root. It now subscribes through cached projections — the merge, and a per-backend worktree-list selector — and useSyncExternalStore bails when they're unchanged. - The keep-everything-mounted loop rendered from the view-filtered list, so in single mode a chip switch unmounted every other host's worktrees and the return trip remounted them all. Switching back to Local remounted 38 worktrees / 64 chat tabs at once; 58 file listings in one second froze local main for ~5s. The loop now spans the whole fleet, with one merge cache per view mode so both stay reference-stable. Chat tabs also dedupe their in-flight file listing per worktree. Co-Authored-By: Claude Opus 5.5 --- src/renderer/App.tsx | 40 +++++++--------- src/renderer/components/JsonModeChat.tsx | 31 ++++++++++-- src/renderer/hooks/useMergedWorktrees.ts | 39 +++++++++++++--- src/renderer/merge-backend-states.test.ts | 57 +++++++++++++++++++++++ src/renderer/merge-backend-states.ts | 38 ++++++++++----- src/renderer/store.ts | 20 ++++++++ 6 files changed, 178 insertions(+), 47 deletions(-) diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index c98c897d..3202ac86 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,7 +125,7 @@ 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. @@ -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. @@ -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 (
() const FILE_CACHE_TTL_MS = 10_000 +// Chat tabs in one worktree mount together (app start, a remount), all +// before the first listing lands, so without this 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_INFLIGHT = new Map>() + +function listWorktreeFilesOnce( + worktreePath: string, + list: (worktreePath: string) => Promise +): Promise { + const pending = FILE_INFLIGHT.get(worktreePath) + if (pending) return pending + const started = list(worktreePath) + .then((files) => { + FILE_CACHE.set(worktreePath, { files, ts: Date.now() }) + return files + }) + .finally(() => FILE_INFLIGHT.delete(worktreePath)) + FILE_INFLIGHT.set(worktreePath, started) + return started +} const MAX_MENTION_RESULTS = 50 // `@worktree:/` is what a worktree mention inserts. The prefix @@ -2195,11 +2216,11 @@ export function JsonModeChat({ sessionId, worktreePath, mode = 'awake' }: JsonMo 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(worktreePath, (p) => backend.listAllFiles(p)) + .then((result) => { + if (!cancelled) setFiles(result) + }) + .catch(() => {}) return () => { cancelled = true } 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 From f79075b0a471d11c92224bc55efcfd00b6d296ab Mon Sep 17 00:00:00 2001 From: Mike Lyons Date: Tue, 3 Nov 2026 11:13:06 -0500 Subject: [PATCH 2/4] Address review: share single-flight, key chat file cache per backend - Move createSingleFlight to src/shared so the renderer reuses the tested helper instead of a second untested copy. - The chat @-mention file cache and its in-flight dedupe are now per backend. With the whole fleet mounted, two hosts with the same paths would otherwise have shared one listing. Co-Authored-By: Claude Opus 5.5 --- src/main/index.ts | 2 +- src/renderer/components/JsonModeChat.tsx | 59 ++++++++++++++-------- src/{main => shared}/single-flight.test.ts | 0 src/{main => shared}/single-flight.ts | 0 4 files changed, 38 insertions(+), 23 deletions(-) rename src/{main => shared}/single-flight.test.ts (100%) rename src/{main => shared}/single-flight.ts (100%) diff --git a/src/main/index.ts b/src/main/index.ts index 2ea4ad9f..268ca6c2 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -36,7 +36,7 @@ import type { ServerTransport } 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 { knownClaudeConfigDirs, onShellEnvCaptured, diff --git a/src/renderer/components/JsonModeChat.tsx b/src/renderer/components/JsonModeChat.tsx index 09e69eb2..dd2e0fed 100644 --- a/src/renderer/components/JsonModeChat.tsx +++ b/src/renderer/components/JsonModeChat.tsx @@ -63,6 +63,8 @@ import { } from './composer-height' import { fuzzyMatch } from '../fuzzy' import { worktreeHandle } from '../../shared/state/worktrees' +import { createSingleFlight } from '../../shared/single-flight' +import type { ElectronAPI } from '../types' import { CLAUDE_MODELS, type ModelOption, @@ -286,28 +288,41 @@ const MarkdownWithFind = memo(function MarkdownWithFind({ // Worktree file list cache. Same TTL/shape as CommandPalette uses — the // list rarely changes during a typing session, and listAllFiles shells // out to git ls-files which is cheap but not free on big repos. -const FILE_CACHE = new Map() +// +// 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 -// Chat tabs in one worktree mount together (app start, a remount), all -// before the first listing lands, so without this 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_INFLIGHT = new Map>() - -function listWorktreeFilesOnce( - worktreePath: string, - list: (worktreePath: string) => Promise -): Promise { - const pending = FILE_INFLIGHT.get(worktreePath) - if (pending) return pending - const started = list(worktreePath) - .then((files) => { - FILE_CACHE.set(worktreePath, { files, ts: Date.now() }) +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 }) - .finally(() => FILE_INFLIGHT.delete(worktreePath)) - FILE_INFLIGHT.set(worktreePath, started) - return started + ) } const MAX_MENTION_RESULTS = 50 @@ -2209,14 +2224,14 @@ 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 listWorktreeFilesOnce(worktreePath, (p) => backend.listAllFiles(p)) + void listWorktreeFilesOnce(backend, worktreePath) .then((result) => { if (!cancelled) setFiles(result) }) @@ -2224,7 +2239,7 @@ export function JsonModeChat({ sessionId, worktreePath, mode = 'awake' }: JsonMo 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/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 From 81faab592588e5ac5fab57496b727619a1e9a5bc Mon Sep 17 00:00:00 2001 From: Mike Lyons Date: Tue, 3 Nov 2026 12:11:08 -0500 Subject: [PATCH 3/4] Stop routing remote worktree reads to the local backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profiled during a repro: local main blocked 8.5s, 8.4s of it in child_process.spawn under runGitRead. perf.log shows those reads were changed-files / branch-commits for /home/mike/personal-assistant-agentjesus, a worktree on the remote, run by the LOCAL backend. On macOS /home is an autofs mount, so each spawn there sits ~1s on the main thread while the automounter gives up. The same lines appear in every incident back to the first report. Cause: App resolved a worktree's owner from the view-filtered merge. Switching the chip to Local drops the remote worktree from that map, so selectedBackendId fell back to the active backend and the right column (changed files, commits, watcher) queried the Mac for a remote path. Every App routing map now comes from the fleet-wide merge — who owns a path doesn't depend on what the sidebar shows. Main also refuses worktree reads for paths it doesn't host (string check against its worktrees, repo roots and pane keys — no filesystem access), so any future misroute fails fast instead of freezing the app. Co-Authored-By: Claude Opus 5.5 --- src/main/index.ts | 32 +++++++++++++++++++++++++++++++- src/main/own-paths.test.ts | 26 ++++++++++++++++++++++++++ src/main/own-paths.ts | 22 ++++++++++++++++++++++ src/renderer/App.tsx | 18 +++++++++--------- 4 files changed, 88 insertions(+), 10 deletions(-) create mode 100644 src/main/own-paths.test.ts create mode 100644 src/main/own-paths.ts diff --git a/src/main/index.ts b/src/main/index.ts index 268ca6c2..d9343895 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -37,6 +37,7 @@ import { detectRuntime } from './paths' import { fixPathFromLoginShell } from './path-fix' import { ChatStartGate } from './chat-start-gate' import { createSingleFlight } from '../shared/single-flight' +import { isWithinRoots } from './own-paths' import { knownClaudeConfigDirs, onShellEnvCaptured, @@ -1974,6 +1975,26 @@ 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 + } + const assertOwnPath = (path: string): void => { + if (!isOwnPath(path)) throw new Error(`Not a worktree on this host: ${path}`) + } + // 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 @@ -1983,6 +2004,7 @@ function registerIpcHandlers(): void { // to an interactive caller. const dirtyFlight = createSingleFlight() transport.onRequest('worktree:isDirty', async (_ctx, path: string, opts?: { bulk?: boolean }) => { + assertOwnPath(path) const priority = opts?.bulk ? 'bulk' : 'interactive' const git = await dirtyFlight(`${priority}\0${path}`, () => isWorktreeDirty(path, priority)) const scratchpad = hasScratchpadNote(store.getSnapshot().state.scratchpad, path) @@ -2118,11 +2140,12 @@ function registerIpcHandlers(): void { // Changed files transport.onRequest('worktree:changedFiles', async (_ctx, worktreePath: string, mode?: 'working' | 'branch') => { + assertOwnPath(worktreePath) return getChangedFiles(worktreePath, mode ?? 'working') }) transport.onSignal('worktree:watchChangedFiles', (ctx, worktreePath: string) => { - if (!worktreePath) return + if (!worktreePath || !isOwnPath(worktreePath)) return let subs = worktreeWatchSubs.get(ctx.clientId) if (!subs) { subs = new Map() @@ -2194,14 +2217,17 @@ function registerIpcHandlers(): void { ) transport.onRequest('worktree:listFiles', async (_ctx, worktreePath: string) => { + assertOwnPath(worktreePath) return listAllFiles(worktreePath) }) transport.onRequest('worktree:recentCommitShas', async (_ctx, worktreePath: string) => { + assertOwnPath(worktreePath) return listRecentCommitShas(worktreePath) }) transport.onRequest('worktree:readFile', async (_ctx, worktreePath: string, filePath: string) => { + assertOwnPath(worktreePath) return readWorktreeFile(worktreePath, filePath) }) @@ -2232,6 +2258,7 @@ function registerIpcHandlers(): void { ) transport.onRequest('worktree:branchCommits', async (_ctx, worktreePath: string) => { + assertOwnPath(worktreePath) return getBranchCommits(worktreePath) }) @@ -2248,14 +2275,17 @@ function registerIpcHandlers(): void { }) transport.onRequest('worktree:commitDiff', async (_ctx, worktreePath: string, hash: string) => { + assertOwnPath(worktreePath) return getCommitDiff(worktreePath, hash) }) transport.onRequest('worktree:commitMeta', async (_ctx, worktreePath: string, hash: string) => { + assertOwnPath(worktreePath) return getCommitMeta(worktreePath, hash) }) transport.onRequest('worktree:commitChangedFiles', async (_ctx, worktreePath: string, hash: string) => { + assertOwnPath(worktreePath) return getCommitChangedFiles(worktreePath, hash) }) diff --git a/src/main/own-paths.test.ts b/src/main/own-paths.test.ts new file mode 100644 index 00000000..17975440 --- /dev/null +++ b/src/main/own-paths.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from 'vitest' +import { isWithinRoots } 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) + }) +}) diff --git a/src/main/own-paths.ts b/src/main/own-paths.ts new file mode 100644 index 00000000..60646c54 --- /dev/null +++ b/src/main/own-paths.ts @@ -0,0 +1,22 @@ +// 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 +} diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 3202ac86..416285a1 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -129,7 +129,7 @@ function DesktopApp(): JSX.Element { // 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 @@ -252,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) => { @@ -860,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, @@ -904,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, @@ -1044,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, @@ -1167,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} From 8a68d689ab7853e8a86357b31383c8a5210af479 Mon Sep 17 00:00:00 2001 From: Mike Lyons Date: Tue, 3 Nov 2026 12:17:24 -0500 Subject: [PATCH 4/4] Address review: guard every worktree handler that reaches git or disk Register worktree-path handlers through onWorktreeRequest / onWorktreeSignal (backed by a tested ownedPathsOnly wrapper) instead of inline checks, and extend coverage to the ones that were missing: file diffs, binary reads, writes, the per-file watcher, tools, editor:open, and the agent session lookups and spawn-args (which probe a login shell in that directory). Store-only handlers are left alone. Co-Authored-By: Claude Opus 5.5 --- src/main/index.ts | 74 +++++++++++++++++++------------------- src/main/own-paths.test.ts | 27 ++++++++++++-- src/main/own-paths.ts | 20 +++++++++++ 3 files changed, 81 insertions(+), 40 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index d9343895..554383a0 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -32,12 +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 '../shared/single-flight' -import { isWithinRoots } from './own-paths' +import { isWithinRoots, ownedPathsOnly } from './own-paths' import { knownClaudeConfigDirs, onShellEnvCaptured, @@ -1991,9 +1991,15 @@ function registerIpcHandlers(): void { } return owned } - const assertOwnPath = (path: string): void => { - if (!isOwnPath(path)) throw new Error(`Not a worktree on this host: ${path}`) - } + // 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 @@ -2003,8 +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 }) => { - assertOwnPath(path) + 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) @@ -2139,13 +2144,12 @@ function registerIpcHandlers(): void { }) // Changed files - transport.onRequest('worktree:changedFiles', async (_ctx, worktreePath: string, mode?: 'working' | 'branch') => { - assertOwnPath(worktreePath) + onWorktreeRequest('worktree:changedFiles', async (_ctx, worktreePath: string, mode?: 'working' | 'branch') => { return getChangedFiles(worktreePath, mode ?? 'working') }) - transport.onSignal('worktree:watchChangedFiles', (ctx, worktreePath: string) => { - if (!worktreePath || !isOwnPath(worktreePath)) return + onWorktreeSignal('worktree:watchChangedFiles', (ctx, worktreePath: string) => { + if (!worktreePath) return let subs = worktreeWatchSubs.get(ctx.clientId) if (!subs) { subs = new Map() @@ -2171,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 @@ -2204,7 +2208,7 @@ function registerIpcHandlers(): void { } ) - transport.onRequest( + onWorktreeRequest( 'worktree:fileDiff', async (_ctx, worktreePath: string, @@ -2216,36 +2220,33 @@ function registerIpcHandlers(): void { } ) - transport.onRequest('worktree:listFiles', async (_ctx, worktreePath: string) => { - assertOwnPath(worktreePath) + onWorktreeRequest('worktree:listFiles', async (_ctx, worktreePath: string) => { return listAllFiles(worktreePath) }) - transport.onRequest('worktree:recentCommitShas', async (_ctx, worktreePath: string) => { - assertOwnPath(worktreePath) + onWorktreeRequest('worktree:recentCommitShas', async (_ctx, worktreePath: string) => { return listRecentCommitShas(worktreePath) }) - transport.onRequest('worktree:readFile', async (_ctx, worktreePath: string, filePath: string) => { - assertOwnPath(worktreePath) + 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, @@ -2257,16 +2258,15 @@ function registerIpcHandlers(): void { } ) - transport.onRequest('worktree:branchCommits', async (_ctx, worktreePath: string) => { - assertOwnPath(worktreePath) + 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 ?? '', @@ -2274,36 +2274,33 @@ function registerIpcHandlers(): void { }) }) - transport.onRequest('worktree:commitDiff', async (_ctx, worktreePath: string, hash: string) => { - assertOwnPath(worktreePath) + onWorktreeRequest('worktree:commitDiff', async (_ctx, worktreePath: string, hash: string) => { return getCommitDiff(worktreePath, hash) }) - transport.onRequest('worktree:commitMeta', async (_ctx, worktreePath: string, hash: string) => { - assertOwnPath(worktreePath) + onWorktreeRequest('worktree:commitMeta', async (_ctx, worktreePath: string, hash: string) => { return getCommitMeta(worktreePath, hash) }) - transport.onRequest('worktree:commitChangedFiles', async (_ctx, worktreePath: string, hash: string) => { - assertOwnPath(worktreePath) + 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) @@ -3318,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) }) @@ -3666,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) @@ -3686,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 index 17975440..916e6eb1 100644 --- a/src/main/own-paths.test.ts +++ b/src/main/own-paths.test.ts @@ -1,5 +1,5 @@ -import { describe, it, expect } from 'vitest' -import { isWithinRoots } from './own-paths' +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'] @@ -24,3 +24,26 @@ describe('isWithinRoots', () => { 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 index 60646c54..1c3e0e5b 100644 --- a/src/main/own-paths.ts +++ b/src/main/own-paths.ts @@ -20,3 +20,23 @@ export function isWithinRoots(path: string, roots: Iterable): boolean { } 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) + } +}