From 156134e80c8c1b2d1dcc83d14d59e6082c59cd6d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:16:58 -0700 Subject: [PATCH] Reduce duplicate worktree refresh scans (#6390) * Reduce duplicate worktree refresh scans * Preserve host identity during overlapping worktree refreshes --------- Co-authored-by: Neil --- .../src/store/slices/worktrees.test.ts | 302 ++++++++++++++++++ src/renderer/src/store/slices/worktrees.ts | 77 ++++- 2 files changed, 370 insertions(+), 9 deletions(-) diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index c2e9480ab..f63a5a0e0 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -485,6 +485,308 @@ describe('fetchWorktrees', () => { expect(result).toBe(false) }) + it('coalesces concurrent duplicate refreshes for the same repo and host', async () => { + const store = createTestStore() + const refreshed = makeWorktree({ + id: 'repo1::/path/refreshed', + repoId: 'repo1', + path: '/path/refreshed' + }) + let releaseScan!: () => void + const scanStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseScan = release + }) + return makeDetectedResult(repoId, [refreshed]) + } + ) + }) + + const requests = Array.from({ length: 8 }, () => store.getState().fetchWorktrees('repo1')) + await scanStarted + + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + + releaseScan() + await expect(Promise.all(requests)).resolves.toEqual(Array(8).fill(true)) + expect(store.getState().worktreesByRepo.repo1).toEqual([refreshed]) + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + }) + + it('coalesces fetchDetectedWorktrees with a matching fetchWorktrees refresh', async () => { + const store = createTestStore() + const refreshed = makeWorktree({ + id: 'repo1::/path/refreshed', + repoId: 'repo1', + path: '/path/refreshed' + }) + let releaseScan!: () => void + const scanStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseScan = release + }) + return makeDetectedResult(repoId, [refreshed]) + } + ) + }) + + const detectedRequest = store.getState().fetchDetectedWorktrees('repo1') + const visibleRequest = store.getState().fetchWorktrees('repo1') + await scanStarted + + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + + releaseScan() + await expect(Promise.all([detectedRequest, visibleRequest])).resolves.toEqual([ + makeDetectedResult('repo1', [refreshed]), + true + ]) + expect(store.getState().worktreesByRepo.repo1).toEqual([refreshed]) + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + }) + + it('keeps authoritative refreshes separate from non-authoritative in-flight results', async () => { + const store = createTestStore() + const fallback = makeWorktree({ + id: 'repo1::/path/fallback', + repoId: 'repo1', + path: '/path/fallback' + }) + const authoritative = makeWorktree({ + id: 'repo1::/path/authoritative', + repoId: 'repo1', + path: '/path/authoritative' + }) + let releaseFallback!: () => void + let releaseAuthoritative!: () => void + const fallbackStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseFallback = release + }) + return makeDetectedResult(repoId, [fallback], { + authoritative: false, + source: 'metadata-fallback' + }) + } + ) + }) + const authoritativeStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseAuthoritative = release + }) + return makeDetectedResult(repoId, [authoritative]) + } + ) + }) + + const bestEffortRequest = store.getState().fetchWorktrees('repo1') + await fallbackStarted + const authoritativeRequest = store + .getState() + .fetchWorktrees('repo1', { requireAuthoritative: true }) + await authoritativeStarted + + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(2) + + releaseFallback() + await expect(bestEffortRequest).resolves.toBe(false) + releaseAuthoritative() + await expect(authoritativeRequest).resolves.toBe(true) + + expect(store.getState().worktreesByRepo.repo1).toEqual([authoritative]) + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(2) + }) + + it('keeps same-repo refreshes separate for different execution hosts', async () => { + const store = createTestStore() + const localWorktree = makeWorktree({ + id: 'repo1::/local/wt1', + repoId: 'repo1', + path: '/local/wt1' + }) + const sshWorktree = makeWorktree({ + id: 'repo1::/ssh/wt1', + repoId: 'repo1', + path: '/home/orca/wt1' + }) + let releaseLocal!: () => void + let releaseSsh!: () => void + const localStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseLocal = release + }) + return makeDetectedResult(repoId, [localWorktree]) + } + ) + }) + const sshStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseSsh = release + }) + return makeDetectedResult(repoId, [sshWorktree]) + } + ) + }) + store.setState({ + hasHydratedWorktreePurge: true, + repos: [ + { + id: 'repo1', + path: '/local/repo1', + displayName: 'Repo One', + badgeColor: '#000', + addedAt: 0, + executionHostId: 'local' + }, + { + id: 'repo1', + path: '/home/orca/repo1', + displayName: 'Repo One SSH', + badgeColor: '#000', + addedAt: 0, + connectionId: 'ssh-1' + } + ] + } as Partial) + + const refresh = store.getState().fetchAllWorktrees() + await Promise.all([localStarted, sshStarted]) + + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(2) + + releaseLocal() + releaseSsh() + await refresh + + expect(store.getState().worktreesByRepo.repo1).toEqual([ + localWorktree, + { ...sshWorktree, hostId: 'ssh:ssh-1' } + ]) + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(2) + }) + + it('preserves SSH host identity when detected and visible refreshes overlap', async () => { + const store = createTestStore() + const sshWorktree = makeWorktree({ + id: 'repo-ssh::/home/orca/wt1', + repoId: 'repo-ssh', + path: '/home/orca/wt1' + }) + let releaseScan!: () => void + const scanStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseScan = release + }) + return makeDetectedResult(repoId, [sshWorktree]) + } + ) + }) + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + repos: [ + { + id: 'repo-ssh', + path: '/home/orca/repo', + displayName: 'SSH Repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'ssh-1' + } + ] + } as Partial) + + const detectedRequest = store.getState().fetchDetectedWorktrees('repo-ssh') + const visibleRequest = store.getState().fetchWorktrees('repo-ssh') + await scanStarted + + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + + releaseScan() + const [, visibleResult] = await Promise.all([detectedRequest, visibleRequest]) + + expect(visibleResult).toBe(true) + expect(store.getState().worktreesByRepo['repo-ssh']).toEqual([ + { ...sshWorktree, hostId: 'ssh:ssh-1' } + ]) + expect(store.getState().detectedWorktreesByRepo['repo-ssh']?.worktrees).toEqual([ + expect.objectContaining({ id: sshWorktree.id, hostId: 'ssh:ssh-1' }) + ]) + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + }) + + it('preserves SSH host identity when visible refresh starts before detected refresh', async () => { + const store = createTestStore() + const sshWorktree = makeWorktree({ + id: 'repo-ssh::/home/orca/wt1', + repoId: 'repo-ssh', + path: '/home/orca/wt1' + }) + let releaseScan!: () => void + const scanStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce( + async ({ repoId }: { repoId: string }) => { + resolve() + await new Promise((release) => { + releaseScan = release + }) + return makeDetectedResult(repoId, [sshWorktree]) + } + ) + }) + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + repos: [ + { + id: 'repo-ssh', + path: '/home/orca/repo', + displayName: 'SSH Repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'ssh-1' + } + ] + } as Partial) + + const visibleRequest = store.getState().fetchWorktrees('repo-ssh') + const detectedRequest = store.getState().fetchDetectedWorktrees('repo-ssh') + await scanStarted + + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + + releaseScan() + const [visibleResult] = await Promise.all([visibleRequest, detectedRequest]) + + expect(visibleResult).toBe(true) + expect(store.getState().worktreesByRepo['repo-ssh']).toEqual([ + { ...sshWorktree, hostId: 'ssh:ssh-1' } + ]) + expect(store.getState().detectedWorktreesByRepo['repo-ssh']?.worktrees).toEqual([ + expect.objectContaining({ id: sshWorktree.id, hostId: 'ssh:ssh-1' }) + ]) + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + }) + it('purges remembered right sidebar tabs for worktrees removed by a committed refresh', async () => { const store = createTestStore() const removed = makeWorktree({ diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 2c9b3ef5e..c300dcc16 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -72,6 +72,7 @@ const pendingActivationTerminalPrepCancels = new Map void>() const detachedHeadAutoDerivedDisplayNames = new Map() const folderWorkspaceWorktreeCache = new WeakMap() const hostedReviewPushTargetLookupsInFlight = new Set() +const detectedWorktreeRefreshesInFlight = new Map>() async function mapReposForWorktreeRefresh( repos: readonly TRepo[], @@ -825,6 +826,44 @@ async function listDetectedWorktreesForRepo( } } +function detectedWorktreeRefreshKey( + settings: AppState['settings'], + repoId: string, + options: { executionHostId: ExecutionHostId; requireAuthoritative?: boolean } +): string { + const target = getActiveRuntimeTarget(settings) + const targetKey = target.kind === 'local' ? 'local' : `runtime:${target.environmentId}` + return [ + repoId, + options.executionHostId, + targetKey, + options.requireAuthoritative === true ? 'authoritative' : 'best-effort' + ].join('\n') +} + +async function listDetectedWorktreesForRepoCoalesced( + settings: AppState['settings'], + repoId: string, + options: { executionHostId: ExecutionHostId; requireAuthoritative?: boolean } +): Promise { + const key = detectedWorktreeRefreshKey(settings, repoId, options) + const existing = detectedWorktreeRefreshesInFlight.get(key) + if (existing) { + return existing + } + // Why: startup/event fan-out can ask for the same repo/host refresh many + // times at once; share only the scan promise so state merge semantics stay local. + const refresh = listDetectedWorktreesForRepo(settings, repoId) + detectedWorktreeRefreshesInFlight.set(key, refresh) + try { + return await refresh + } finally { + if (detectedWorktreeRefreshesInFlight.get(key) === refresh) { + detectedWorktreeRefreshesInFlight.delete(key) + } + } +} + async function listWorktreeLineageForRuntime(settings: AppState['settings']): Promise<{ worktreeLineageById: Record workspaceLineageByChildKey: Record @@ -1691,12 +1730,26 @@ export const createWorktreeSlice: StateCreator fetchDetectedWorktrees: async (repoId) => { try { - const result = await listDetectedWorktreesForRepo(settingsForRepoOwner(get(), repoId), repoId) - set((s) => - areDetectedWorktreeResultsEqual(s.detectedWorktreesByRepo[repoId], result) - ? s - : { detectedWorktreesByRepo: { ...s.detectedWorktreesByRepo, [repoId]: result } } + const ownerState = get() + const hostId = repoHostId(ownerState, repoId) + const result = await listDetectedWorktreesForRepoCoalesced( + settingsForRepoOwner(ownerState, repoId, hostId), + repoId, + { executionHostId: hostId } ) + set((s) => { + // Why: detected-only refreshes can overlap host-scoped visible refreshes; + // keep detected state stamped/merged so SSH/runtime rows are not clobbered. + const mergedDetected = mergeDetectedWorktreesForHost( + s.detectedWorktreesByRepo[repoId], + result, + hostId, + worktreeHostMatchOptions(s, repoId, hostId) + ) + return areDetectedWorktreeResultsEqual(s.detectedWorktreesByRepo[repoId], mergedDetected) + ? s + : { detectedWorktreesByRepo: { ...s.detectedWorktreesByRepo, [repoId]: mergedDetected } } + }) return result } catch (err) { console.error(`Failed to fetch detected worktrees for repo ${repoId}:`, err) @@ -1709,7 +1762,10 @@ export const createWorktreeSlice: StateCreator const ownerState = get() const hostId = repoHostId(ownerState, repoId) const settings = settingsForRepoOwner(ownerState, repoId, hostId) - const detected = await listDetectedWorktreesForRepo(settings, repoId) + const detected = await listDetectedWorktreesForRepoCoalesced(settings, repoId, { + executionHostId: hostId, + requireAuthoritative: options?.requireAuthoritative + }) if (options?.requireAuthoritative && !detected.authoritative) { return false } @@ -1834,7 +1890,9 @@ export const createWorktreeSlice: StateCreator await mapReposForWorktreeRefresh(repos, async (r) => { const hostId = getRepoExecutionHostId(r) const settings = settingsForKnownRepoOwner(get().settings, r) - const detected = await listDetectedWorktreesForRepo(settings, r.id) + const detected = await listDetectedWorktreesForRepoCoalesced(settings, r.id, { + executionHostId: hostId + }) const worktrees = toVisibleWorktrees(detected, hostId) set((s) => { const matchOptions = worktreeHostMatchOptions(s, r.id, hostId) @@ -1890,9 +1948,10 @@ export const createWorktreeSlice: StateCreator > => { try { const hostId = getRepoExecutionHostId(r) - const detected = await listDetectedWorktreesForRepo( + const detected = await listDetectedWorktreesForRepoCoalesced( settingsForKnownRepoOwner(get().settings, r), - r.id + r.id, + { executionHostId: hostId } ) const list = toVisibleWorktrees(detected, hostId) const current = get().worktreesByRepo[r.id]